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

Use a shared connection pool for fetching gems #7079

Merged
merged 1 commit into from Oct 19, 2023

Conversation

segiddins
Copy link
Member

Closes #7076

Bundler will now use the same (shared) remote fetcher instance that
RubyGems uses.

This will allow installs to use a shared connection pool, which
represents a significant performance improvement on a clean install.

Closes #7076

Bundler will now use the same (shared) remote fetcher instance that
RubyGems uses.

This will allow installs to use a shared connection pool, which
represents a significant performance improvement on a clean install.
Copy link
Member

@martinemde martinemde left a comment

Choose a reason for hiding this comment

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

LOL. I had to double take... "how could this be that simple?" I didn't even notice this method on RemoteFetcher but that looks like exactly the right solution.

Copy link
Member

@deivid-rodriguez deivid-rodriguez left a comment

Choose a reason for hiding this comment

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

Hard to beat such a simple patch!

@simi simi enabled auto-merge October 19, 2023 18:30
@simi simi merged commit 54a7a7a into master Oct 19, 2023
92 checks passed
@simi simi deleted the reuse-gem-remotefetcher-instance-in-bundler branch October 19, 2023 21:40
@rye-stripe
Copy link

rye-stripe commented Oct 23, 2023

Thank you for the fix @segiddins!

When I was thinking about how to solve this, I thought sharing the same fetcher object between multiple threads (when --jobs flag is set) could lead to issues. In particular, each thread would clobber the http headers set by other threads on this line. Furthermore, the headers are never reset after the requests, so they may "leak" to further users of GemFetcher. Are these not valid concerns?

@segiddins
Copy link
Member Author

#7092

@rye-stripe
Copy link

That PR addresses my concerns, thanks!

deivid-rodriguez pushed a commit that referenced this pull request Nov 8, 2023
…ce-in-bundler

Reuse Gem::RemoteFetcher instance in bundler

(cherry picked from commit 54a7a7a)
deivid-rodriguez pushed a commit that referenced this pull request Nov 8, 2023
…ce-in-bundler

Reuse Gem::RemoteFetcher instance in bundler

(cherry picked from commit 54a7a7a)
@deivid-rodriguez deivid-rodriguez changed the title Reuse Gem::RemoteFetcher instance in bundler Use a shared connection pool for fetching gems Dec 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Connections not reused for gem downloads
5 participants