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

Split nailgun server and client inputs #13787

Closed
stuhood opened this issue Dec 3, 2021 · 0 comments · Fixed by #13813
Closed

Split nailgun server and client inputs #13787

stuhood opened this issue Dec 3, 2021 · 0 comments · Fixed by #13813
Assignees

Comments

@stuhood
Copy link
Member

stuhood commented Dec 3, 2021

Triage in #13779 showed that materializing the inputs for nailgunned JVM processes represented up to a 300ms constant factor.

But because those inputs currently include both the use_nailgun: Digest and the input_files: Digest fields (because the use_nailgun digest must be a subset of the input_digest: see), a lot of that work is completely redundant. On top of that, because we have materialized more (unnecessary) stuff into the sandbox, we have more to clean up afterwards.

This hits source analysis processes for Java/Scala particularly hard: in some cases, it represented ~500ms of total overhead on ~150ms processes.

@stuhood stuhood self-assigned this Dec 4, 2021
stuhood added a commit that referenced this issue Dec 6, 2021
As discussed in #13787, the `input_files` digest must currently include the `use_nailgun` digest, and that means that the input files for the server are materialized for every run.
* Add an `InputDigests` struct to encapsulate managing the collection of input digests for a `Process` (which will soon also include the immutable digests from #12716), and split the `use_nailgun` digest from the `input_files` digest.
* Move nailgun spawning (which uses `std::process` and `std::fs`, and so is synchronous) onto the `Executor`.
* Adjust the (still hardcoded) nailgun pool size to keep idle servers beyond the number that are currently active (e.g. to allow for idle `javac` processes while `scalac` processes are active).

Collectively, these changes make compilation 40% faster.

Fixes #13787.
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 a pull request may close this issue.

1 participant