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

Improve performance of sandbox creation for Scala #13435

Closed
stuhood opened this issue Nov 1, 2021 · 4 comments
Closed

Improve performance of sandbox creation for Scala #13435

stuhood opened this issue Nov 1, 2021 · 4 comments
Assignees

Comments

@stuhood
Copy link
Member

stuhood commented Nov 1, 2021

Unless Scala compiler errors have dramatically improved (when transitive dependencies are not present, the errors in 2.11 at least tended to look like compiler crashes, but also did not give great information about what was missing), compilation support is likely to require transitive dependencies.

That will mean very large sandboxes, and will likely require either #12716 or potentially FUSE support.

@stuhood
Copy link
Member Author

stuhood commented Dec 6, 2021

#13779 showed that we will need to do some more work here, and that to reduce IO (and to dodge xprotect refingerprinting coursier each time we call it), we should implement #12716.

After implementing #12716, the last big performance unknown will be around using a JVM-per-core model. While JVM-per-core was implemented very intentionally (allows for precise memory usage, classpath isolation, cancellation, etc), and aligns with Bazel's default, Bazel experimentally supports sharing a worker via multiplex workers, which saves JIT warmup time. To prioritize that, we'd need to get a sense of how much CPU we're spending on JIT warmup.

@stuhood stuhood changed the title Address likely performance issue with sandbox creation for Scala Improve performance of sandbox creation for Scala Dec 6, 2021
@stuhood
Copy link
Member Author

stuhood commented Dec 6, 2021

Unless Scala compiler errors have dramatically improved (when transitive dependencies are not present, the errors in 2.11 at least tended to look like compiler crashes, but also did not give great information about what was missing), compilation support is likely to require transitive dependencies.

It's worth noting though that #13603 completely removed the need for transitive dependencies for Java, which is exciting. While it is likely that we can also lean in for Scala to add exported types to shrink sandboxes, I'm less certain of the timeline for that than I am of #12716 (which had a lot of groundwork laid in #13813).

EDIT: Opened #13836 for this.

@stuhood stuhood self-assigned this Dec 7, 2021
@stuhood
Copy link
Member Author

stuhood commented Dec 13, 2021

After #13862, the average sandbox creation time in our test repo is around 40 ms, so from that perspective, I think that we can call this issue closed.

The overall performance still needs some work (we're about half as fast as I'd like us to be), but that might have to go on the back burner until we have a bit more feature work completed.

stuhood added a commit that referenced this issue Dec 13, 2021
…3862)

As described in #13435, we expect large sandboxes for JVM compiles (for Scala in particular). #13848 added support for symlinking immutable inputs into sandboxes, and used it for tools. This change uses it for JVM classpaths as well.

In a test repo, this reduces the total amount of time taken to create sandboxes for compilation by ~30% (from 14s to 10s), with an average sandbox creation time of 40ms (including amortized materialization of the symlink destinations).

[ci skip-rust]
[ci skip-build-wheels]
@stuhood
Copy link
Member Author

stuhood commented Dec 13, 2021

Post #13848 and #13862, sandbox creation is no longer the long pole on JVM compilation performance. Resolving.

#13866 and #13836 are open for followup work.

@stuhood stuhood closed this as completed Dec 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant