Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(133)

Issue 56433003: GN threading refactor (Closed)

Created:
7 years, 1 month ago by brettw
Modified:
7 years, 1 month ago
Reviewers:
scottmg
CC:
chromium-reviews
Visibility:
Public.

Description

Refactor the GN threading model to reduce locking and simplify execution. Old design: item_tree manages dependency resolution, target_manager creates and hooks up the target dependencies, and the toolchain manager does all kinds of crazy ordering and loading stuff on every thread. There is lots of locking every time the dependency tree is modified. This patch completely deletes the toolchain manager, item tree, item node, and target manager. The new design is that the build files are executed on background threads, but then they come back to the main thread which assembles the dependency tree and schedules other file loads. This eliminates almost all locking and a lot of complexity. The Loader now manages loading the different build files in the correct context, and managing dependencies (loading the build config first, for example). The Builder manages the dependency tree and requests that the Loader load files that it encounters references to. This simpler design reduces non-test code by ~350 lines. In the current gn build, lock acquisitions go down from 1808 to 116 and it saves about 20ms wall clock time (8% faster). This is a bit deceiving, though, because most of the time is spent on pkg-config which is constant. It speeds up running individual build files by 1000-1500%. The work of putting the tree together that used to take up this time inside locks is now transferred to the main thread, which looks like it is *sometimes* a bottleneck. This should be easier to optimize in the future, though. Other tweaks: - I fixed cycle finding - Directories look prettier on Windows using the "desc" command - Tracing output now includes the main thread and marks it as such - I updated the base BUILD.gn file for the more recent tree. R=scottmg@chromium.org Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=234032

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : #

Patch Set 4 : #

Patch Set 5 : #

Patch Set 6 : #

Patch Set 7 : #

Patch Set 8 : #

Patch Set 9 : #

Patch Set 10 : #

Patch Set 11 : linux fixes #

Patch Set 12 : #

Patch Set 13 : #

Patch Set 14 : #

Patch Set 15 : #

Patch Set 16 : #

Patch Set 17 : #

Patch Set 18 : #

