Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Download source code archive directly for ubuntu and macos setup #9198

Closed

Conversation

liujiayi771
Copy link
Contributor

@liujiayi771 liujiayi771 commented Mar 21, 2024

It is better to use wget_and_untar to download the source code archive
directly, instead of using git to clone the source code. Because direct download
is much faster than using git clone. The setup-centos8.sh script has been
consistently downloading source code archive directly.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Mar 21, 2024
Copy link

netlify bot commented Mar 21, 2024

Deploy Preview for meta-velox canceled.

Name Link
🔨 Latest commit 6346875
🔍 Latest deploy log https://app.netlify.com/sites/meta-velox/deploys/66a1e5e59a527e0008dbffe1

@liujiayi771
Copy link
Contributor Author

In our test environment, for boost, it took about 11 min before (from 2024-03-21T06:20:47 to 2024-03-21T06:31:13):

2024-03-21T06:20:47.3262778Z + run_and_time install_boost
2024-03-21T06:20:47.3263537Z + install_boost
2024-03-21T06:20:47.3264743Z + github_checkout boostorg/boost boost-1.84.0 --recursive
2024-03-21T06:20:47.3271539Z + local REPO=boostorg/boost
2024-03-21T06:20:47.3272490Z + shift
2024-03-21T06:20:47.3273275Z + local VERSION=boost-1.84.0
2024-03-21T06:20:47.3274005Z + shift
2024-03-21T06:20:47.3274716Z + local GIT_CLONE_PARAMS=--recursive
2024-03-21T06:20:47.3275628Z ++ basename boostorg/boost
2024-03-21T06:20:47.3276434Z + Finished running install_fmt
2024-03-21T06:20:47.3277281Z + local DIRNAME=boost
2024-03-21T06:20:47.3278123Z + SUDO='sudo --preserve-env'
2024-03-21T06:20:47.3279167Z + cd /opt/gluten/ep/build-velox/build/velox_ep
2024-03-21T06:20:47.3280153Z + '[' -z boost ']'
2024-03-21T06:20:47.3281462Z + '[' -d boost ']'
2024-03-21T06:20:47.3282170Z + '[' '!' -d boost ']'
2024-03-21T06:20:47.3283545Z + git clone -q -b boost-1.84.0 --recursive https://github.com/boostorg/boost.git
2024-03-21T06:21:00.5554970Z Note: switching to 'ad09f667e61e18f5c31590941e748ac38e5a81bf'.
2024-03-21T06:21:00.5556728Z 
2024-03-21T06:21:00.5557598Z You are in 'detached HEAD' state. You can look around, make experimental
2024-03-21T06:21:00.5559211Z changes and commit them, and you can discard any commits you make in this
2024-03-21T06:21:00.5560889Z state without impacting any branches by switching back to a branch.
2024-03-21T06:21:00.5561865Z 
2024-03-21T06:21:00.5562552Z If you want to create a new branch to retain commits you create, you may
2024-03-21T06:21:00.5564281Z do so (now or later) by using -c with the switch command. Example:
2024-03-21T06:21:00.5565210Z 
2024-03-21T06:21:00.5565585Z   git switch -c <new-branch-name>
2024-03-21T06:21:00.5667647Z 
2024-03-21T06:21:00.5668525Z Or undo this operation with:
2024-03-21T06:21:00.5669179Z 
2024-03-21T06:21:00.5669430Z   git switch -
2024-03-21T06:21:00.5788570Z 
2024-03-21T06:21:00.5869925Z Turn off this advice by setting config variable advice.detachedHead to false
2024-03-21T06:21:00.5948424Z 
2024-03-21T06:31:13.8519110Z + cd boost

It took about 20s now (from 2024-03-21T08:25:15 to 2024-03-21T08:25:32)

