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

Windows/CMake: add /MP for multi process MSBuild #227

Merged
merged 1 commit into from
Apr 28, 2024

Conversation

th1000s
Copy link
Contributor

@th1000s th1000s commented Apr 28, 2024

Description

"The /MP option can reduce the total time to compile the source files on the command line. The /MP option causes the compiler to create one or more copies of itself, each in a separate process."

https://learn.microsoft.com/en-us/cpp/build/reference/mp-build-with-multiple-processes?view=msvc-170


Yes, a compile option which modifies the build process...

Pull Request Type

  • GitHub Workflow changes
  • Documentation or Wiki changes
  • Build and Dependency changes
  • Runtime changes

Checklist

  • I have tested my changes locally and verified that they work as intended.
  • I have documented any new or modified functionality.
  • I have reviewed the changes to ensure they do not introduce any unnecessary complexity or duplicate code.
  • I understand that by submitting this pull request, I am agreeing to license my contributions under the project's license.

"The /MP option can reduce the total time to compile the source
files on the command line. The /MP option causes the compiler
to create one or more copies of itself, each in a separate process."

<https://learn.microsoft.com/en-us/cpp/build/reference/mp-build-with-multiple-processes?view=msvc-170>
@th1000s th1000s marked this pull request as ready for review April 28, 2024 23:15
Copy link
Contributor

@bryanperris bryanperris left a comment

Choose a reason for hiding this comment

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

CI build time for windows took 58 seconds rather than 1 and a half minutes.

@th1000s
Copy link
Contributor Author

th1000s commented Apr 28, 2024

Indeed, so:

  • I have tested my changes locally in the cloud and verified that they work as intended.

Is an option to disable this parallelism needed? Not sure if -D CMAKE_CXX_FLAGS="/MP 1" works (no Windows here, this is copied from $WORK). I often use ninja -j1 so the output is cleaner, but that doesn't require a full cmake reconfigure.

@bryanperris
Copy link
Contributor

Indeed, so:

  • I have tested my changes locally in the cloud and verified that they work as intended.

Is an option to disable this parallelism needed? Not sure if -D CMAKE_CXX_FLAGS="/MP 1" works (no Windows here, this is copied from $WORK). I often use ninja -j1 so the output is cleaner, but that doesn't require a full cmake reconfigure.

Lets wait for a Windows based developer to check over this.

@bryanperris
Copy link
Contributor

SaladBadger has approved the change

@bryanperris bryanperris merged commit 9d2aa8d into DescentDevelopers:main Apr 28, 2024
10 checks passed
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.

2 participants