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

Refactor: Extract new crate simple-git #1304

Merged
merged 1 commit into from
Aug 19, 2023
Merged

Refactor: Extract new crate simple-git #1304

merged 1 commit into from
Aug 19, 2023

Conversation

NobodyXu
Copy link
Member

binstalk-downloader contains stuff about http(s) before the git code is moved into it and now it becomes http and git.

While git indeed uses http stuff, which is why I decided to put it into binstalk-downloader, it is more than just downloading since it is stateful (can be cached locally and updated) where as http is stateless.

Also binstalk-downloader's codegen time now increases dramatically and it also creates extra dependencies for binstalk-fetchers, delaying its execution.

The git code also don't use anything from binstalk-downloader at all, it makes sense to be an independent crate.

@NobodyXu
Copy link
Member Author

@passcod @Byron Just realize that progress_tracing implementation actually uses tokio, so that part cannot be merged into gix trivally.

`binstalk-downloader` contains stuff about http(s) before the
git code is moved into it and now it becomes http and git.

While git indeed uses http stuff, which is why I decided to put
it into binstalk-downloader, it is more than just downloading
since it is stateful (can be cached locally and updated)
where as http is stateless.

Also `binstalk-downloader`'s codegen time now increases
dramatically and it also creates extra dependencies for
binstalk-fetchers, delaying its execution.

The git code also don't use anything from `binstalk-downloader`
at all, it makes sense to be an independent crate.

Signed-off-by: Jiahao XU <Jiahao_XU@outlook.com>
@NobodyXu
Copy link
Member Author

On x86_64h-apple-darwin, this indeed improves compilation time a lot:

image

In other targets where caching is effective, the build time is mainly limited by:

  • the order of compilation: cargo decided to compile and run cargo-binstall build-script quite late in the process
  • not enough CPU cores

But this is still a win though given that simple-git took significantly longer to compile than binstalk-downloader, even if it contains less code.

@Byron
Copy link
Contributor

Byron commented Aug 18, 2023

I really appreciate the compile-time work you are doing here and since you are determined to do so, if you want to lead this in gitoxide and start earlier than I planned, we can do that.

I'd rather learn to work with the difficulties that come with extensive feature toggling than to pass up on the opportunity. The work to improve compile times has to be put in at some point anyway, and starting early allows for these costs to be incremental, especially if the workflow issues around feature toggles are resolvable (i.e. optimizing compile times shouldn't be a permanent tax on development time).

@NobodyXu NobodyXu added this pull request to the merge queue Aug 19, 2023
Merged via the queue into main with commit dc77a1a Aug 19, 2023
27 checks passed
@NobodyXu NobodyXu deleted the refactor-git branch August 19, 2023 01:36
@NobodyXu
Copy link
Member Author

@passcod Invitation sent!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants