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

Issue 2325973002: Remove aggregation of generated binding code. (Closed)

Created:
4 years, 3 months ago by peria
Modified:
4 years, 3 months ago
Reviewers:
haraken, bashi, Yuki
CC:
chromium-reviews, blink-reviews, blink-reviews-bindings_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

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. 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 Committed: https://crrev.com/aff7c7109dd3651121f94ceea720464844c86ec3 Cr-Commit-Position: refs/heads/master@{#417935}

Patch Set 1 #

Patch Set 2 : Aggregate on Windows #

Patch Set 3 : Fix build #

Unified diffs Side-by-side diffs Delta from patch set Stats (+124 lines, -321 lines) Patch
M third_party/WebKit/Source/bindings/core/v8/BUILD.gn View 1 2 4 chunks +30 lines, -98 lines 0 comments Download
M third_party/WebKit/Source/bindings/modules/v8/BUILD.gn View 4 chunks +4 lines, -26 lines 0 comments Download
M third_party/WebKit/Source/bindings/modules/v8/generated.gni View 2 chunks +18 lines, -91 lines 0 comments Download
M third_party/WebKit/Source/bindings/scripts/aggregate_generated_bindings.py View 1 4 chunks +29 lines, -85 lines 0 comments Download
M third_party/WebKit/Source/bindings/scripts/generate_init_partial_interfaces.py View 2 chunks +31 lines, -3 lines 0 comments Download
M third_party/WebKit/Source/bindings/scripts/scripts.gni View 1 chunk +10 lines, -16 lines 0 comments Download
M third_party/WebKit/Source/modules/BUILD.gn View 1 chunk +2 lines, -2 lines 0 comments Download

Messages

Total messages: 15 (8 generated)
peria
PTL. win_chrome_official bot shows the status what the original CL has failed.
4 years, 3 months ago (2016-09-12 08:44:41 UTC) #5
Yuki
LGTM, but I'd highly recommend to make the aggregation enabled for all windows builds, not ...
4 years, 3 months ago (2016-09-12 08:54:29 UTC) #6
peria
On 2016/09/12 08:54:29, Yuki wrote: > LGTM, but I'd highly recommend to make the aggregation ...
4 years, 3 months ago (2016-09-12 08:58:45 UTC) #7
haraken
LGTM
4 years, 3 months ago (2016-09-12 11:37:01 UTC) #8
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2325973002/80001
4 years, 3 months ago (2016-09-12 13:12:04 UTC) #11
commit-bot: I haz the power
Committed patchset #3 (id:80001)
4 years, 3 months ago (2016-09-12 14:35:43 UTC) #13
commit-bot: I haz the power
4 years, 3 months ago (2016-09-12 14:37:42 UTC) #15
Message was sent while issue was closed.
Patchset 3 (id:??) landed as
https://crrev.com/aff7c7109dd3651121f94ceea720464844c86ec3
Cr-Commit-Position: refs/heads/master@{#417935}

Powered by Google App Engine
This is Rietveld 408576698