Repository navigation
Add /MP flag for MSVC multiprocessor builds, remove unity build. - #1644
Conversation
Enable parallel compilation with MSVC to speed up build times. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@CedricGuillemet I wonder if UNITY_BUILD being on is the reason why the build time is not decreasing in CI. Have you tried turning off UNITY_BUILD and turn on /MP? |
|
Another option to explore is the |
|
@bghgary I remember adding UNITY_BUILD for footprint issue with BRN on Windows. Now it's not a problem anymore, do you think we should remove it? I also want multicore with local builds with MSVC. So, the MSBuild flag is not enough. EDIT: build time change is not significant with UNITY_BUILD off and /mp on the CI. |
|
I think so. It's annoying sometimes since it doesn't work, so we have to turn it off for some targets. |
There was a problem hiding this comment.
Pull request overview
This PR aims to speed up Windows/MSVC builds by enabling MSVC multiprocessor compilation, but it also removes the existing “unity build” configuration plumbing across CI scripts, Gradle, and per-target CMake settings.
Changes:
- Add MSVC
/MPto the rootCMakeLists.txtcompile options. - Remove the
UNITY_BUILD/CMAKE_UNITY_BUILDCI and Android Gradle wiring. - Remove multiple per-target
UNITY_BUILD falseoverrides across CMake targets/dependencies.
Reviewed changes
Copilot reviewed 23 out of 23 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| CMakeLists.txt | Adds MSVC /MP flag. |
| azure-pipelines.yml | Removes UNITY_BUILD pipeline variable. |
| .github/jobs/win32.yml | Drops -D CMAKE_UNITY_BUILD=$(UNITY_BUILD) from Win32 configure step. |
| .github/jobs/win32_precompiled_shader_test.yml | Drops unity-build CMake define from configure step. |
| .github/jobs/uwp.yml | Drops unity-build CMake define from configure step. |
| .github/jobs/test_install_win32.yml | Drops unity-build CMake define from configure step. |
| .github/jobs/test_install_macos.yml | Drops unity-build CMake define from configure step. |
| .github/jobs/test_install_linux.yml | Drops unity-build CMake define from configure step. |
| .github/jobs/test_install_ios.yml | Drops unity-build CMake define from configure step. |
| .github/jobs/macos.yml | Drops unity-build CMake define from configure step. |
| .github/jobs/linux.yml | Drops unity-build CMake define from configure step. |
| .github/jobs/ios.yml | Drops unity-build CMake define from configure step. |
| Apps/Playground/Android/app/build.gradle | Removes unity-build property plumbing; minor formatting change. |
| Apps/Playground/Android/BabylonNative/build.gradle | Removes unity-build property plumbing and -DCMAKE_UNITY_BUILD arg; minor formatting change. |
| Plugins/TestUtils/CMakeLists.txt | Removes target-level UNITY_BUILD false override on Apple. |
| Plugins/NativeCamera/CMakeLists.txt | Removes target-level UNITY_BUILD false override on Apple. |
| Dependencies/xr/CMakeLists.txt | Removes target-level UNITY_BUILD false override on Apple. |
| Dependencies/CMakeLists.txt | Removes multiple target-level UNITY_BUILD false overrides (bgfx 3rdparty, JsRuntimeHost-related targets, libwebp sharpyuv, etc.). |
| Core/Graphics/CMakeLists.txt | Removes target-level UNITY_BUILD false override. |
| Apps/UnitTests/CMakeLists.txt | Removes target-level UNITY_BUILD false override. |
| Apps/PrecompiledShaderTest/CMakeLists.txt | Removes target-level UNITY_BUILD false overrides. |
| Apps/Playground/CMakeLists.txt | Removes target-level UNITY_BUILD false override on Apple. |
| Apps/HeadlessScreenshotApp/CMakeLists.txt | Removes target-level UNITY_BUILD false override. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Update BabylonNative commit reference to pick up the fix for null dereference on error in iOS CameraDevice (BabylonJS/BabylonNative#1649). The error variable passed to newRenderPipelineStateWithDescriptor may be nil even when the call fails. The new commit guards against dereferencing nil when building the error message string. Also includes: - MSVC /MP multiprocessor build flag (BabylonJS/BabylonNative#1644) - bgfx update for x86 crash fix (BabylonJS/BabylonNative#1643) - Crash dump collection on all CI platforms (BabylonJS/BabylonNative#1641) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Update BabylonNative commit reference to pick up the fix for null dereference on error in iOS CameraDevice (BabylonJS/BabylonNative#1649). The error variable passed to newRenderPipelineStateWithDescriptor may be nil even when the call fails. The new commit guards against dereferencing nil when building the error message string. Also includes: - MSVC /MP multiprocessor build flag (BabylonJS/BabylonNative#1644) - bgfx update for x86 crash fix (BabylonJS/BabylonNative#1643) - Crash dump collection on all CI platforms (BabylonJS/BabylonNative#1641) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Description
Adds the MSVC
/MP(multiprocessor compilation) flag to the root CMakeLists.txt, enabling parallel compilation of source files when building with MSVC.Remove Unity build as not mandatory for BRN Windows prebuilt binaries.
Changes
add_compile_options(/MP)in the existing MSVC compiler-flags block in the rootCMakeLists.txt.Note: build time decrease is not that significant on the CI. Should be better with local builds.
local test PG debug full rebuild.
no MP :
with MP: