GN: refactor concurrent_links to use explicit link_pool |
||||||
Issue descriptionThe copy-bundle-data step is really IO bound because it invokes a python script that does some conversion on the input (for .strings file). This cause a compilation using goma and -j1000 to stall when there are lots of copy-bundle-data target run in parallel. I think we should use a smaller pool for copy-bundle-data target. Since this is a tool, maybe we can extend the "tool" objects in gn to allow configuration of a pool with a size. brettw: any objections?
,
May 21 2016
Proposed design document: https://docs.google.com/document/d/1b598i4WmeFOe3_iRBrjPKSXdcR5R32UPtgxqKmoY2-M/edit#
,
May 30 2016
,
Jun 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/40451c5093a4108bf65d3aae30bea2ab014f4b94 commit 40451c5093a4108bf65d3aae30bea2ab014f4b94 Author: sdefresne <sdefresne@chromium.org> Date: Sat Jun 04 12:37:29 2016 Add support for user defined "pool" to GN. Allow user to define pool of limited size to limit the number of concurrent tasks that are executed for a given set of tools. The pool object directly correspond to a ninja pool and can be shared by multiple targets. Design document: https://docs.google.com/document/d/1b598i4WmeFOe3_iRBrjPKSXdcR5R32UPtgxqKmoY2-M/view BUG= 612786 Review-Url: https://codereview.chromium.org/2006923004 Cr-Commit-Position: refs/heads/master@{#397917} [modify] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/BUILD.gn [modify] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/builder.cc [modify] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/builder.h [modify] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/builder_record.cc [modify] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/builder_record.h [modify] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/docs/reference.md [modify] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/function_toolchain.cc [modify] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/functions.cc [modify] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/functions.h [modify] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/gn.gyp [modify] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/item.cc [modify] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/item.h [modify] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/ninja_build_writer.cc [modify] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/ninja_build_writer.h [modify] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/ninja_build_writer_unittest.cc [modify] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/ninja_toolchain_writer.cc [modify] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/ninja_writer.cc [modify] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/ninja_writer.h [add] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/pool.cc [add] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/pool.h [modify] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/tool.h [modify] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/toolchain.cc [modify] https://crrev.com/40451c5093a4108bf65d3aae30bea2ab014f4b94/tools/gn/toolchain.h
,
Jun 7 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/e9efe32c54352ee81afac228219361eb77c832cf commit e9efe32c54352ee81afac228219361eb77c832cf Author: sdefresne <sdefresne@chromium.org> Date: Tue Jun 07 22:21:59 2016 Fix documentation for "pool" object in gn binary. Followup to comments on https://codereview.chromium.org/2006923004/. BUG= 612786 Review-Url: https://codereview.chromium.org/2045713003 Cr-Commit-Position: refs/heads/master@{#398400} [modify] https://crrev.com/e9efe32c54352ee81afac228219361eb77c832cf/tools/gn/docs/reference.md [modify] https://crrev.com/e9efe32c54352ee81afac228219361eb77c832cf/tools/gn/functions.cc
,
Jun 8 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/a78c5b20b9d0d8cb6e883e3efb83dafbc7849655 commit a78c5b20b9d0d8cb6e883e3efb83dafbc7849655 Author: sdefresne <sdefresne@chromium.org> Date: Wed Jun 08 16:10:05 2016 [GN] Configure a pool for copy_bundle_data and compile_xcassets tools. Reduce the number of tasks using the copy_bundle_data and compile_xcassets tools as they can cause lots of I/O contention when invoking ninja with a large number of parallel jobs (e.g. when using distributed build like goma). Use the same depth as the link_pool (but in a separate pool). BUG= 612786 Review-Url: https://codereview.chromium.org/2018553003 Cr-Commit-Position: refs/heads/master@{#398585} [modify] https://crrev.com/a78c5b20b9d0d8cb6e883e3efb83dafbc7849655/build/toolchain/mac/BUILD.gn
,
Jun 8 2016
This is a superset of the concurrent_links, so next step is to migrate code using concurrent_links to declare a link_pool with a depth equal to concurrent_links and to use the link_pool in the tools.
,
Jun 9 2016
I'm seeing the error
"""
Generating files...
ERROR at //build/toolchain/mac/BUILD.gn:30:3: Unknown function.
pool("bundle_pool") {
^---
See //BUILD.gn:61:1: which caused the file to be included.
group("gn_all") {
^----------------
"""
,
Jun 9 2016
That probably means you skipping running hooks when you synced.
,
Jun 24 2016
Hmm ... so this implements per-(tool, toolchain) pools, right? How hard would it be to extend that to per-target pools? That could potentially help with bug 617429 and 610459.
,
Jun 27 2016
I think it would be possible to allow setting a pool per target, but I'm not sure it would be useful. AFAICT, ninja pool means that for each job in the same pool, only N (where N is the pool depth) can be run in parallel. Currently our link target all go to the link_pool with a depth of computed by get_concurrent_links.py. If we want to use a smaller pool for large link target, then they will be allowed to be run concurrently to any other target in other pool, including link_pool. This may not be what we want. Instead we probably want to make the link_pool smaller.
,
Jun 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/f42145ad884229d1bd0f1b9012d5afeedb180f68 commit f42145ad884229d1bd0f1b9012d5afeedb180f68 Author: sdefresne <sdefresne@chromium.org> Date: Mon Jun 27 14:05:27 2016 [iOS/GN] Add a variable to configure the depth of "bundle_pool". Chrome on iOS downstream developer wants to be able to control the size of the "bundle_pool" independently of the number of concurrent link targets. BUG= 612786 Review-Url: https://codereview.chromium.org/2085463002 Cr-Commit-Position: refs/heads/master@{#402166} [modify] https://crrev.com/f42145ad884229d1bd0f1b9012d5afeedb180f68/build/toolchain/mac/BUILD.gn
,
Jun 27 2016
in comment #11, sdefresne@ wrote: > If we want to use a smaller pool for large link target, > then they will be allowed to be run concurrently to any > other target in other pool, including link_pool. This > may not be what we want. Instead we probably want to > make the link_pool smaller. It's true that we wouldn't want to max out both linker pools at the same time. However, just making the link_pool smaller will underutilize the system, too. Ideally we'd want something more dynamic that could mix small and large targets (e.g., base things on a weighted sum) or be dynamically adjusting things based on the memory in use (or projected to be in use somehow). Of course, such things are substantially harder. And we should take this discussion to one of the other bugs anyway ...
,
Jun 29 2016
The following revision refers to this bug: https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/2d3ebfc6b0745fcd365a821e9daee56f7ad896ed commit 2d3ebfc6b0745fcd365a821e9daee56f7ad896ed Author: sdefresne <sdefresne@google.com> Date: Wed Jun 29 15:26:56 2016
,
Jun 29 2016
The following revision refers to this bug: https://chrome-internal.googlesource.com/chrome/ios_internal.git/+/2d3ebfc6b0745fcd365a821e9daee56f7ad896ed commit 2d3ebfc6b0745fcd365a821e9daee56f7ad896ed Author: sdefresne <sdefresne@google.com> Date: Wed Jun 29 15:26:56 2016
,
Jun 29 2016
,
Jul 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1c27131e1bf77b059ef4c4f444bdccecdddeb0f6 commit 1c27131e1bf77b059ef4c4f444bdccecdddeb0f6 Author: sdefresne <sdefresne@chromium.org> Date: Wed Jul 27 08:45:57 2016 [GN] Prefer explicit pool to automatic link_pool for link targets. In preparation of refactoring the concurrent_links code to use explicit pools, prefer to use an explicit pool if defined over the automatic link_pool for link targets. BUG= 612786 Review-Url: https://codereview.chromium.org/2182093002 Cr-Commit-Position: refs/heads/master@{#408077} [modify] https://crrev.com/1c27131e1bf77b059ef4c4f444bdccecdddeb0f6/tools/gn/ninja_toolchain_writer.cc
,
Aug 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/96e7407d139b8a8e43f6db7d4af3118078aba7de commit 96e7407d139b8a8e43f6db7d4af3118078aba7de Author: sdefresne <sdefresne@chromium.org> Date: Thu Aug 04 18:41:06 2016 Use explicit pool to define concurrent_links jobs. Define an explicit pool //build/toolchain:link_pool with a depth of concurrent_links and convert all the toolchain link targets to use it instead of setting concurrent_links property of the tool. BUG= 612786 Review-Url: https://codereview.chromium.org/2201363004 Cr-Commit-Position: refs/heads/master@{#409846} [add] https://crrev.com/96e7407d139b8a8e43f6db7d4af3118078aba7de/build/toolchain/BUILD.gn [modify] https://crrev.com/96e7407d139b8a8e43f6db7d4af3118078aba7de/build/toolchain/gcc_toolchain.gni [modify] https://crrev.com/96e7407d139b8a8e43f6db7d4af3118078aba7de/build/toolchain/mac/BUILD.gn [modify] https://crrev.com/96e7407d139b8a8e43f6db7d4af3118078aba7de/build/toolchain/win/BUILD.gn
,
Aug 4 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/662a802270f017a00bc870f12866b8f581596d79 commit 662a802270f017a00bc870f12866b8f581596d79 Author: sdefresne <sdefresne@chromium.org> Date: Thu Aug 04 21:54:49 2016 Cleanup references to concurrent_links in gn binary. Explicit pool are recommended and concurrent_links is deprecated. Remove concurrent_links support as it should no longer be used. BUG= 612786 Review-Url: https://codereview.chromium.org/2211593002 Cr-Commit-Position: refs/heads/master@{#409909} [modify] https://crrev.com/662a802270f017a00bc870f12866b8f581596d79/tools/gn/docs/reference.md [modify] https://crrev.com/662a802270f017a00bc870f12866b8f581596d79/tools/gn/function_toolchain.cc [modify] https://crrev.com/662a802270f017a00bc870f12866b8f581596d79/tools/gn/ninja_build_writer.cc [modify] https://crrev.com/662a802270f017a00bc870f12866b8f581596d79/tools/gn/ninja_build_writer_unittest.cc [modify] https://crrev.com/662a802270f017a00bc870f12866b8f581596d79/tools/gn/toolchain.cc [modify] https://crrev.com/662a802270f017a00bc870f12866b8f581596d79/tools/gn/toolchain.h
,
Aug 4 2016
,
Aug 12 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/7d1c95f62081964bdfbce57f05ec226918a2d940 commit 7d1c95f62081964bdfbce57f05ec226918a2d940 Author: sdefresne <sdefresne@chromium.org> Date: Fri Aug 12 19:35:18 2016 Remove obsolete "link_pool" code from GN's ninja writer. The default "link_pool" was removed when code was refactored to use explicit pool, but this code was forgotten. It is now dead, removing. BUG= 612786 Review-Url: https://codereview.chromium.org/2240673003 Cr-Commit-Position: refs/heads/master@{#411735} [modify] https://crrev.com/7d1c95f62081964bdfbce57f05ec226918a2d940/tools/gn/ninja_toolchain_writer.cc |
||||||
►
Sign in to add a comment |
||||||
Comment 1 by brettw@chromium.org
, May 19 2016