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.

Sign up for GitHub

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

Jump to bottom

windows: gyp 0.5.0 made build time multiple times slower #35921

Closed
bzoz opened this issue Nov 2, 2020 · 3 comments
Closed

windows: gyp 0.5.0 made build time multiple times slower #35921

bzoz opened this issue Nov 2, 2020 · 3 comments

Comments

@bzoz
Copy link
Contributor

bzoz commented Nov 2, 2020

  • Version: master
  • Platform: Windows
  • Subsystem: build files

What steps will reproduce the bug?

GYP update in #32698 made the build time rise from 3minutes to 16minutes at least on the latest MSBuild 16.7.0+b89cb5fde.

Additional information

Looks like the GYP change changed the genrated .vcxproj files a bit, e.g. embedtest.vcxproj:

diff "fast\embedtest.vcxproj" "slow\embedtest.vcxproj"
137,139c137,145
<     <ClCompile Include="src\node_snapshot_stub.cc"/>
<     <ClCompile Include="src\node_code_cache_stub.cc"/>
<     <ClCompile Include="test\embedding\embedtest.cc"/>
---
>     <ClCompile Include="src\node_snapshot_stub.cc">
>       <ObjectFileName>$(IntDir)\src\node_snapshot_stub.obj</ObjectFileName>
>     </ClCompile>
>     <ClCompile Include="src\node_code_cache_stub.cc">
>       <ObjectFileName>$(IntDir)\src\node_code_cache_stub.obj</ObjectFileName>
>     </ClCompile>
>     <ClCompile Include="test\embedding\embedtest.cc">
>       <ObjectFileName>$(IntDir)\test\embedding\embedtest.obj</ObjectFileName>
>     </ClCompile>
142a149
>       <ObjectFileName>$(IntDir)\tools\msvs\pch\node_pch.obj</ObjectFileName>

Looks like the <ObjectFileName> tags make the build not run in parallel, thus dramatically increasing the build time.

/cc @nodejs/build-files @nodejs/platform-windows
@bzoz bzoz mentioned this issue Nov 2, 2020
Closed
@targos
Copy link
Member

targos commented Nov 2, 2020

I think this discussion is exactly about the issue: https://developercommunity.visualstudio.com/idea/586584/vs2017-cc-multi-processor-compilation-does-not-wor.html

It seems that we can improve it by only providing the directory name ending with a slash instead of the full file path: https://developercommunity.visualstudio.com/comments/586371/view.html
Another option is to try an experimental feature: https://developercommunity.visualstudio.com/comments/742133/view.html

I do not have the time to investigate these options.

Note that adding <ObjectFileName> was necessary to compile V8 because it has some files with duplicate names.

@bzoz
Copy link
Contributor Author

bzoz commented Nov 2, 2020

@targos the experimental fix works, I'll make a PR

bzoz added a commit to JaneaSystems/node that referenced this issue Nov 2, 2020
@bzoz
win, build: fix build time on Windows
c581197 
Sets MSBuild experimental switches to make it build in parallel project
files generated by gyp 0.5.0.

Fixes: nodejs#35921
@bzoz bzoz mentioned this issue Nov 2, 2020
Closed
2 tasks
@bzoz
Copy link
Contributor Author

bzoz commented Nov 2, 2020

PR in #35932

@Trott Trott closed this as completed in bfc9847 Nov 7, 2020
danielleadams pushed a commit that referenced this issue Nov 9, 2020
@bzoz @danielleadams
win, build: fix build time on Windows
7a2edea 
Sets MSBuild experimental switches to make it build in parallel project
files generated by gyp 0.5.0.

Fixes: #35921

PR-URL: #35932
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
BethGriggs pushed a commit that referenced this issue Dec 9, 2020
@bzoz @BethGriggs
win, build: fix build time on Windows
eb4cb46 
Sets MSBuild experimental switches to make it build in parallel project
files generated by gyp 0.5.0.

Fixes: #35921

PR-URL: #35932
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
BethGriggs pushed a commit that referenced this issue Dec 10, 2020
@bzoz @BethGriggs
win, build: fix build time on Windows
5227aba 
Sets MSBuild experimental switches to make it build in parallel project
files generated by gyp 0.5.0.

Fixes: #35921

PR-URL: #35932
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
BethGriggs pushed a commit that referenced this issue Dec 15, 2020
@bzoz @BethGriggs
win, build: fix build time on Windows
ffe517b 
Sets MSBuild experimental switches to make it build in parallel project
files generated by gyp 0.5.0.

Fixes: #35921

PR-URL: #35932
Reviewed-By: Anna Henningsen <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Richard Lau <[email protected]>
Reviewed-By: Jiawen Geng <[email protected]>
Reviewed-By: Zeyu Yang <[email protected]>
Reviewed-By: Gireesh Punathil <[email protected]>
Reviewed-By: Rich Trott <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

win, build: fix build time on Windows JaneaSystems/node
2 participants
@targos @bzoz