Unified diffs Side-by-side diffs Delta from patch set Stats (+2220 lines, -2257 lines) Patch
M tools/gn/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 6 chunks +9 lines, -10 lines 0 comments Download
M tools/gn/bin/linux/gn.sha1 View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/bin/win/gn.exe.sha1 View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/binary_target_generator.h View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/binary_target_generator.cc View 1 2 3 4 5 2 chunks +8 lines, -9 lines 0 comments Download
M tools/gn/build_settings.h View 1 2 3 4 5 chunks +8 lines, -37 lines 0 comments Download
M tools/gn/build_settings.cc View 1 2 3 4 5 6 2 chunks +7 lines, -11 lines 0 comments Download
A tools/gn/builder.h View 1 2 3 4 5 6 7 8 9 1 chunk +135 lines, -0 lines 0 comments Download
A tools/gn/builder.cc View 1 2 3 4 5 6 7 8 9 1 chunk +471 lines, -0 lines 0 comments Download
A tools/gn/builder_record.h View 1 2 3 4 5 6 7 8 9 10 11 1 chunk +112 lines, -0 lines 0 comments Download
A tools/gn/builder_record.cc View 1 1 chunk +69 lines, -0 lines 0 comments Download
A tools/gn/builder_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +226 lines, -0 lines 0 comments Download
M tools/gn/command_desc.cc View 1 2 3 4 5 6 7 8 9 4 chunks +20 lines, -3 lines 0 comments Download
M tools/gn/command_gen.cc View 1 2 3 4 5 6 7 8 9 10 2 chunks +24 lines, -6 lines 0 comments Download
M tools/gn/command_gyp.cc View 1 2 3 4 5 6 7 8 9 7 chunks +51 lines, -38 lines 0 comments Download
M tools/gn/command_refs.cc View 1 2 3 4 5 4 chunks +15 lines, -18 lines 0 comments Download
M tools/gn/commands.cc View 1 2 3 4 5 6 7 8 9 4 chunks +7 lines, -14 lines 0 comments Download
M tools/gn/config.h View 1 2 3 4 2 chunks +0 lines, -15 lines 0 comments Download
M tools/gn/config.cc View 1 2 3 4 5 6 7 8 2 chunks +0 lines, -58 lines 0 comments Download
M tools/gn/config_values_generator.h View 1 2 3 4 5 2 chunks +4 lines, -2 lines 0 comments Download
M tools/gn/config_values_generator.cc View 1 2 3 4 5 1 chunk +5 lines, -6 lines 0 comments Download
M tools/gn/copy_target_generator.h View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/copy_target_generator.cc View 1 2 3 4 5 2 chunks +5 lines, -6 lines 0 comments Download
M tools/gn/function_set_default_toolchain.cc View 1 2 3 4 3 chunks +7 lines, -6 lines 0 comments Download
M tools/gn/function_toolchain.cc View 1 2 3 4 5 6 2 chunks +5 lines, -14 lines 0 comments Download
M tools/gn/functions.cc View 1 2 3 4 5 6 2 chunks +6 lines, -16 lines 0 comments Download
M tools/gn/functions_target.cc View 1 2 3 4 5 3 chunks +3 lines, -4 lines 0 comments Download
M tools/gn/gn.gyp View 1 2 3 4 5 chunks +8 lines, -9 lines 0 comments Download
M tools/gn/group_target_generator.h View 1 2 3 4 5 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/group_target_generator.cc View 1 2 3 4 5 1 chunk +6 lines, -5 lines 0 comments Download
M tools/gn/gyp_binary_target_writer.cc View 1 2 3 4 5 6 7 8 9 6 chunks +16 lines, -10 lines 0 comments Download
M tools/gn/gyp_target_writer.h View 1 2 3 4 5 6 7 8 9 2 chunks +5 lines, -4 lines 0 comments Download
M tools/gn/gyp_target_writer.cc View 1 2 3 4 5 6 7 8 9 3 chunks +7 lines, -4 lines 0 comments Download
M tools/gn/input_file_manager.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +3 lines, -0 lines 0 comments Download
M tools/gn/item.h View 1 2 3 4 3 chunks +4 lines, -9 lines 0 comments Download
M tools/gn/item.cc View 1 chunk +2 lines, -1 line 0 comments Download
D tools/gn/item_node.h View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -159 lines 0 comments Download
D tools/gn/item_node.cc View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -120 lines 0 comments Download
D tools/gn/item_tree.h View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -93 lines 0 comments Download
D tools/gn/item_tree.cc View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -208 lines 0 comments Download
M tools/gn/label_ptr.h View 1 1 chunk +3 lines, -0 lines 0 comments Download
A tools/gn/loader.h View 1 2 3 4 5 6 7 8 9 1 chunk +177 lines, -0 lines 0 comments Download
A tools/gn/loader.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +398 lines, -0 lines 0 comments Download
A tools/gn/loader_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +191 lines, -0 lines 0 comments Download
M tools/gn/ninja_target_writer.h View 1 2 3 4 1 chunk +1 line, -1 line 0 comments Download
M tools/gn/ninja_target_writer.cc View 1 2 3 4 3 chunks +6 lines, -16 lines 0 comments Download
M tools/gn/ninja_toolchain_writer.h View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +6 lines, -6 lines 0 comments Download
M tools/gn/ninja_toolchain_writer.cc View 1 2 3 4 5 6 7 8 9 10 11 12 5 chunks +6 lines, -20 lines 0 comments Download
M tools/gn/ninja_writer.h View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +6 lines, -8 lines 0 comments Download
M tools/gn/ninja_writer.cc View 1 2 3 4 5 6 7 8 9 10 11 12 2 chunks +32 lines, -34 lines 0 comments Download
M tools/gn/scheduler.h View 1 2 3 4 5 6 7 8 9 10 11 2 chunks +6 lines, -1 line 0 comments Download
M tools/gn/scheduler.cc View 1 2 3 4 5 4 chunks +6 lines, -13 lines 0 comments Download
M tools/gn/scope.h View 1 2 chunks +1 line, -8 lines 0 comments Download
M tools/gn/scope.cc View 1 2 chunks +1 line, -20 lines 0 comments Download
M tools/gn/scope_per_file_provider.cc View 1 2 3 4 2 chunks +1 line, -4 lines 0 comments Download
M tools/gn/script_target_generator.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +1 line, -2 lines 0 comments Download
M tools/gn/script_target_generator.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 2 chunks +8 lines, -6 lines 0 comments Download
M tools/gn/secondary/base/BUILD.gn View 1 2 3 4 5 6 7 8 9 2 chunks +0 lines, -3 lines 0 comments Download
M tools/gn/settings.h View 1 2 3 4 2 chunks +11 lines, -3 lines 0 comments Download
M tools/gn/settings.cc View 1 2 3 4 3 chunks +1 line, -3 lines 0 comments Download
M tools/gn/setup.h View 1 2 3 4 5 6 7 8 9 4 chunks +7 lines, -2 lines 0 comments Download
M tools/gn/setup.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 5 chunks +37 lines, -6 lines 0 comments Download
M tools/gn/target.h View 1 2 3 4 2 chunks +0 lines, -10 lines 0 comments Download
M tools/gn/target.cc View 1 2 3 4 5 6 7 8 9 3 chunks +3 lines, -25 lines 0 comments Download
M tools/gn/target_generator.h View 1 2 3 4 5 3 chunks +6 lines, -10 lines 0 comments Download
M tools/gn/target_generator.cc View 1 2 3 4 5 6 7 8 9 10 11 6 chunks +29 lines, -98 lines 0 comments Download
D tools/gn/target_manager.h View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -69 lines 0 comments Download
D tools/gn/target_manager.cc View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -129 lines 0 comments Download
D tools/gn/target_manager_unittest.cc View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -95 lines 0 comments Download
M tools/gn/test_with_scope.cc View 1 2 3 4 5 6 7 8 9 1 chunk +1 line, -1 line 0 comments Download
D tools/gn/toolchain_manager.h View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -170 lines 0 comments Download
D tools/gn/toolchain_manager.cc View 1 2 3 4 5 6 7 8 1 chunk +0 lines, -593 lines 0 comments Download
M tools/gn/trace.h View 1 2 3 4 5 6 7 2 chunks +3 lines, -1 line 0 comments Download
M tools/gn/trace.cc View 1 2 3 4 5 6 7 4 chunks +19 lines, -0 lines 0 comments Download

Messages

Total messages: 3 (0 generated)
brettw
7 years, 1 month ago (2013-11-06 22:48:48 UTC) #1
scottmg
Seems a lot better. lgtm
7 years, 1 month ago (2013-11-08 05:06:21 UTC) #2
brettw
7 years, 1 month ago (2013-11-08 23:25:50 UTC) #3
Message was sent while issue was closed.
Committed patchset #18 manually as r234032 (presubmit successful).

Powered by Google App Engine
This is Rietveld 408576698