Generated .cpp files for bindings layer should be compiled independently |
||||||
Issue descriptionI found the compilation of V8GeneratedCoreBindings*.cpp takes long time. They takes around 180 seconds each, and one of them takes 440 seconds on Windows. Can we break them into pieces for faster build, or do some better handling to these generated code?
,
Aug 4 2016
Those V8GeneratedCoreBindings*.cpp files include other V8X.cpp generated from X.idl files. I think we can specify V8X.cpp and V8X.h as source in .gn/.gni files, each compilation can be run in parallel. And as another concern of this issue is to run needless compilations. Assume V8GeneratedCoreBindings0.cpp includes V8A.cpp and V8B.cpp, and we update A.idl. Then we don't have to compile V8B.cpp, but do it through V8GeneratedCoreBindings0.cpp. So if we can split compilations, we may skip some compilations.
,
Aug 4 2016
+1 to stop aggregating generated file. Actually, when I took over IDL stuff from nbarth@ he suggested to stop aggregating. (I'll share you a doc nbarth@ wrote) You don't need to worry about recompilation as the code generator won't touch files if there is no diff. See write_file() in utilities.py
,
Aug 4 2016
Hmm, we need to resolve another issue to work for this issue. Generated files may not include enough files to be compiled. For example, V8Location.cpp, generated from interface_base.cpp, uses toLocalWindow() function that is defined in core/frame/LocalDOMWindow.h, but it does not include the header file.
,
Aug 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/2a213fd43d3915932361089a18ffdca4390775d4 commit 2a213fd43d3915932361089a18ffdca4390775d4 Author: peria <peria@chromium.org> Date: Thu Aug 04 14:38:19 2016 Put a prototype class of DOMException. We have no declaration of DOMException in header files recursively included from ErrorCallback.h. Compiles passed well by chance. BUG= 634231 Review-Url: https://codereview.chromium.org/2217453002 Cr-Commit-Position: refs/heads/master@{#409782} [modify] https://crrev.com/2a213fd43d3915932361089a18ffdca4390775d4/third_party/WebKit/Source/modules/filesystem/ErrorCallback.h
,
Aug 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/eb4c3ca5ce05cb31f699401813f0664ee70fda19 commit eb4c3ca5ce05cb31f699401813f0664ee70fda19 Author: peria <peria@chromium.org> Date: Thu Aug 04 15:02:30 2016 Fix compile errors ignored by binding aggregations In binding layer, we compile many .cpp file at once, aggregated into few .cpp files. This aggregation hides some lacks of dependency, and it strongly dependent on current aggregation style. (For example, crrev.com/2216563002 makes it obvious) This CL fixes those hidden compile errors in generated code. BUG= 634231 Review-Url: https://codereview.chromium.org/2210973002 Cr-Commit-Position: refs/heads/master@{#409785} [modify] https://crrev.com/eb4c3ca5ce05cb31f699401813f0664ee70fda19/third_party/WebKit/Source/bindings/scripts/v8_interface.py [modify] https://crrev.com/eb4c3ca5ce05cb31f699401813f0664ee70fda19/third_party/WebKit/Source/bindings/tests/results/core/V8TestInterfaceCheckSecurity.cpp [modify] https://crrev.com/eb4c3ca5ce05cb31f699401813f0664ee70fda19/third_party/WebKit/Source/core/html/HTMLFrameSetElement.h
,
Aug 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a77ec79a00851a95760c94c0fe9daa02ce44ab6c commit a77ec79a00851a95760c94c0fe9daa02ce44ab6c Author: peria <peria@chromium.org> Date: Thu Aug 04 16:44:15 2016 Increase the number of files to aggregate generated .cpp files from .idl files. (This change is a workaround change.) It is expected to reduce compile time because of following reasons. - The number of lines to be processed in one compile reduced. - The number of compile tasks increased, so they can be run in parallel. BUG= 634231 Review-Url: https://codereview.chromium.org/2216563002 Cr-Commit-Position: refs/heads/master@{#409809} [modify] https://crrev.com/a77ec79a00851a95760c94c0fe9daa02ce44ab6c/third_party/WebKit/Source/bindings/core/v8/generated.gni [modify] https://crrev.com/a77ec79a00851a95760c94c0fe9daa02ce44ab6c/third_party/WebKit/Source/bindings/core/v8/generated.gypi [modify] https://crrev.com/a77ec79a00851a95760c94c0fe9daa02ce44ab6c/third_party/WebKit/Source/bindings/modules/v8/generated.gni [modify] https://crrev.com/a77ec79a00851a95760c94c0fe9daa02ce44ab6c/third_party/WebKit/Source/bindings/modules/v8/generated.gypi
,
Aug 17 2016
There is another concern in aggregating .cpp files; if we use anonymous namespace in generated .cpp files, names of variables or functions defined in the anonymous namespace can conflict.
,
Aug 17 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/76aad2dc24a03efe1f0a2bcbcc4dbe806b911217 commit 76aad2dc24a03efe1f0a2bcbcc4dbe806b911217 Author: peria <peria@chromium.org> Date: Wed Aug 17 15:00:46 2016 Add option parser in aggregate_generated_bindings.py Its arguments list does not depend on the order and the index of '--'. This change enables us to update the .py file step by step. BUG= 634231 Review-Url: https://codereview.chromium.org/2248363002 Cr-Commit-Position: refs/heads/master@{#412530} [modify] https://crrev.com/76aad2dc24a03efe1f0a2bcbcc4dbe806b911217/third_party/WebKit/Source/bindings/core/v8/generated.gyp [modify] https://crrev.com/76aad2dc24a03efe1f0a2bcbcc4dbe806b911217/third_party/WebKit/Source/bindings/modules/v8/generated.gyp [modify] https://crrev.com/76aad2dc24a03efe1f0a2bcbcc4dbe806b911217/third_party/WebKit/Source/bindings/scripts/aggregate_generated_bindings.py [modify] https://crrev.com/76aad2dc24a03efe1f0a2bcbcc4dbe806b911217/third_party/WebKit/Source/bindings/scripts/scripts.gni
,
Aug 18 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/df0a0c5106ff224578092178373faca20bca2b02 commit df0a0c5106ff224578092178373faca20bca2b02 Author: peria <peria@chromium.org> Date: Thu Aug 18 06:21:59 2016 Specify if we aggregate partial interfaces by an argument. Before this CL, file names to include were decided with the path of original IDL files. This behavior could mislead us as if we included generated files for partial interface and non-partial interface in one aggregation file, although we aggregate files for all partial interfaces into one file. This change make it obvious in .gn/.gyp files to aggregate files for (non-)partial interfaces. BUG= 634231 Review-Url: https://codereview.chromium.org/2259463002 Cr-Commit-Position: refs/heads/master@{#412756} [modify] https://crrev.com/df0a0c5106ff224578092178373faca20bca2b02/third_party/WebKit/Source/bindings/core/v8/BUILD.gn [modify] https://crrev.com/df0a0c5106ff224578092178373faca20bca2b02/third_party/WebKit/Source/bindings/modules/v8/BUILD.gn [modify] https://crrev.com/df0a0c5106ff224578092178373faca20bca2b02/third_party/WebKit/Source/bindings/modules/v8/generated.gyp [modify] https://crrev.com/df0a0c5106ff224578092178373faca20bca2b02/third_party/WebKit/Source/bindings/scripts/aggregate_generated_bindings.py [modify] https://crrev.com/df0a0c5106ff224578092178373faca20bca2b02/third_party/WebKit/Source/bindings/scripts/scripts.gni
,
Sep 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/134a59bd48149d721ce06554570db1b2b5bdee55 commit 134a59bd48149d721ce06554570db1b2b5bdee55 Author: peria <peria@chromium.org> Date: Thu Sep 08 07:14:41 2016 Remove aggregation of generated binding code. After this change, we maybe able to skip needless compiles based on redundant dependencies. ( issue 388172 ) Plus, it is expected to compile those files in parallel. BUG= 634231 , 388172 Review-Url: https://codereview.chromium.org/2318933002 Cr-Commit-Position: refs/heads/master@{#417216} [modify] https://crrev.com/134a59bd48149d721ce06554570db1b2b5bdee55/third_party/WebKit/Source/bindings/core/v8/BUILD.gn [modify] https://crrev.com/134a59bd48149d721ce06554570db1b2b5bdee55/third_party/WebKit/Source/bindings/modules/v8/BUILD.gn [modify] https://crrev.com/134a59bd48149d721ce06554570db1b2b5bdee55/third_party/WebKit/Source/bindings/modules/v8/generated.gni [delete] https://crrev.com/9fd8459b286577c2c4fa8becfae92101765bf2f1/third_party/WebKit/Source/bindings/scripts/aggregate_generated_bindings.py [modify] https://crrev.com/134a59bd48149d721ce06554570db1b2b5bdee55/third_party/WebKit/Source/bindings/scripts/generate_init_partial_interfaces.py [modify] https://crrev.com/134a59bd48149d721ce06554570db1b2b5bdee55/third_party/WebKit/Source/bindings/scripts/scripts.gni [modify] https://crrev.com/134a59bd48149d721ce06554570db1b2b5bdee55/third_party/WebKit/Source/modules/BUILD.gn
,
Sep 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d7586a77bd18573597cc48bbd2e0e6fb0fbc0fb9 commit d7586a77bd18573597cc48bbd2e0e6fb0fbc0fb9 Author: peria <peria@chromium.org> Date: Thu Sep 08 08:31:15 2016 Revert of [Bindings] Remove aggregation of generated binding code (patchset #3 id:60001 of https://codereview.chromium.org/2318933002/ ) Reason for revert: Build break http://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Win/builds/10652 Original issue's description: > Remove aggregation of generated binding code. > > After this change, we maybe able to skip needless compiles > based on redundant dependencies. ( issue 388172 ) > Plus, it is expected to compile those files in parallel. > > BUG= 634231 , 388172 > > Committed: https://crrev.com/134a59bd48149d721ce06554570db1b2b5bdee55 > Cr-Commit-Position: refs/heads/master@{#417216} TBR=bashi@chromium.org,yukishiino@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 634231 , 388172 Review-Url: https://codereview.chromium.org/2325583002 Cr-Commit-Position: refs/heads/master@{#417225} [modify] https://crrev.com/d7586a77bd18573597cc48bbd2e0e6fb0fbc0fb9/third_party/WebKit/Source/bindings/core/v8/BUILD.gn [modify] https://crrev.com/d7586a77bd18573597cc48bbd2e0e6fb0fbc0fb9/third_party/WebKit/Source/bindings/modules/v8/BUILD.gn [modify] https://crrev.com/d7586a77bd18573597cc48bbd2e0e6fb0fbc0fb9/third_party/WebKit/Source/bindings/modules/v8/generated.gni [add] https://crrev.com/d7586a77bd18573597cc48bbd2e0e6fb0fbc0fb9/third_party/WebKit/Source/bindings/scripts/aggregate_generated_bindings.py [modify] https://crrev.com/d7586a77bd18573597cc48bbd2e0e6fb0fbc0fb9/third_party/WebKit/Source/bindings/scripts/generate_init_partial_interfaces.py [modify] https://crrev.com/d7586a77bd18573597cc48bbd2e0e6fb0fbc0fb9/third_party/WebKit/Source/bindings/scripts/scripts.gni [modify] https://crrev.com/d7586a77bd18573597cc48bbd2e0e6fb0fbc0fb9/third_party/WebKit/Source/modules/BUILD.gn
,
Sep 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0f82e43808017ab95d877be86eb63fe1e4057e97 commit 0f82e43808017ab95d877be86eb63fe1e4057e97 Author: peria <peria@chromium.org> Date: Thu Sep 08 08:32:23 2016 Revert of [Bindings] Remove aggregation of generated binding code (patchset #3 id:60001 of https://codereview.chromium.org/2318933002/ ) Reason for revert: Build break http://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Win/builds/10652 Original issue's description: > Remove aggregation of generated binding code. > > After this change, we maybe able to skip needless compiles > based on redundant dependencies. ( issue 388172 ) > Plus, it is expected to compile those files in parallel. > > BUG= 634231 , 388172 > > Committed: https://crrev.com/134a59bd48149d721ce06554570db1b2b5bdee55 > Cr-Commit-Position: refs/heads/master@{#417216} TBR=bashi@chromium.org,yukishiino@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 634231 , 388172 Review-Url: https://codereview.chromium.org/2325583002 Cr-Commit-Position: refs/heads/master@{#417226} [modify] https://crrev.com/0f82e43808017ab95d877be86eb63fe1e4057e97/third_party/WebKit/Source/bindings/scripts/scripts.gni
,
Sep 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/134a59bd48149d721ce06554570db1b2b5bdee55 commit 134a59bd48149d721ce06554570db1b2b5bdee55 Author: peria <peria@chromium.org> Date: Thu Sep 08 07:14:41 2016 Remove aggregation of generated binding code. After this change, we maybe able to skip needless compiles based on redundant dependencies. ( issue 388172 ) Plus, it is expected to compile those files in parallel. BUG= 634231 , 388172 Review-Url: https://codereview.chromium.org/2318933002 Cr-Commit-Position: refs/heads/master@{#417216} [modify] https://crrev.com/134a59bd48149d721ce06554570db1b2b5bdee55/third_party/WebKit/Source/bindings/core/v8/BUILD.gn [modify] https://crrev.com/134a59bd48149d721ce06554570db1b2b5bdee55/third_party/WebKit/Source/bindings/modules/v8/BUILD.gn [modify] https://crrev.com/134a59bd48149d721ce06554570db1b2b5bdee55/third_party/WebKit/Source/bindings/modules/v8/generated.gni [delete] https://crrev.com/9fd8459b286577c2c4fa8becfae92101765bf2f1/third_party/WebKit/Source/bindings/scripts/aggregate_generated_bindings.py [modify] https://crrev.com/134a59bd48149d721ce06554570db1b2b5bdee55/third_party/WebKit/Source/bindings/scripts/generate_init_partial_interfaces.py [modify] https://crrev.com/134a59bd48149d721ce06554570db1b2b5bdee55/third_party/WebKit/Source/bindings/scripts/scripts.gni [modify] https://crrev.com/134a59bd48149d721ce06554570db1b2b5bdee55/third_party/WebKit/Source/modules/BUILD.gn
,
Sep 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/d7586a77bd18573597cc48bbd2e0e6fb0fbc0fb9 commit d7586a77bd18573597cc48bbd2e0e6fb0fbc0fb9 Author: peria <peria@chromium.org> Date: Thu Sep 08 08:31:15 2016 Revert of [Bindings] Remove aggregation of generated binding code (patchset #3 id:60001 of https://codereview.chromium.org/2318933002/ ) Reason for revert: Build break http://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Win/builds/10652 Original issue's description: > Remove aggregation of generated binding code. > > After this change, we maybe able to skip needless compiles > based on redundant dependencies. ( issue 388172 ) > Plus, it is expected to compile those files in parallel. > > BUG= 634231 , 388172 > > Committed: https://crrev.com/134a59bd48149d721ce06554570db1b2b5bdee55 > Cr-Commit-Position: refs/heads/master@{#417216} TBR=bashi@chromium.org,yukishiino@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 634231 , 388172 Review-Url: https://codereview.chromium.org/2325583002 Cr-Commit-Position: refs/heads/master@{#417225} [modify] https://crrev.com/d7586a77bd18573597cc48bbd2e0e6fb0fbc0fb9/third_party/WebKit/Source/bindings/core/v8/BUILD.gn [modify] https://crrev.com/d7586a77bd18573597cc48bbd2e0e6fb0fbc0fb9/third_party/WebKit/Source/bindings/modules/v8/BUILD.gn [modify] https://crrev.com/d7586a77bd18573597cc48bbd2e0e6fb0fbc0fb9/third_party/WebKit/Source/bindings/modules/v8/generated.gni [add] https://crrev.com/d7586a77bd18573597cc48bbd2e0e6fb0fbc0fb9/third_party/WebKit/Source/bindings/scripts/aggregate_generated_bindings.py [modify] https://crrev.com/d7586a77bd18573597cc48bbd2e0e6fb0fbc0fb9/third_party/WebKit/Source/bindings/scripts/generate_init_partial_interfaces.py [modify] https://crrev.com/d7586a77bd18573597cc48bbd2e0e6fb0fbc0fb9/third_party/WebKit/Source/bindings/scripts/scripts.gni [modify] https://crrev.com/d7586a77bd18573597cc48bbd2e0e6fb0fbc0fb9/third_party/WebKit/Source/modules/BUILD.gn
,
Sep 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/0f82e43808017ab95d877be86eb63fe1e4057e97 commit 0f82e43808017ab95d877be86eb63fe1e4057e97 Author: peria <peria@chromium.org> Date: Thu Sep 08 08:32:23 2016 Revert of [Bindings] Remove aggregation of generated binding code (patchset #3 id:60001 of https://codereview.chromium.org/2318933002/ ) Reason for revert: Build break http://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Win/builds/10652 Original issue's description: > Remove aggregation of generated binding code. > > After this change, we maybe able to skip needless compiles > based on redundant dependencies. ( issue 388172 ) > Plus, it is expected to compile those files in parallel. > > BUG= 634231 , 388172 > > Committed: https://crrev.com/134a59bd48149d721ce06554570db1b2b5bdee55 > Cr-Commit-Position: refs/heads/master@{#417216} TBR=bashi@chromium.org,yukishiino@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG= 634231 , 388172 Review-Url: https://codereview.chromium.org/2325583002 Cr-Commit-Position: refs/heads/master@{#417226} [modify] https://crrev.com/0f82e43808017ab95d877be86eb63fe1e4057e97/third_party/WebKit/Source/bindings/scripts/scripts.gni
,
Sep 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/aff7c7109dd3651121f94ceea720464844c86ec3 commit aff7c7109dd3651121f94ceea720464844c86ec3 Author: peria <peria@chromium.org> Date: Mon Sep 12 14:35:22 2016 Remove aggregation of generated binding code. After this change, we maybe able to skip needless compiles based on redundant dependencies. ( issue 388172 ) Plus, it is expected to compile those files in parallel. This CL is same with http://crrev.com/2318933002/ with a fix. The fix is to aggregate them only on Windows Official build, where we need to preserve symbol table. BUG= 634231 , 388172 Review-Url: https://codereview.chromium.org/2325973002 Cr-Commit-Position: refs/heads/master@{#417935} [modify] https://crrev.com/aff7c7109dd3651121f94ceea720464844c86ec3/third_party/WebKit/Source/bindings/core/v8/BUILD.gn [modify] https://crrev.com/aff7c7109dd3651121f94ceea720464844c86ec3/third_party/WebKit/Source/bindings/modules/v8/BUILD.gn [modify] https://crrev.com/aff7c7109dd3651121f94ceea720464844c86ec3/third_party/WebKit/Source/bindings/modules/v8/generated.gni [modify] https://crrev.com/aff7c7109dd3651121f94ceea720464844c86ec3/third_party/WebKit/Source/bindings/scripts/aggregate_generated_bindings.py [modify] https://crrev.com/aff7c7109dd3651121f94ceea720464844c86ec3/third_party/WebKit/Source/bindings/scripts/generate_init_partial_interfaces.py [modify] https://crrev.com/aff7c7109dd3651121f94ceea720464844c86ec3/third_party/WebKit/Source/bindings/scripts/scripts.gni [modify] https://crrev.com/aff7c7109dd3651121f94ceea720464844c86ec3/third_party/WebKit/Source/modules/BUILD.gn
,
Sep 15 2016
Static build on Windows, we need an aggregation to minimize image data. [LNK1248] We've done for other platforms, so let me close this issue as fixed.
,
Mar 27 2017
It is a bit ironic, considering that bug 388172 is about reducing compilation times, that this change might have achieved the opposite. Building in "windows" mode my test compilations of bindings need 108 seconds real (wall clock) time. 381 seconds CPU time. Building in "linux/mac" mode with many small files it needs 379 seconds real (wall clock) time. 2796 seconds CPU time. I looked at it because it's the most time consuming folder during compilation and I was investigating the potential gains from unity builds.
,
Mar 27 2017
s/2796/2874/ CPU seconds. I forgot to add kernel time to that one. So the split up build uses about 8 times more CPU time it would seem. For someone with *a lot* of parallel cores it might still be faster or about as fast. For people with more ordinary computers it becomes quite a lot of minutes slower.
,
Mar 27 2017
Re; #19,20 Yes, it can happen. I'm not sure which one is the critical one, but I think up some reasons. One is parallel I/O on disk. It depends on the number of parallelized process and HDD/SSD you use. One is #include's. In Windows style, all .cpp files are compiled as a one .cpp file, so it ignores many duplicated .h files, while all .h files are included in linux/Mac OSX style. But anyway, we are not working to fix the issue for now, because (1) if the number of processes can resolve it, now we don't have a way to control the number in GN, (2) most Chormium developers are in Google and using Goma (distributed compiling system), and this is not problematic for them, and (3) the windows style has a potential problems. I already fixed some of them, but it is impossible not to introduce new and same type problems.
,
Mar 27 2017
Yes, this is really tricky. Optimizing the build for scenario with a 1000 (virtual) parallel cores is clearly the right thing for people with that kind of (virtual) hardware, but on the other hand there are also quite a lot of people outside that would like to contribute or work at companies that build products based on Chromium and those are discouraged by the hardware requirements. Right now, in this code, you basically have two "if (is_win)" statements. I guess that is where you were thinking of "if (is_win or virtual_cpu_count < 10)". But I'm just investigating and experimenting now. It looks like it's possible to make Chrome build in a tenth of the time this way, counted in CPU cycles, which makes it a tempting to keep investigating.
,
Mar 27 2017
For goma users, their virtual CPUs can be < 10, because goma does not provide internal virtual CPUs, and whether goma works or not can change after GN works. How about adding a new GN flag, if it is acceptable for you? Of cause in that case, you need an action to specify the flag to switch to Windows style compile. (no automated routine works to switch) What do you think?
,
Mar 28 2017
I think a global flag is one option that is worth pursuing but I'm looking at more code than just this so maybe hold off until there are some kind of agreement of how to configure it? One option is to build it into the gn binary itself, so that it will group files if you ask it to do so. May be a bit too magic when everything happens automatically. Listing it by hand is much less magic but requires changes in many more places.
,
Dec 21 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/6bdfe3b98a5c84fb9cdd1eeccccdce05f16caba8 commit 6bdfe3b98a5c84fb9cdd1eeccccdce05f16caba8 Author: Takuto Ikuta <tikuta@chromium.org> Date: Thu Dec 21 02:27:26 2017 Use V8GeneratedCoreBindings only in jumbo build This is retry of https://codereview.chromium.org/2318933002/ Current chromium/src can build all with is_official_build=true in windows. This improves build parallelism on windows. Bug: 634231 Change-Id: Ie16541b56d90718ab01a992b7f4cbc07caebcd22 Reviewed-on: https://chromium-review.googlesource.com/836627 Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Kenichi Ishibashi <bashi@chromium.org> Commit-Queue: Takuto Ikuta <tikuta@chromium.org> Cr-Commit-Position: refs/heads/master@{#525565} [modify] https://crrev.com/6bdfe3b98a5c84fb9cdd1eeccccdce05f16caba8/third_party/WebKit/Source/bindings/core/v8/BUILD.gn
,
Dec 21 2017
> This improves build parallelism on windows. Did you check if it was also faster? Last I checked it was 22 times more work compiling the files one by one so even if higher parallelism keeps many cores active, it might still be slower.
,
Dec 21 2017
+tikuta so that they can see my question.
,
Dec 21 2017
In my 48 thread windows 10 machine, my CL improved build time of third_party/WebKit/Source/bindings/core/v8:bindings_core_impl from 330 seconds to 171 seconds when I use goma to specify -j1000. Compiling V8GeneratedCoreBindings.cpp takes more than 200 seconds in my build. Small task with high parallelism is very important in goma build.
,
Dec 21 2017
Indeed. Maybe it should be controlled by whether you have goma or not. Does it make things faster without goma as well (if you have a 48 thread machine).
,
Dec 21 2017
I measured the build time on the same machine, and it increased from 848 seconds to 1023 seconds. But if you want to build faster, you can do it by enabling jumbo build yet. And I prefer jumbo config controls such a thing.
,
Dec 22 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/42a666ceeee18625040f708ae3084f1e43a2f6e9 commit 42a666ceeee18625040f708ae3084f1e43a2f6e9 Author: Takuto Ikuta <tikuta@google.com> Date: Fri Dec 22 10:05:16 2017 Revert "Use V8GeneratedCoreBindings only in jumbo build" This reverts commit 6bdfe3b98a5c84fb9cdd1eeccccdce05f16caba8. Reason for revert: Broke official win asan build crbug.com/797202 FAILED: obj/third_party/WebKit/Source/bindings/core/v8/bindings_core_impl.lib C:/b/depot_tools/win_tools-2_7_6_bin/python/bin/python.exe ../../build/toolchain/win/tool_wrapper.py link-wrapper environment.x86 False lib.exe /nologo /ignore:4221 /OUT:obj/third_party/WebKit/Source/bindings/core/v8/bindings_core_impl.lib @obj/third_party/WebKit/Source/bindings/core/v8/bindings_core_impl.lib.rsp LINK : MSIL .netmodule or module compiled with /GL found; restarting link with /LTCG; add /LTCG to the link command line to improve linker performance obj/third_party/WebKit/Source/bindings/core/v8/bindings_core_impl.lib : fatal error LNK1248: image size (1001D34E4) exceeds maximum allowable size (FFFFFFFF) Original change's description: > Use V8GeneratedCoreBindings only in jumbo build > > This is retry of > https://codereview.chromium.org/2318933002/ > > Current chromium/src can build all with is_official_build=true in windows. > This improves build parallelism on windows. > > Bug: 634231 > Change-Id: Ie16541b56d90718ab01a992b7f4cbc07caebcd22 > Reviewed-on: https://chromium-review.googlesource.com/836627 > Reviewed-by: Kentaro Hara <haraken@chromium.org> > Reviewed-by: Kenichi Ishibashi <bashi@chromium.org> > Commit-Queue: Takuto Ikuta <tikuta@chromium.org> > Cr-Commit-Position: refs/heads/master@{#525565} TBR=peria@chromium.org,bashi@chromium.org,haraken@chromium.org,tikuta@google.com,tikuta@chromium.org # Not skipping CQ checks because original CL landed > 1 day ago. Bug: 634231 , 797202 Change-Id: I86c3a554a5121ab94c7950a97de7b9914b0a7b95 Reviewed-on: https://chromium-review.googlesource.com/842564 Commit-Queue: Shinya Kawanaka <shinyak@chromium.org> Reviewed-by: Shinya Kawanaka <shinyak@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Cr-Commit-Position: refs/heads/master@{#525966} [modify] https://crrev.com/42a666ceeee18625040f708ae3084f1e43a2f6e9/third_party/WebKit/Source/bindings/core/v8/BUILD.gn
,
Dec 25 2017
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/dee25c1bd32025929f350ae4cb23039bbb2c4c11 commit dee25c1bd32025929f350ae4cb23039bbb2c4c11 Author: Takuto Ikuta <tikuta@chromium.org> Date: Mon Dec 25 11:22:03 2017 Do not use V8GeneratedCoreBindings in non-offical win build This is retry of https://chromium-review.googlesource.com/836627 Note that following builder switched to clang-cl and /GL is not added now. That made me thinking the reverted CL is safe to land. https://ci.chromium.org/buildbot/chromium.chrome/Google%20Chrome%20Win/ To improve build speed when using goma, let me stop using amalgamation on non-official win build. This CL won't effect builder causing revert of previous CL. In my 48 thread windows 10 machine, my CL improved build time of third_party/WebKit/Source/bindings/core/v8:bindings_core_impl from 330 seconds to 171 seconds when I use goma to specify -j1000 without backend cache. Bug: 634231 Change-Id: I212c50d4d0d5d2a6399004be877665a9c95de838 Reviewed-on: https://chromium-review.googlesource.com/843996 Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Kenichi Ishibashi <bashi@chromium.org> Commit-Queue: Takuto Ikuta <tikuta@chromium.org> Cr-Commit-Position: refs/heads/master@{#526172} [modify] https://crrev.com/dee25c1bd32025929f350ae4cb23039bbb2c4c11/third_party/WebKit/Source/bindings/core/v8/BUILD.gn
,
Sep 17
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/5b6fe95874503156d33bf99b8df3e2abbff23554 commit 5b6fe95874503156d33bf99b8df3e2abbff23554 Author: David Michael Barr <david.barr@samsung.com> Date: Mon Sep 17 08:58:24 2018 Do not use v8_generated_core_bindings in jumbo build For low values of jumbo_file_merge_limit, this removes a "long pole". The rationale is similar to https://chromium-review.googlesource.com/843996 When building with use_jumbo_build=true and cc_wrapper="sccache"[1], the unit containing v8_generated_core_bindings was refused by the cache. It also was an outlier for compilation time, 2 times the next slowest object and 3x the next jumbo unit (jumbo_file_merge_limit=14). For linux builds of content shell with a hot cache, this improved build times by about a minute, ~20% of wall time, due to the expensive miss. [1] https://github.com/mozilla/sccache Bug: 634231 Change-Id: I07ee42fc442ee3372e54ae757466d7af0a5845e8 Reviewed-on: https://chromium-review.googlesource.com/1164145 Reviewed-by: Kenichi Ishibashi <bashi@chromium.org> Reviewed-by: Kentaro Hara <haraken@chromium.org> Reviewed-by: Hitoshi Yoshida <peria@chromium.org> Reviewed-by: Daniel Bratell <bratell@opera.com> Commit-Queue: Hitoshi Yoshida <peria@chromium.org> Cr-Commit-Position: refs/heads/master@{#591622} [modify] https://crrev.com/5b6fe95874503156d33bf99b8df3e2abbff23554/third_party/blink/renderer/bindings/core/v8/BUILD.gn |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by yukishiino@chromium.org
, Aug 4 2016Components: Blink>Bindings
Owner: peria@chromium.org