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

Fix sbt/sbt#2982: Add a parallel Ivy engine #90

Merged
merged 1 commit into from May 2, 2017

Conversation

Projects
None yet
4 participants
@jvican
Copy link
Member

commented Apr 26, 2017

This is a port of sbt/sbt#2992.

Original description of the feature:

Ivy downloads have traditionally been single-threaded.

Parallel downloads are a must for a modern build tool. This commit
builds upon the work done by Josh Suereth in the branch
sbt/ivy-parallel-download-artifact.

To avoid adding external dependencies, it uses the Scala parallel
collections. If maintainers consider that is worth it to use a more
modern and appropriate approach, like Task, I'm happy to reimplement
the features with it.

Co-authored-by: Josh Suereth joshua.suereth@gmail.com

@eed3si9n eed3si9n added the ready label Apr 26, 2017

Fix sbt/sbt#2982: Add a parallel Ivy engine
This is a port of sbt/sbt#2992.

Original description of the feature:

```
Ivy downloads have traditionally been single-threaded.

Parallel downloads are a must for a modern build tool. This commit
builds upon the work done by Josh Suereth in the branch
sbt/ivy-parallel-download-artifact.

To avoid adding external dependencies, it uses the Scala parallel
collections. If maintainers consider that is worth it to use a more
modern and appropriate approach, like Task, I'm happy to reimplement
the features with it.
```

This commit does not preserve Josh's metadata in the commit since the
whole design of the repository has changed and I did not know how to
import a commit from sbt/sbt. However, it does apply several changes to
the original PR.

Co-authored-by: Josh Suereth <joshua.suereth@gmail.com>

@jvican jvican force-pushed the scalacenter:add-parallel-ivy branch from 44cb5c0 to 8c993d2 Apr 26, 2017

@jvican

This comment has been minimized.

Copy link
Member Author

commented Apr 26, 2017

Thanks to this PR, the CI tests run in 90 seconds instead of 150. This is ready for review.

@eed3si9n eed3si9n merged commit ec65213 into sbt:1.0 May 2, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@eed3si9n eed3si9n removed the ready label May 2, 2017

@jameskoch2

This comment has been minimized.

Copy link

commented Sep 8, 2017

As the author of a custom Resolver I wanted to poke around the thread-safety here, using 1.0.1 sources.

It looks to me like there's a small amount of mutable state inside most stock Resolvers' download() methods, by virtue of their reliance upon BasicResolver.download.

I haven't seen this cause any actual issues in my testing, but thought I'd mention it. It looks like this would only impact logging of failed download attempts (based on quick read of unfamiliar code).

@eed3si9n

This comment has been minimized.

Copy link
Member

commented Sep 8, 2017

@jameskoch2 Thanks for the comment! I am in fact seeing some unexplainable NPEs and inconsistent failure logging, especially with fresh Ivy cache, so maybe there's a race condition.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.