-
Notifications
You must be signed in to change notification settings - Fork 14.7k
[cmake] Don't generate per-file "__SHORT_FILE__" defines on visual studio builds #151167
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
Conversation
endif() | ||
endforeach() | ||
|
||
# Don't generate __SHORT_FILE__ on MSVC builds as it can force repeated cache regeneration. |
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.
Can you provide more info on the "repeated cache generation" aspect? You mentioned before that the build would come to a halt which seems a bit different than a cache problem?
At least link to the discussion on the original PR maybe?
It's not clear to me that someone reading this right now would even connect the dots to MSBuild-specific issue.
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.
I don't know enough about how cmake does its thing on visual studio - is it possible it gets run multiple times or something during the build? All I know is #150677 slowed everything to a crawl.
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.
I mean you refer to a "cache", which cache? In general I think of "ccache" when I read this. If you mean that this triggers repeated cmake invocation, then why not write it as is?
You likely should be able to see which process is churning why things seem slow? If cmake is re-running that should be visible, and there should be no compilation job running concurrently?
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.
The update message may be good enough for now, I hope this isn't setting up a Chesterton's Fence though!
The commit summary is a bit confusing - it doesn't impact MSVC builds, it impacts the VS generator. |
Yes, it demonstrates my lack of understanding of what actually goes on in cmake vs builds - all I can see is that there is at most 2 cores being used for cl.exe and cmake.exe keeps reappearing as well. |
As reported on llvm#150677 - this prevents build parallelisation as cmake is repeatedly updating the cache
404e6b3
to
a7c24e3
Compare
…udio builds (llvm#151167) As reported on llvm#150677 - this is preventing msbuild parallelization as cmake is repeatedly being updated
As reported on #150677 - this is preventing msbuild parallelization as cmake is repeatedly being updated