-
-
Notifications
You must be signed in to change notification settings - Fork 32.5k
Closed
Labels
buildIssues and PRs related to build files or the CI.Issues and PRs related to build files or the CI.windowsIssues and PRs related to the Windows platform.Issues and PRs related to the Windows platform.
Description
Description
Chrome and Firefox are built with Clang under Windows.
@targos made it possible to build Node.js using clang under Windows, see #35433
At least in some tests, this seems to result in better performance:
confidence improvement accuracy (*) (**) (***)
v8\get-stats.js n=1000000 method='getHeapSpaceStatistics' 3.36 % ±10.99% ±14.62% ±19.03%
v8\get-stats.js n=1000000 method='getHeapStatistics' 10.35 % ±10.90% ±14.50% ±18.87%
v8\serialize.js n=1000000 len=16384 1.97 % ±2.21% ±3.14% ±4.53%
v8\serialize.js n=1000000 len=256 *** 5.59 % ±1.61% ±2.30% ±3.36%
v8\serialize.js n=1000000 len=524288 19.28 % ±48.92% ±76.74% ±130.72%
It is only preliminary, but it is not very difficult to conduct a full examination. There are some maintenance benefits to switching to LLVM under Windows as well. I stress that Node works right now under Windows when compiled with LLVM (credit to @targos).
There is at least one minor issue with ICU: #34201 A tiny patch might be required. I actually expect that it might be the only problem.
I am opening this issue following a request by @mcollina
Further reading: Should Node.js be built with ClangCL under Windows?
Checklist:
- Add a configuration option in
vcbuild.bat
to pick from MSVC and CLangCL - Landed in build: add option to enable clang-cl on Windows #52870 - harmonize Clang checks and update V8 warning flags - Landed in build: harmonize Clang checks and update V8 warning cflags #52873
- Upstream changes in
gyp-next
to enable setting language standard keys in msvs_settings - Landed upstream in feat: support language standard keys in msvs_settings gyp-next#252, will eventually get updated in the main project - Install ClangCL on Windows hosts in CI - Landed in ansible: add ClangCL to VS2022 build#3714
- Upstream fix for ICU issue for cross-compilation to ARM64 - @StefanStojanovic is looking into this, ideally, we'll fix it upstream and pull it to Node.js as a dependency. PR awaiting one more reviewer ICU-22787 Fix ClangCL compilation on Windows unicode-org/icu#3023
- Upstream fix for V8 ARM64 cross-compilation - Landed upstream in FP16 Fix FP16 bitcasts for ClangCL on ARM/ARM64 Maratyszcza/FP16#26 and pulled in V8 in https://chromium-review.googlesource.com/c/v8/v8/+/5553996, will eventually get updated in the main project
- Implement logic for detecting installed ClangCL version - This is currently hardcoded to the version that can be installed via Visual Studio as a component. @StefanStojanovic has discussed this with @huseyinacacak-janea and they are currently investigating it.
- Configure Windows CI machines to be able to compile and test ClangCL - This is done, there were 2 build repo PRS: ansible: add ClangCL to VS2022 build#3714 and ansible: add ClangCL to VS2022 fix build#3786
- Configure Windows CI jobs to build and test ClangCL - @StefanStojanovic will work on this once most other things are done so we can start gaining confidence for ClangCL builds
- Run benchmarks to ensure performances are not worsened by moving to ClangCL from MSVC - This is self-explanatory.
- Make sure that the Node.js ecosystem is not facing issues because of the migration - Make sure node-gyp, popular npm packages, etc. are all still working well
- Investigate the compilation times between MSVC and ClangCL - This is done now. With ccache, once initial compilation is finished, thus filling the cache, ClangCL compiles in around 15-20 minutes, which is noticeable improvement from MSVC.
-
If there are no reasons not to, switch to using ClangCL as a default compiler for future releases, the same can also be said for dropping x86 support based on Should we drop support for 32 bit Windows? #42543 (comment)V8 has already dropped support for MSVC and will not accept MSVC-compatibility patches since 13.0, so we'll need to switch to using ClangCL for future releases that use V8 > 13.0
aminya
Metadata
Metadata
Assignees
Labels
buildIssues and PRs related to build files or the CI.Issues and PRs related to build files or the CI.windowsIssues and PRs related to the Windows platform.Issues and PRs related to the Windows platform.