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

Fork all sbt tasks #1000

Merged
merged 2 commits into from
Apr 16, 2019
Merged

Fork all sbt tasks #1000

merged 2 commits into from
Apr 16, 2019

Conversation

seldridge
Copy link
Member

@seldridge seldridge commented Jan 24, 2019

This causes all sbt test/run tasks (test, testOnly, and testQuick, run, runMain, Test / run, and Test / runMain) to fork to a separate JVM. This avoids a situation where repeatedly running tasks exhausts the JVM's metaspace. This doesn't necessarily appear to be our problem (I think it's in sbt/scalaTest), but this buys us some time to figure it out.

This happens for developers repeatedly running tasks (e.g., test or run) in the same sbt session. Tasks that are run in a -DminimalResources environment (e.g., CircleCI) will not fork and will continue to run serially.

More information:

The performance impact of this should only be the fork overhead. I spot checked this on a 6-core machine:

Baseline

real 3m24.726s
user 28m22.200s
sys 1m33.868s

Test / fork := true

real 8m59.627s
user 18m34.172s
sys 1m3.148s

Test / fork := true; Test / testForkedParallel := true

real 3m40.284s
user 28m34.132s
sys 1m31.152s

Related issue: chipsalliance/firrtl#978

Type of change: bug report

Impact: no functional change

Development Phase: implementation

Release Notes

(No release notes as this is a testing-only change and we don't publish tests.)

@seldridge seldridge requested a review from a team as a code owner January 24, 2019 19:07
@seldridge
Copy link
Member Author

One caveat with this is that this seems to serialize the tests in the forked JVM... That isn't the behavior I intended here. Looking into this.

@seldridge
Copy link
Member Author

Sbt needs an additional option to re-enable parallel execution in the forked JVM. That option took too long to find. This should provide the same performance as before (documented above) without the metaspace issues that crop up in testing.

@seldridge seldridge force-pushed the fork branch 3 times, most recently from 12805f7 to 98107b9 Compare January 25, 2019 22:17
@seldridge seldridge changed the title Fork the tests Fork all sbt tasks Jan 25, 2019
This causes sbt tasks (run, test, etc.) to fork to a separate JVM to
avoid running out of metaspace. This issue crops up for developers or
users repeatedly running sbt tasks in the same sbt session.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
Copy link
Contributor

@ucbjrl ucbjrl left a comment

Choose a reason for hiding this comment

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

lgtm - sorry to take so long to review this

@seldridge seldridge merged commit fe5571d into chipsalliance:master Apr 16, 2019
ucbjrl pushed a commit that referenced this pull request Apr 24, 2019
This causes sbt tasks (run, test, etc.) to fork to a separate JVM to
avoid running out of metaspace. This issue crops up for developers or
users repeatedly running sbt tasks in the same sbt session.

Signed-off-by: Schuyler Eldridge <schuyler.eldridge@ibm.com>
jackkoenig pushed a commit that referenced this pull request Feb 28, 2023
…pbundles (#1000)

* Attempt to deal with timing vagaries in UniquifySpec.quicklyrenamedeepbundles
Switching to Scala 2.12.8 cause this test to start failing on OSX. Try earlier scheme to compare shallow vs deep to reduce brittleness.

* Address review concerns; update comment.
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.

2 participants