2024-03-21T08:25:15.1901014Z + run_and_time install_boost
2024-03-21T08:25:15.1902104Z + install_boost
2024-03-21T08:25:15.1904364Z + wget_and_untar https://github.com/boostorg/boost/releases/download/boost-1.84.0/boost-1.84.0.tar.gz boost
2024-03-21T08:25:15.1907965Z + local URL=https://github.com/boostorg/boost/releases/download/boost-1.84.0/boost-1.84.0.tar.gz
2024-03-21T08:25:15.1909631Z + local DIR=boost
2024-03-21T08:25:15.1910348Z + mkdir -p boost
2024-03-21T08:25:15.1922276Z + pushd boost
2024-03-21T08:25:15.1924758Z + curl -L https://github.com/boostorg/boost/releases/download/boost-1.84.0/boost-1.84.0.tar.gz
2024-03-21T08:25:15.1927531Z /opt/gluten/ep/build-velox/build/velox_ep/fmt/boost /opt/gluten/ep/build-velox/build/velox_ep/fmt
2024-03-21T08:25:15.2047924Z   % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
2024-03-21T08:25:15.2049463Z                                  Dload  Upload   Total   Spent    Left  Speed
2024-03-21T08:25:15.2050292Z 
2024-03-21T08:25:16.2292144Z   0     0    0     0    0     0      0      0 --:--:-- --:--:-- --:--:--     0
2024-03-21T08:25:16.2293827Z   0     0    0     0    0     0      0      0 --:--:--  0:00:01 --:--:--     0
2024-03-21T08:25:16.2295320Z   0     0    0     0    0     0      0      0 --:--:--  0:00:01 --:--:--     0
2024-03-21T08:25:17.3928559Z 
2024-03-21T08:25:18.1827510Z   0  118M    0  7421    0     0   3391      0 10:08:26  0:00:02 10:08:24  3391
2024-03-21T08:25:19.1764844Z   0  118M    0  191k    0     0  65753      0  0:31:22  0:00:02  0:31:20  232k
2024-03-21T08:25:20.1637281Z   4  118M    4 5913k    0     0  1489k      0  0:01:21  0:00:03  0:01:18 3310k
2024-03-21T08:25:21.1679483Z  15  118M   15 18.4M    0     0  3807k      0  0:00:31  0:00:04  0:00:27 6811k
2024-03-21T08:25:22.1508048Z  26  118M   26 31.2M    0     0  5364k      0  0:00:22  0:00:05  0:00:17 8469k
2024-03-21T08:25:23.1499963Z  37  118M   37 44.0M    0     0  6490k      0  0:00:18  0:00:06  0:00:12 9473k
2024-03-21T08:25:24.1504551Z  48  118M   48 56.9M    0     0  7343k      0  0:00:16  0:00:07  0:00:09 11.4M
2024-03-21T08:25:25.1513762Z  59  118M   59 69.8M    0     0  8000k      0  0:00:15  0:00:08  0:00:07 12.8M
2024-03-21T08:25:26.1492539Z  70  118M   70 82.8M    0     0  8525k      0  0:00:14  0:00:09  0:00:05 12.9M
2024-03-21T08:25:27.1490347Z  81  118M   81 95.8M    0     0  8966k      0  0:00:13  0:00:10  0:00:03 12.9M
2024-03-21T08:25:27.7938709Z  92  118M   92  108M    0     0  9332k      0  0:00:12  0:00:11  0:00:01 12.9M
2024-03-21T08:25:27.7941828Z 100  118M  100  118M    0     0  9603k      0  0:00:12  0:00:12 --:--:-- 13.1M
2024-03-21T08:25:27.8029221Z + tar -xz --strip-components=1 -f boost.tar.gz
2024-03-21T08:25:32.2022371Z + popd
2024-03-21T08:25:32.2023384Z + cd boost

@liujiayi771 liujiayi771 force-pushed the ubuntu-download branch 4 times, most recently from c694651 to 99cca16 Compare March 21, 2024 13:52
@liujiayi771
Copy link
Contributor Author

@kgpai Could you help to review?

@liujiayi771
Copy link
Contributor Author

cc @assignUser. Could you help to review?

@liujiayi771 liujiayi771 force-pushed the ubuntu-download branch 2 times, most recently from b545d28 to fdbcd20 Compare March 26, 2024 01:38
Copy link
Collaborator

