Skip to content
This repository has been archived by the owner on Feb 28, 2021. It is now read-only.

Be more frugal with HTTP client thread pool #145

Merged
merged 2 commits into from
Mar 27, 2018

Conversation

retronym
Copy link
Member

@retronym retronym commented Mar 5, 2018

  • Ensure that we don't needlessly create HTTP clients and leak threads
  • Lazily initialize the HTTP client (and its thread pool)

More details in the commit comments.

Historical context:

Prior to #53, the HTTP client
was created and discarded on each operation (which wasn't leaky but
was presumably a drag on performance.) I notice that in that change,
a close operation was added to BintrayRepo but it isn't actually
called.

Fixes #144

retronym added 2 commits March 5, 2018 15:51
ConcurrentMap.getOrElseUpdate may run the thunk without eventually
insering the computed value in the map, if multiple threads are
calling it for a given key. This situation seems to happen in SBT
setting evaluation, perhaps in multi-module builds. The upshot is
that Netty creates 2 x $numcpus "New NIO Worker" threads that
live until the JVM exits.

This commit adds synchronization around the interaction with the
cache to ensure a given cache key only creates a single HTTP client
and thread pool.

Future work could reuse a HTTP client for all of Bintray's usage.
I haven't made that change because I can't find documentation of
the threadsafety of dispatch.Http.
The idea is to avoid paying for these threads until one actually
publishes something.
@retronym retronym requested a review from eed3si9n March 5, 2018 06:20
@retronym
Copy link
Member Author

retronym commented Mar 5, 2018

I'm not sure how much automated testing we have here, so this might need manual testing before merge.

@2m
Copy link
Member

2m commented Mar 27, 2018

I have tried these changes on a simple project and it seems to help. Loading sbt with the sbt-bintray 0.5.3:

before

Loading sbt with the sbt-bintray from this PR:

after

Copy link
Member

@dwijnand dwijnand left a comment

Choose a reason for hiding this comment

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

lgtm

@eed3si9n eed3si9n merged commit cf4dfa1 into sbt:master Mar 27, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Plugin creates an unreasonable amount of "New I/O worker #NN" threads
4 participants