[JSC] Enable ThinLTO
Created attachment 435392 [details] Patch
Created attachment 435393 [details] Patch
Comment on attachment 435393 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=435393&action=review > Source/JavaScriptCore/ChangeLog:3 > + [JSC] Enable ThinLTO What is the build time impact? What I’m observing for the rest of WebKit is substantially slower than what was promised when we were enabling it, and we should consider disabling it everywhere :( If this adds minutes to build time, it’s likely not worth 0.3%.
Comment on attachment 435393 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=435393&action=review r=me > Source/JavaScriptCore/ChangeLog:14 > + We should enable ThinLTO for all WebKit components (only JSC is not using that). I think you meant to say that "We already enable ThinLTO for all WebKit components (only JSC is not using it). This patch now allows JSC to use ThinLTO too."
Comment on attachment 435393 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=435393&action=review >> Source/JavaScriptCore/ChangeLog:3 >> + [JSC] Enable ThinLTO > > What is the build time impact? What I’m observing for the rest of WebKit is substantially slower than what was promised when we were enabling it, and we should consider disabling it everywhere :( > > If this adds minutes to build time, it’s likely not worth 0.3%. No, we are really needing to have any sub-percent currently. 0.3% is *huge*. >> Source/JavaScriptCore/ChangeLog:14 >> + We should enable ThinLTO for all WebKit components (only JSC is not using that). > > I think you meant to say that "We already enable ThinLTO for all WebKit components (only JSC is not using it). This patch now allows JSC to use ThinLTO too." Added.
Comment on attachment 435393 [details] Patch r-, this is unacceptable to land when the trade off isn’t even measured.
I'm not going to comment on whether .3% is or isn't worth the trade-off in build time but I think it
(In reply to Keith Miller from comment #7) > I'm not going to comment on whether .3% is or isn't worth the trade-off in > build time but I think it Ugh, I hate how easy it is to press tab then enter and submit before finishing typing... Anyway, I'm not going to comment on whether .3% is or isn't worth the trade-off in build time but I think it might be worth considering having a non-lto release testing build flavor and a release/production build, which enables it. We already have this to some degree for JSC, where we have a release build with debug asserts enabled. Perhaps we should consider migrating to something like that more generally?
Comment on attachment 435393 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=435393&action=review >>> Source/JavaScriptCore/ChangeLog:3 >>> + [JSC] Enable ThinLTO >> >> What is the build time impact? What I’m observing for the rest of WebKit is substantially slower than what was promised when we were enabling it, and we should consider disabling it everywhere :( >> >> If this adds minutes to build time, it’s likely not worth 0.3%. > > No, we are really needing to have any sub-percent currently. 0.3% is *huge*. Turned out this only adds ~1 min to builder.
Comment on attachment 435393 [details] Patch This is still not ready for review, as discussed online.
<rdar://problem/82107543>
Created attachment 454122 [details] patch
Comment on attachment 454122 [details] patch r=me
Committed r290993 (248171@main): <https://commits.webkit.org/248171@main> All reviewed patches have been landed. Closing bug and clearing flags on attachment 454122 [details].