@assignUser assignUser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't have any objections to this.

The number of submodules is the issue with boost, as a workaround there is a git config that allows you to increase the number of parallel submodule checkouts.

scripts/setup-ubuntu.sh Outdated Show resolved Hide resolved
@liujiayi771 liujiayi771 force-pushed the ubuntu-download branch 2 times, most recently from 7565004 to 2540184 Compare April 2, 2024 01:53
@liujiayi771
Copy link
Contributor Author

liujiayi771 commented Apr 2, 2024

@assignUser @kgpai Could you help review again? I have added the functionality to change directory to a specific path in the cmake_install method.

@assignUser
Copy link
Collaborator

Sorry for the delay, could you rebase again?

@liujiayi771
Copy link
Contributor Author

@assignUser Rebased to the latest main branch.

Copy link
Collaborator

@assignUser assignUser left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM @majetideepak you have also worked on the setup script extensively recently, any objections?

@assignUser assignUser requested a review from majetideepak April 13, 2024 02:51
@kgpai
Copy link
Contributor

kgpai commented Apr 13, 2024

@liujiayi771 I havent noticed any particular slowness with git and it seems roughly equivalent to download. The advantage of using git is that its probably a little more secure than plain old downloads of tar.gz's (FYI the checksums we calculate arent really secure anyway).
Do other folks see significant differences b/w git and downloading tar.gz's ?
@majetideepak ?

@liujiayi771
Copy link
Contributor Author

liujiayi771 commented Apr 14, 2024

@kgpai In my test, the time difference for Boost is quite significant, and the script for CentOS 8 does not use git currently, so both methods should work.

Copy link

stale bot commented Jul 13, 2024

This pull request has been automatically marked as stale because it has not had recent activity. If you'd still like this PR merged, please comment on the PR, make sure you've addressed reviewer comments, and rebase on the latest main. Thank you for your contributions!

@stale stale bot added the stale label Jul 13, 2024
@majetideepak
Copy link
Collaborator

majetideepak commented Jul 19, 2024

Sorry, I missed this PR. I think we should uniformly use tar.gz for all because 1) We can then use a common shared file across the platforms to download and install all the dependencies. 2) Downloading a single file should be better than git (involves extra metadata + processing)?
We can also include the checksums in a single file to validate.
@liujiayi771 can you please rebase this PR?

@stale stale bot removed the stale label Jul 19, 2024
@majetideepak majetideepak self-assigned this Jul 19, 2024
@liujiayi771
Copy link
Contributor Author

@majetideepak Rebased to the latest main branch.

Copy link
Collaborator

@majetideepak majetideepak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @liujiayi771

@majetideepak majetideepak added the ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall label Jul 20, 2024
Copy link
Collaborator

@majetideepak majetideepak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@liujiayi771 one change needed.

scripts/setup-helper-functions.sh Outdated Show resolved Hide resolved
@majetideepak majetideepak removed the ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall label Jul 20, 2024
@majetideepak majetideepak added the ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall label Jul 21, 2024
@majetideepak
Copy link
Collaborator

@Yuhta, can you please import this PR? Thanks!

@facebook-github-bot
Copy link
Contributor

@Yuhta has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link
Contributor

@Yuhta merged this pull request in b5713ef.

Copy link

Conbench analyzed the 1 benchmark run on commit b5713efc.

There was 1 benchmark result indicating a performance regression:

The full Conbench report has more details.

liujiayi771 added a commit to liujiayi771/velox that referenced this pull request Jul 28, 2024
…ebookincubator#9198)

Summary:
It is better to use wget_and_untar to download the source code archive
directly, instead of using git to clone the source code. Because direct download
is much faster than using git clone. The setup-centos8.sh script has been
consistently downloading source code archive directly.

Pull Request resolved: facebookincubator#9198

Reviewed By: kgpai

Differential Revision: D60288820

Pulled By: Yuhta

fbshipit-source-id: acf111d61673e8648adf0e82319375de92a2e942
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. Merged ready-to-merge PR that have been reviewed and are ready for merging. PRs with this tag notify the Velox Meta oncall
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants