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

Add symlinked immutable Process inputs #13848

Merged
merged 8 commits into from
Dec 10, 2021

Conversation

stuhood
Copy link
Member

@stuhood stuhood commented Dec 9, 2021

As designed by @tdyas in #12716, this change adds immutable_inputs to Process, and adapts the JVM @rules to use them: for now, only for the tools themselves. A followup change will move all JVM classpath inputs to immutable_inputs.

Nailgun support is now enabled by specifying the subset of the immutable_inputs which should be used to start the server: all others will be used to start the client.

[ci skip-build-wheels]

@stuhood stuhood force-pushed the stuhood/immutable-caches branch 2 times, most recently from d5fa12c to 16d4cb4 Compare December 9, 2021 19:00
@stuhood stuhood force-pushed the stuhood/immutable-caches branch 2 times, most recently from bff632b to 3a9593b Compare December 9, 2021 22:47
@stuhood stuhood marked this pull request as ready for review December 9, 2021 22:55
@stuhood
Copy link
Member Author

stuhood commented Dec 9, 2021

This should now be ready for review. The first six commits include work from @tdyas, and are probably easiest reviewed as one diff (since I went back and forth a few times and renamed the field), and the last commit is useful to look at independently.

Copy link
Contributor

@jsirois jsirois left a comment

Choose a reason for hiding this comment

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

I just looked at the rust side of this.

src/rust/engine/process_execution/src/immutable_inputs.rs Outdated Show resolved Hide resolved
src/rust/engine/process_execution/src/lib.rs Outdated Show resolved Hide resolved
@stuhood stuhood force-pushed the stuhood/immutable-caches branch 2 times, most recently from 321b2e5 to fd9a9e5 Compare December 10, 2021 19:04
stuhood added a commit that referenced this pull request Dec 10, 2021
…-only. (#13857)

To make #13848 more rigorous, directory structures that are created for use as immutable inputs should be created as readonly. This change adds support to `materialize_directory` for creating files/directories as readonly.

[ci skip-build-wheels]
Tom Dyas and others added 7 commits December 10, 2021 12:42
[ci skip-build-wheels]
[ci skip-build-wheels]
[ci skip-build-wheels]
# Building wheels and fs_util will be skipped. Delete if not intended.
[ci skip-build-wheels]
… list of `immutable_inputs` paths.

[ci skip-build-wheels]
@stuhood stuhood enabled auto-merge (squash) December 10, 2021 21:01
[ci skip-build-wheels]
@stuhood stuhood merged commit 3abcb4c into pantsbuild:main Dec 10, 2021
@stuhood stuhood deleted the stuhood/immutable-caches branch December 10, 2021 23:38
stuhood added a commit that referenced this pull request 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]
@wisechengyi wisechengyi mentioned this pull request Dec 13, 2021
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.

3 participants