-
Notifications
You must be signed in to change notification settings - Fork 5.3k
Replace deprecated wmic with MSBuild parallelism properties #122386
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
base: main
Are you sure you want to change the base?
Conversation
- Remove wmic-based NumberOfCores detection from build-runtime.cmd and build.cmd - Replace with UseMultiToolTask, EnforceProcessCountAcrossBuilds, and EnableClServerMode environment variables - Remove CL_MPCount property from Directory.Build.props which depended on NumberOfCores - These changes follow Microsoft's recommended approach for C++ build parallelism Co-authored-by: akoeplinger <1376924+akoeplinger@users.noreply.github.com>
|
I tested building |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR replaces deprecated wmic usage with modern MSBuild environment variables for controlling C++ build parallelism on Windows. The change addresses issues on newer Windows builds where wmic is no longer available.
- Removes
wmic cpu get NumberOfCoresdetection logic from both CoreCLR and test build scripts - Implements Microsoft's recommended MSBuild parallelism settings via environment variables
- Cleans up the now-unused
CL_MPCountproperty from CoreCLR build configuration
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/tests/build.cmd | Replaces wmic-based core detection with MSBuild parallelism environment variables |
| src/coreclr/build-runtime.cmd | Replaces wmic-based core detection with MSBuild parallelism environment variables |
| src/coreclr/Directory.Build.props | Removes obsolete CL_MPCount property that was derived from NumberOfCores |
The changes look good overall. The implementation correctly follows Microsoft's recommended approach for C++ build parallelism. The removal of wmic usage is complete, and the new MSBuild environment variables (UseMultiToolTask, EnforceProcessCountAcrossBuilds, and EnableClServerMode) are set appropriately in both affected build scripts. All references to NumberOfCores, wmic, and CL_MPCount have been properly removed from the codebase.
The intent of The metric to measure is the build wallclock time. The build wallclock time is very noisy. I usually do these kind of experiments by running the build in a loop overnight and computing average time.
|
- Remove environment variable settings from build-runtime.cmd and build.cmd - Add UseMultiToolTask, EnforceProcessCountAcrossBuilds, and EnableClServerMode as MSBuild properties in root Directory.Build.props - Keep wmic removal and CL_MPCount removal from previous commit Co-authored-by: akoeplinger <1376924+akoeplinger@users.noreply.github.com>
jkotas
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you please validate that building (with msbuild) is about as fast as before?
LGTM otherwise
|
I've validated that the build times are basically the same on my 16 core devbox (old: 11m7s, new: 10m51s) when using -msbuild on a restored repo. |
wmicis deprecated and missing on newer Windows builds. The build scripts used it to detect processor cores for CL parallelism tuning viaCL_MPCount.Changes:
wmic cpu get NumberOfCoresdetection fromsrc/coreclr/build-runtime.cmdandsrc/tests/build.cmdCL_MPCountproperty fromsrc/coreclr/Directory.Build.propsDirectory.Build.propsto control build parallelism:UseMultiToolTask=trueEnforceProcessCountAcrossBuilds=trueEnableClServerMode=trueThe MSBuild properties are set globally in the root Directory.Build.props file (in a separate PropertyGroup right after the Arcade SDK import) so they apply to all projects in the repository.
This follows Microsoft's recommended approach for C++ build parallelism on modern systems.
Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.