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 spawning excessive conhost processes on Windows #1143

Merged
merged 1 commit into from
May 20, 2022

Conversation

FooBarWidget
Copy link
Contributor

Fixes #514 (comment) (comment 2022-03-08 by @mitchhentges)

Any console-based subprocess spawned with CREATE_NO_WINDOW actually has a hidden console, and thus an associated conhost process. Since the sccache server is already started with CREATE_NO_WINDOW, it's unnecessary to spawn further subprocesses with CREATE_NO_WINDOW. Removing this flag allows subprocesses to share the sccache server's hidden console, thus avoiding each subprocess from getting its own conhost.

For an extended explanation see this comment and its follow-ups:
#514 (comment)

@FooBarWidget
Copy link
Contributor Author

CI is breaking due to a Rust compiler panic on thirtyfour. Not sure what this is.

@mitchhentges
Copy link
Contributor

CI is breaking due to a Rust compiler panic on thirtyfour. Not sure what this is.

Looks like this nightly ICE.

Copy link
Contributor

@mitchhentges mitchhentges left a comment

Choose a reason for hiding this comment

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

Makes sense, thanks for the patch! Let's land this after v0.3.0, because this is no longer an issue-blocking-release and I'd like to avoid re-running the benchmarking tasks with this commit included.

@mitchhentges
Copy link
Contributor

Hey, heads up that v0.3.0 was released, so this should be unblocked: it might be worth rebasing off main, perhaps that'll jolt CI into being green.

Fixes mozilla#514 (comment) (comment 2022-03-08 by @mitchhentges)

Any console-based subprocess spawned with CREATE_NO_WINDOW actually has a hidden console, and thus an associated conhost process. Since the sccache server is already started with CREATE_NO_WINDOW, it's unnecessary to spawn further subprocesses with CREATE_NO_WINDOW. Removing this flag allows subprocesses to share the sccache server's hidden console, thus avoiding each subprocess from getting its own conhost.

For an extended explanation see this comment and its follow-ups:
mozilla#514 (comment)
@codecov-commenter
Copy link

codecov-commenter commented May 6, 2022

Codecov Report

Merging #1143 (f5d5584) into main (5032a83) will decrease coverage by 0.17%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #1143      +/-   ##
==========================================
- Coverage   35.11%   34.94%   -0.18%     
==========================================
  Files          47       47              
  Lines       12774    12770       -4     
  Branches     6637     6635       -2     
==========================================
- Hits         4486     4462      -24     
- Misses       3986     3988       +2     
- Partials     4302     4320      +18     
Impacted Files Coverage Δ
src/mock_command.rs 50.93% <ø> (-1.84%) ⬇️
src/util.rs 42.10% <ø> (ø)
src/test/utils.rs 36.36% <0.00%> (-3.04%) ⬇️
src/azure/blobstore.rs 20.19% <0.00%> (-0.99%) ⬇️
src/compiler/args.rs 59.67% <0.00%> (-0.90%) ⬇️
src/compiler/gcc.rs 57.04% <0.00%> (-0.48%) ⬇️
src/lru_disk_cache/mod.rs 40.72% <0.00%> (-0.31%) ⬇️
src/compiler/compiler.rs 34.92% <0.00%> (-0.16%) ⬇️
src/compiler/msvc.rs 43.55% <0.00%> (-0.15%) ⬇️
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5032a83...f5d5584. Read the comment docs.

@FooBarWidget
Copy link
Contributor Author

I see you already did the rebase yourself. All good now?

@drahnr drahnr merged commit d5c8f4d into mozilla:main May 20, 2022
emabrey pushed a commit to emabrey/sccache that referenced this pull request Aug 10, 2022
Fixes mozilla#514 (comment) (comment 2022-03-08 by @mitchhentges)

Any console-based subprocess spawned with CREATE_NO_WINDOW actually has a hidden console, and thus an associated conhost process. Since the sccache server is already started with CREATE_NO_WINDOW, it's unnecessary to spawn further subprocesses with CREATE_NO_WINDOW. Removing this flag allows subprocesses to share the sccache server's hidden console, thus avoiding each subprocess from getting its own conhost.

For an extended explanation see this comment and its follow-ups:
mozilla#514 (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.

Windows compiler invocation opens multiple console windows
4 participants