New issue
Advanced search Search tips

Issue 713137 link

Starred by 5 users

Issue metadata

Status: Fixed
Owner:
Closed: Mar 2018
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , Mac
Pri: 3
Type: Task


Sign in to add a comment

Switch Blink to Jumbo compile

Project Member Reported by brat...@opera.com, Apr 19 2017

Issue description

It takes a lot of time to compile Blink which is mostly because every compilation unit has a lot of headers (200,000 lines) and not much content (50-1,000 lines). Experiments show that it's 10x faster to compile a target such as html with the files merged instead of compiling them one by one.

This task is to track the project to make such a build system default for Blink (similar build systems are already used by sqlite and blink bindings).

A lot more details can be found in https://docs.google.com/document/d/19jGsZxh7DX8jkAKbL1nYBa5rcByUL2EeidnYsoXfsYQ/edit#

 
Labels: -OS-iOS

Comment 2 by bokan@chromium.org, Apr 19 2017

Components: -Blink Blink>Internals

Comment 3 by brat...@opera.com, Apr 28 2017

Labels: jumbo

Comment 4 by brat...@opera.com, Jun 14 2017

Blockedon: 733193
Project Member

Comment 5 by bugdroid1@chromium.org, Jul 3 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/45a1ceb8309b6076aea7d2885b7a62f8d3c549c9

commit 45a1ceb8309b6076aea7d2885b7a62f8d3c549c9
Author: bratell <bratell@opera.com>
Date: Mon Jul 03 11:14:37 2017

Scripts for unity/jumbo (default disabled) compilation.

To speed up compilation times, jumbo allows files to be compiled
together. This is a well known method ("unity builds") to both
compile faster and create a poor man's "full program optimization".
For Chromium we are only interested in the compile times.

This patch includes the basic scripts that do the source file merging
and changes Blink Core to use those scripts. If the gn configuration
includes: use_jumbo_build = true then Blink Core will use jumbo
compile. Otherwise it will compile as usual.

The expected speedup from using Jumbo on Blink Core (and nothing else)
is about 17% of the content_shell+blink_tests compilation CPU
time. This is about half an hour for people building with an ordinary
computer, but less both in percentage and minutes if using some kind
of build accelerator like goma.

More information in
https://docs.google.com/document/d/19jGsZxh7DX8jkAKbL1nYBa5rcByUL2EeidnYsoXfsYQ/edit#

BUG= 713137 

Review-Url: https://codereview.chromium.org/2963733003
Cr-Commit-Position: refs/heads/master@{#483986}

[add] https://crrev.com/45a1ceb8309b6076aea7d2885b7a62f8d3c549c9/build/config/jumbo.gni
[add] https://crrev.com/45a1ceb8309b6076aea7d2885b7a62f8d3c549c9/build/config/merge_for_jumbo.py
[modify] https://crrev.com/45a1ceb8309b6076aea7d2885b7a62f8d3c549c9/third_party/WebKit/Source/core/core.gni
[modify] https://crrev.com/45a1ceb8309b6076aea7d2885b7a62f8d3c549c9/third_party/WebKit/Source/core/exported/BUILD.gn
[modify] https://crrev.com/45a1ceb8309b6076aea7d2885b7a62f8d3c549c9/third_party/WebKit/Source/core/html/BUILD.gn
[modify] https://crrev.com/45a1ceb8309b6076aea7d2885b7a62f8d3c549c9/third_party/WebKit/Source/core/inspector/BUILD.gn

Comment 6 by brat...@opera.com, Jul 3 2017

Blockedon: 738871
Project Member

Comment 8 by bugdroid1@chromium.org, Jul 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c5aaecf90aacfdb4c8fce11394431e552f1e6339

commit c5aaecf90aacfdb4c8fce11394431e552f1e6339
Author: bratell <bratell@opera.com>
Date: Wed Jul 05 17:19:57 2017

Use aggregated bindings for jumbo as well as for Windows.

If the builder requests a jumbo build, use aggregated bindings since
that is more or less the same thing.

BUG= 713137 

Review-Url: https://codereview.chromium.org/2971713002
Cr-Commit-Position: refs/heads/master@{#484298}

[modify] https://crrev.com/c5aaecf90aacfdb4c8fce11394431e552f1e6339/third_party/WebKit/Source/bindings/core/v8/BUILD.gn

Project Member

Comment 9 by bugdroid1@chromium.org, Jul 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/d0fbcabbd9fe9c624a67fdc14379697c83cb6798

commit d0fbcabbd9fe9c624a67fdc14379697c83cb6798
Author: bratell <bratell@opera.com>
Date: Fri Jul 07 17:04:46 2017

Use unique variable names in gperf generated code

By default gperf generates code with the types and variables
stringpool_t and stringpool. If jumbo combines more than one gperf
generated file those collides.

This patch changes the variable names to something more unique.

R=fs@opera.com
BUG= 713137 

Review-Url: https://codereview.chromium.org/2972193002
Cr-Commit-Position: refs/heads/master@{#484961}

[modify] https://crrev.com/d0fbcabbd9fe9c624a67fdc14379697c83cb6798/third_party/WebKit/Source/build/scripts/make_css_property_names.py
[modify] https://crrev.com/d0fbcabbd9fe9c624a67fdc14379697c83cb6798/third_party/WebKit/Source/build/scripts/make_css_value_keywords.py

Project Member

Comment 10 by bugdroid1@chromium.org, Jul 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/ed0ad4be4fe8cf37c22664947124f67ee108a9a1

commit ed0ad4be4fe8cf37c22664947124f67ee108a9a1
Author: Daniel Bratell <bratell@opera.com>
Date: Mon Jul 10 11:26:23 2017

Add jumbo support for Blink/core generated files

Core generated files does not use the same template as most blink core
code so it needs to be explicitly turned on (note jumbo is still by
default disabled so normal builds are not yet affected).

This saves about 8 CPU minutes for me.

Bug:  713137 
Change-Id: I73ad558021b245c8742672cd3903e27dfdf48ab5
Reviewed-on: https://chromium-review.googlesource.com/563682
Reviewed-by: Philip Rogers <pdr@chromium.org>
Commit-Queue: bratell at Opera <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#485232}
[modify] https://crrev.com/ed0ad4be4fe8cf37c22664947124f67ee108a9a1/third_party/WebKit/Source/core/BUILD.gn

Project Member

Comment 11 by bugdroid1@chromium.org, Jul 10 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/31abd9178c297b8c98cb43bf4037ff6b07ab0f83

commit 31abd9178c297b8c98cb43bf4037ff6b07ab0f83
Author: bratell <bratell@opera.com>
Date: Mon Jul 10 14:40:18 2017

Jumbo for blink/core generated files as well (saving 8 CPU minutes)

The target that compiles generated files does not use the same
template as other code in blink core so it didn't automatically become
jumbo enabled. Since it's a non-negliable part of the build time (~1%)
this patch enables jumbo for this target as well.

R=fs@opera.com
BUG= 713137 

Review-Url: https://codereview.chromium.org/2973603003
Cr-Commit-Position: refs/heads/master@{#485267}

[modify] https://crrev.com/31abd9178c297b8c98cb43bf4037ff6b07ab0f83/build/config/jumbo.gni
[modify] https://crrev.com/31abd9178c297b8c98cb43bf4037ff6b07ab0f83/third_party/WebKit/Source/core/BUILD.gn

Project Member

Comment 12 by bugdroid1@chromium.org, Jul 11 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/061195588f7477633e30fc525e394e4af1d13da8

commit 061195588f7477633e30fc525e394e4af1d13da8
Author: bratell <bratell@opera.com>
Date: Tue Jul 11 11:25:27 2017

Change code to make jumbo exceptions unnecessary in core/html

In jumbo several compilation units are merged so you have to
have more unique names in each one, and don't #undef macros
needed by other compilation units.

BUG= 713137 

Review-Url: https://codereview.chromium.org/2971683003
Cr-Commit-Position: refs/heads/master@{#485585}

[modify] https://crrev.com/061195588f7477633e30fc525e394e4af1d13da8/third_party/WebKit/Source/core/html/BUILD.gn
[modify] https://crrev.com/061195588f7477633e30fc525e394e4af1d13da8/third_party/WebKit/Source/core/html/HTMLIFrameElementSandbox.cpp
[modify] https://crrev.com/061195588f7477633e30fc525e394e4af1d13da8/third_party/WebKit/Source/core/html/parser/HTMLTokenizer.cpp

Project Member

Comment 13 by bugdroid1@chromium.org, Jul 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/aff249261b8bc19325b7064c1f9a6551604a86ca

commit aff249261b8bc19325b7064c1f9a6551604a86ca
Author: Daniel Bratell <bratell@opera.com>
Date: Wed Jul 12 10:17:53 2017

Also check jumbo_excluded_sources when jumbo is disabled

Since jumbo is still disabled by default and there is no active
bot that checks things, it is best to check that jumbo_excluded_sources
are correct also when jumbo is disabled.

Bug:  713137 
Change-Id: I85bb92f283be240a704ce3ec0d5765933ba52016
Reviewed-on: https://chromium-review.googlesource.com/563683
Commit-Queue: bratell at Opera <bratell@opera.com>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#485913}
[modify] https://crrev.com/aff249261b8bc19325b7064c1f9a6551604a86ca/build/config/jumbo.gni

Project Member

Comment 14 by bugdroid1@chromium.org, Jul 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b4c3bcce5ee53d982eb59e9bb4c445608ef36084

commit b4c3bcce5ee53d982eb59e9bb4c445608ef36084
Author: Daniel Bratell <bratell@opera.com>
Date: Fri Jul 14 08:13:06 2017

Fix error in error message concatenation in jumbo script.

Best to not try to add a string to a set without converting the set to
a string first.

R=dpranke@chromium.org

Bug:  713137 
Change-Id: Id4c632b1f944fcafdd046983b1ddd4678c44928f
Reviewed-on: https://chromium-review.googlesource.com/564939
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Commit-Queue: bratell at Opera <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#486706}
[modify] https://crrev.com/b4c3bcce5ee53d982eb59e9bb4c445608ef36084/build/config/merge_for_jumbo.py

Comment 15 by brat...@opera.com, Jul 14 2017

Blockedon: 742987
Project Member

Comment 16 by bugdroid1@chromium.org, Jul 14 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/2c1ca75dd87c652025b0ad4d51cb377c6e698727

commit 2c1ca75dd87c652025b0ad4d51cb377c6e698727
Author: Daniel Bratell <bratell@opera.com>
Date: Fri Jul 14 18:01:13 2017

Remove some jumbo exclusions no longer needed

The files listed in core_generated jumbo exclusions have since been
fixed so the exclusions are no longer necessary.

R=fs@opera.com

Bug:  713137 
Change-Id: Ib9a9216147e9df7d8b196de761556d3f23997448
Reviewed-on: https://chromium-review.googlesource.com/571222
Commit-Queue: Daniel Bratell <bratell@opera.com>
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Cr-Commit-Position: refs/heads/master@{#486804}
[modify] https://crrev.com/2c1ca75dd87c652025b0ad4d51cb377c6e698727/third_party/WebKit/Source/core/BUILD.gn

Project Member

Comment 17 by bugdroid1@chromium.org, Jul 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/b2bba347da7bb61199dfb2a6f665c296a4414c86

commit b2bba347da7bb61199dfb2a6f665c296a4414c86
Author: Daniel Bratell <bratell@opera.com>
Date: Mon Jul 17 13:54:49 2017

Add support for jumbo_component.

Normal jumbo_target has the wrong default configs for components so this
adds a new jumbo_component that has the right default configs.

Bug:  713137 
Change-Id: I32fd2d015162d85b7ff07d5515489b9e33cfb987
Reviewed-on: https://chromium-review.googlesource.com/571790
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#487068}
[modify] https://crrev.com/b2bba347da7bb61199dfb2a6f665c296a4414c86/build/config/jumbo.gni

Comment 18 by brat...@opera.com, Jul 18 2017

Blockedon: 716395

Comment 19 by brat...@opera.com, Jul 18 2017

Blockedon: 745732
Project Member

Comment 20 by bugdroid1@chromium.org, Jul 19 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/12a3a5ee08bbe50d1eb1fa45dcd384aefdd053f7

commit 12a3a5ee08bbe50d1eb1fa45dcd384aefdd053f7
Author: Daniel Bratell <bratell@opera.com>
Date: Wed Jul 19 11:23:52 2017

Make multiple jumbo_* templates instead of just jumbo_target

Initially there was just one template, jumbo_target, but that
caused problems with default configurations and also made every
patch one line more verbose than necessary. Better to have multiple
templates and make the usage just a little bit simple.

Bug:  713137 
Change-Id: I7e2b1f0c5216b4465bae331763d6aa5a08e1c996
Reviewed-on: https://chromium-review.googlesource.com/575058
Commit-Queue: Daniel Bratell <bratell@opera.com>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#487823}
[modify] https://crrev.com/12a3a5ee08bbe50d1eb1fa45dcd384aefdd053f7/build/config/jumbo.gni
[modify] https://crrev.com/12a3a5ee08bbe50d1eb1fa45dcd384aefdd053f7/docs/jumbo.md
[modify] https://crrev.com/12a3a5ee08bbe50d1eb1fa45dcd384aefdd053f7/third_party/WebKit/Source/core/BUILD.gn
[modify] https://crrev.com/12a3a5ee08bbe50d1eb1fa45dcd384aefdd053f7/third_party/WebKit/Source/core/core.gni

Comment 21 by most...@opera.com, Jul 19 2017

Cc: most...@opera.com
Project Member

Comment 22 by bugdroid1@chromium.org, Jul 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/08a8c75946f971fad5499d6596d10d4734bc9d57

commit 08a8c75946f971fad5499d6596d10d4734bc9d57
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Jul 20 09:54:08 2017

Make blink/modules support jumbo compilations (-60 CPU minutes)

Compiling browser code in blink/modules currently use
about 80 CPU minutes. That is about 7% of the total compilation time.
If you use jumbo compilation (merge many files into a single
translation unit) that time drops to about a fifth of that.

There are also unit_tests in modules that will be jumbofied in
a different patch.

Bug:  713137 
Change-Id: I9155d2af0c9dce6b3178f77b9366062eb45d4560
Reviewed-on: https://chromium-review.googlesource.com/568302
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#488179}
[modify] https://crrev.com/08a8c75946f971fad5499d6596d10d4734bc9d57/third_party/WebKit/Source/modules/BUILD.gn
[modify] https://crrev.com/08a8c75946f971fad5499d6596d10d4734bc9d57/third_party/WebKit/Source/modules/fetch/BUILD.gn
[modify] https://crrev.com/08a8c75946f971fad5499d6596d10d4734bc9d57/third_party/WebKit/Source/modules/modules.gni
[modify] https://crrev.com/08a8c75946f971fad5499d6596d10d4734bc9d57/third_party/WebKit/Source/modules/webaudio/BUILD.gn

Comment 23 by brat...@opera.com, Jul 20 2017

Blocking: 746956
Project Member

Comment 24 by bugdroid1@chromium.org, Jul 20 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/fcb5e630fa8d5c2723a6048bb0940a288fa5e962

commit fcb5e630fa8d5c2723a6048bb0940a288fa5e962
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Jul 20 14:37:23 2017

Support jumbo builds in core unit tests (except editing)

This patch makes the build system for core unit tests support jumbo
builds (unity builds) which saves roughly 60 CPU minutes (5% of the
build time) on my computer. Currently jumbo is by default disabled so
this will have no direct effect unless you have
  use_jumbo_build = true
in your gn settings.

Bug:  713137 
Change-Id: I376e62fb66738cba9135d02f8079d983cfe67495
Reviewed-on: https://chromium-review.googlesource.com/575055
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#488238}
[modify] https://crrev.com/fcb5e630fa8d5c2723a6048bb0940a288fa5e962/third_party/WebKit/Source/core/BUILD.gn

Comment 25 by brat...@opera.com, Jul 21 2017

Blocking: 747263

Comment 26 by brat...@opera.com, Jul 21 2017

Blocking: -747263

Comment 27 by brat...@opera.com, Sep 1 2017

Blockedon: 761475
Project Member

Comment 28 by bugdroid1@chromium.org, Sep 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/026e4d7c20c84a78bb4d8e0a36eea53f71562c54

commit 026e4d7c20c84a78bb4d8e0a36eea53f71562c54
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Sep 05 16:26:53 2017

Enable jumbo compiling for modules unit test (-13 CPU minutes)

When use_jumbo_build=true this patch should save roughly 12.5
CPU minutes on my reference machine.

Bug:  713137 
Change-Id: Icaa9e691ca6212f336691755aea83ddbdbe23a18
Reviewed-on: https://chromium-review.googlesource.com/648408
Commit-Queue: Daniel Bratell <bratell@opera.com>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Cr-Commit-Position: refs/heads/master@{#499651}
[modify] https://crrev.com/026e4d7c20c84a78bb4d8e0a36eea53f71562c54/third_party/WebKit/Source/modules/BUILD.gn

Comment 29 by most...@vewd.com, Sep 30 2017

Cc: -most...@opera.com most...@vewd.com
Project Member

Comment 30 by bugdroid1@chromium.org, Oct 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/62b04937e2563374f4a25f9e6503418a1b001414

commit 62b04937e2563374f4a25f9e6503418a1b001414
Author: Mostyn Bramley-Moore <mostynb@vewd.com>
Date: Mon Oct 02 06:55:02 2017

[jumbo] avoid duplicate kUnknownString in FontVariant*.cpp

TBR=eae@chromium.org

Bug:  713137 
Change-Id: I16863c64825f9b39a18ac9a3b7ba23a3f3243cf0
Reviewed-on: https://chromium-review.googlesource.com/693863
Commit-Queue: Mostyn Bramley-Moore <mostynb@vewd.com>
Reviewed-by: Walter Korman <wkorman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#505548}
[modify] https://crrev.com/62b04937e2563374f4a25f9e6503418a1b001414/third_party/WebKit/Source/platform/fonts/FontVariantEastAsian.cpp
[modify] https://crrev.com/62b04937e2563374f4a25f9e6503418a1b001414/third_party/WebKit/Source/platform/fonts/FontVariantNumeric.cpp

Project Member

Comment 31 by bugdroid1@chromium.org, Oct 2 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e9ecb9183bfa78878f8a1f10f9c233e780f4ed89

commit e9ecb9183bfa78878f8a1f10f9c233e780f4ed89
Author: Mostyn Bramley-Moore <mostynb@vewd.com>
Date: Mon Oct 02 17:12:41 2017

[jumbo] alternative duplicate kUnknownString fix

Followup to https://chromium-review.googlesource.com/693863

Bug:  713137 
Change-Id: I06ae8c1514d89de27573b96dc7ee96fb73d29800
Reviewed-on: https://chromium-review.googlesource.com/695341
Reviewed-by: Emil A Eklund <eae@chromium.org>
Commit-Queue: Mostyn Bramley-Moore <mostynb@vewd.com>
Cr-Commit-Position: refs/heads/master@{#505656}
[modify] https://crrev.com/e9ecb9183bfa78878f8a1f10f9c233e780f4ed89/third_party/WebKit/Source/platform/fonts/FontVariantEastAsian.cpp
[modify] https://crrev.com/e9ecb9183bfa78878f8a1f10f9c233e780f4ed89/third_party/WebKit/Source/platform/fonts/FontVariantNumeric.cpp

Project Member

Comment 32 by bugdroid1@chromium.org, Oct 5 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/a2b3186369400385dd3f58710bd6e4b72ac5d5b4

commit a2b3186369400385dd3f58710bd6e4b72ac5d5b4
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Oct 05 21:41:48 2017

Support jumbo for webkit_unit_tests (-2.5 CPU minutes)

This makes webkit_unit_tests support jumbo compilation. In CPU effort
that reduces the reference build time by 2.5 CPU minutes (about 0.3%
of the total build time)

Bug:  713137 
Change-Id: I358ff81769e5fb0ebf91cd1264713c547fd3e47e
Reviewed-on: https://chromium-review.googlesource.com/677449
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#506867}
[modify] https://crrev.com/a2b3186369400385dd3f58710bd6e4b72ac5d5b4/third_party/WebKit/Source/controller/BUILD.gn

Project Member

Comment 33 by bugdroid1@chromium.org, Oct 7 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/16a004fe1218715520e4ecfd0d673d2cf0101c60

commit 16a004fe1218715520e4ecfd0d673d2cf0101c60
Author: Dirk Pranke <dpranke@chromium.org>
Date: Sat Oct 07 00:35:07 2017

Revert "Support jumbo for webkit_unit_tests (-2.5 CPU minutes)"

This reverts commit a2b3186369400385dd3f58710bd6e4b72ac5d5b4.

Reason for revert: Speculative revert for  crbug.com/772271 .

Original change's description:
> Support jumbo for webkit_unit_tests (-2.5 CPU minutes)
> 
> This makes webkit_unit_tests support jumbo compilation. In CPU effort
> that reduces the reference build time by 2.5 CPU minutes (about 0.3%
> of the total build time)
> 
> Bug:  713137 
> Change-Id: I358ff81769e5fb0ebf91cd1264713c547fd3e47e
> Reviewed-on: https://chromium-review.googlesource.com/677449
> Reviewed-by: Dirk Pranke <dpranke@chromium.org>
> Reviewed-by: Kentaro Hara <haraken@chromium.org>
> Commit-Queue: Daniel Bratell <bratell@opera.com>
> Cr-Commit-Position: refs/heads/master@{#506867}

TBR=dpranke@chromium.org,bratell@opera.com,haraken@chromium.org

# Not skipping CQ checks because original CL landed > 1 day ago.

Bug:  713137 
Change-Id: I6b2f2944ff593cfba235435ee09417b5881b6a4c
Reviewed-on: https://chromium-review.googlesource.com/706235
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Dirk Pranke <dpranke@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507242}
[modify] https://crrev.com/16a004fe1218715520e4ecfd0d673d2cf0101c60/third_party/WebKit/Source/controller/BUILD.gn

Project Member

Comment 34 by bugdroid1@chromium.org, Oct 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/4c48b5daa096160b7ef0a3cd6674d082fa3e9d2d

commit 4c48b5daa096160b7ef0a3cd6674d082fa3e9d2d
Author: Daniel Bratell <bratell@opera.com>
Date: Mon Oct 09 11:35:03 2017

Remove core/exported jumbo_excluded_source that is no longer needed

Because of an abundance of ToElement WebSearchableFormData was excluded
from jumbo. ToElement<ListedElement> was renamed so this exclude is no
longer needed.

Bug:  713137 
Change-Id: I255f6ac5df213f62e5d1f6c2c3ea6b0e96540448
Reviewed-on: https://chromium-review.googlesource.com/704696
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#507358}
[modify] https://crrev.com/4c48b5daa096160b7ef0a3cd6674d082fa3e9d2d/third_party/WebKit/Source/core/exported/BUILD.gn

Project Member

Comment 35 by bugdroid1@chromium.org, Oct 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/eb841686b565f1ea839a5034e087a88c764ea29e

commit eb841686b565f1ea839a5034e087a88c764ea29e
Author: Philip Rogers <pdr@chromium.org>
Date: Mon Oct 09 16:08:50 2017

Update linux and mac build docs with jumbo build info

This patch updates the linux and mac build instructions
to mention jumbo builds: how to enable it, and where to
find more information.

Bug:  713137 
Change-Id: I95f060660ecdb071b9102c9bba2f45259b08fd77
Reviewed-on: https://chromium-review.googlesource.com/706155
Reviewed-by: Daniel Bratell <bratell@opera.com>
Commit-Queue: Philip Rogers <pdr@chromium.org>
Cr-Commit-Position: refs/heads/master@{#507387}
[modify] https://crrev.com/eb841686b565f1ea839a5034e087a88c764ea29e/docs/linux_build_instructions.md
[modify] https://crrev.com/eb841686b565f1ea839a5034e087a88c764ea29e/docs/mac_build_instructions.md

Project Member

Comment 36 by bugdroid1@chromium.org, Oct 9 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/8c8260ed76e3db0b608fce9e24316fad67041949

commit 8c8260ed76e3db0b608fce9e24316fad67041949
Author: Daniel Bratell <bratell@opera.com>
Date: Mon Oct 09 20:25:05 2017

Support jumbo for webkit_unit_tests (-2.5 CPU minutes)

This makes webkit_unit_tests support jumbo compilation. In CPU effort
that reduces the reference build time by 2.5 CPU minutes (about 0.3%
of the total build time)

This landed once already as
https://chromium-review.googlesource.com/677449 but broke Android
testing. This has a fix for that problem (splitting test correctly
into test + source_set)

Bug:  713137 
Change-Id: I81a42f51e80f02c1874bc5171774d6d292c78a62
Reviewed-on: https://chromium-review.googlesource.com/707137
Reviewed-by: Kentaro Hara <haraken@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#507468}
[modify] https://crrev.com/8c8260ed76e3db0b608fce9e24316fad67041949/third_party/WebKit/Source/controller/BUILD.gn

Project Member

Comment 37 by bugdroid1@chromium.org, Oct 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/29ba55889314a138747f90d782fede9bbafd6d32

commit 29ba55889314a138747f90d782fede9bbafd6d32
Author: Morten Stenshorne <mstensho@opera.com>
Date: Thu Oct 12 11:14:37 2017

Fix jumbo build.

TBR=kojii@chromium.org

Bug:  713137 
Change-Id: I77d45dc11495dd15740b0189556fce07b6f39d6e
Reviewed-on: https://chromium-review.googlesource.com/715996
Commit-Queue: Morten Stenshorne <mstensho@opera.com>
Reviewed-by: Daniel Bratell <bratell@opera.com>
Reviewed-by: Morten Stenshorne <mstensho@opera.com>
Cr-Commit-Position: refs/heads/master@{#508309}
[modify] https://crrev.com/29ba55889314a138747f90d782fede9bbafd6d32/third_party/WebKit/Source/core/layout/ng/geometry/ng_logical_rect_test.cc
[modify] https://crrev.com/29ba55889314a138747f90d782fede9bbafd6d32/third_party/WebKit/Source/core/layout/ng/geometry/ng_physical_offset_rect_test.cc

Project Member

Comment 38 by bugdroid1@chromium.org, Oct 12 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/095851bc149483816fa9984a70081aa54e22e2df

commit 095851bc149483816fa9984a70081aa54e22e2df
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Oct 12 15:08:18 2017

Undef MAYBE* macros for jumbo builds.

In jumbo builds source files are merged and macros leak between
cc files. The easiest way to avoid that is to undef them after use.

This fixes a compilation problem with the MAYBE() macro which was
defined in two different tests.

R=fs@opera.com, mstensho@opera.com

Bug:  713137 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_layout_ng
Change-Id: Ie06359ad1fd1f432ccf666ca12cbd824216283fc
Reviewed-on: https://chromium-review.googlesource.com/716216
Reviewed-by: Morten Stenshorne <mstensho@opera.com>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#508352}
[modify] https://crrev.com/095851bc149483816fa9984a70081aa54e22e2df/third_party/WebKit/Source/core/editing/RenderedPositionTest.cpp
[modify] https://crrev.com/095851bc149483816fa9984a70081aa54e22e2df/third_party/WebKit/Source/core/exported/WebAssociatedURLLoaderImplTest.cpp
[modify] https://crrev.com/095851bc149483816fa9984a70081aa54e22e2df/third_party/WebKit/Source/core/frame/BrowserControlsTest.cpp
[modify] https://crrev.com/095851bc149483816fa9984a70081aa54e22e2df/third_party/WebKit/Source/core/html/ImageDataTest.cpp
[modify] https://crrev.com/095851bc149483816fa9984a70081aa54e22e2df/third_party/WebKit/Source/core/html/ImageDocumentTest.cpp
[modify] https://crrev.com/095851bc149483816fa9984a70081aa54e22e2df/third_party/WebKit/Source/core/imagebitmap/ImageBitmapTest.cpp
[modify] https://crrev.com/095851bc149483816fa9984a70081aa54e22e2df/third_party/WebKit/Source/core/layout/ng/inline/ng_inline_layout_algorithm_test.cc
[modify] https://crrev.com/095851bc149483816fa9984a70081aa54e22e2df/third_party/WebKit/Source/core/layout/ng/inline/ng_line_breaker_test.cc
[modify] https://crrev.com/095851bc149483816fa9984a70081aa54e22e2df/third_party/WebKit/Source/core/scheduler/VirtualTimeTest.cpp

Project Member

Comment 39 by bugdroid1@chromium.org, Oct 16 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/09cbee74049ecad80c597767435ec544c0bb301c

commit 09cbee74049ecad80c597767435ec544c0bb301c
Author: Daniel Bratell <bratell@opera.com>
Date: Mon Oct 16 15:27:53 2017

Add jumbo also for core/inspector generated files (-1 CPU minute)

Just clearing up minor targets that does not use the standard
templates for third_party/WebKit/Source/core and therefore did not get
jumbo automatically.

This removes about 1 CPU minute from the build effort, which is about
3% of the total cost of building blink/core (post-jumbo), and about
0.1% of the total effort building chrome+content_shell+blink_tests.

R=fs@opera.com

Bug:  713137 
Change-Id: Iba18116c3da677dd0c530b8db98327e587187368
Reviewed-on: https://chromium-review.googlesource.com/720961
Reviewed-by: Fredrik Söderquist <fs@opera.com>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#509050}
[modify] https://crrev.com/09cbee74049ecad80c597767435ec544c0bb301c/third_party/WebKit/Source/core/inspector/BUILD.gn

Project Member

Comment 40 by bugdroid1@chromium.org, Oct 17 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/c30d78054b24014aec2a4e47ffd83b5f472bed43

commit c30d78054b24014aec2a4e47ffd83b5f472bed43
Author: Bruce Dawson <brucedawson@chromium.org>
Date: Tue Oct 17 21:22:05 2017

Rename DictionaryTest to fix jumbo builds

blink::anonymous::DictionaryTest becomes ambiguous with
blink::DictionaryTest in jumbo builds. This renames DictionaryTest to
avoid the ambiguity.

Build errors look like:
third_party/WebKit/Source\core/testing/Internals.h(408): error C2872: 'DictionaryTest': ambiguous symbol
third_party/WebKit/Source\core/testing/Internals.h(53): note: could be 'blink::DictionaryTest'
third_party/WebKit/Source/bindings/core/v8/DictionaryTest.cpp(18): note: or       'blink::`anonymous-namespace'::DictionaryTest'

Bug:  713137 
Change-Id: Id13065afdb406808a6fa3a53b958ae976629d850
Reviewed-on: https://chromium-review.googlesource.com/724000
Reviewed-by: Jeremy Roman <jbroman@chromium.org>
Commit-Queue: Bruce Dawson <brucedawson@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509537}
[modify] https://crrev.com/c30d78054b24014aec2a4e47ffd83b5f472bed43/third_party/WebKit/Source/bindings/core/v8/DictionaryTest.cpp

I just got a report of an example of where jumbo builds are slowing down incremental builds significantly. This has always been an acknowledged possibility but I still thought it was worth documenting the specifics. These were the initial build settings:

dcheck_always_on = true
enable_nocompile_tests = true
is_component_build = true
is_debug = false
use_goma = true
symbol_level = 0

The build steps worked out to:

touch third_party\WebKit\Source\core\exported\WebViewTest.cpp
ninja -C out/Default webkit_unit_tests

Without jumbo the compilation and linking are both quite fast. With jumbo enabled the compilation increases dramatically, to ~28 s for the one file affected, and sometimes closer to 40 s. Since the link time (incremental linking) is just a few seconds the slowdown is at least 2x.

I reproed this on Windows but it was initially noticed on Linux where the slowdown was apparently 4x.

Setting jumbo_file_merge_limit to 20 didn't help - it made things worse. Setting it to 10 did help.

Setting jumbo_file_merge_limit to 10 meant that roughly 25% of the savings for full builds were lost, while giving much better incremental build times, without having to explicitly exclude the file in question.

FWIW.

Comment 42 by brat...@opera.com, Oct 20 2017

Sounds like there are one or some test files taking an awful long time to compile and if you get yourself grouped with that one, or those, you are "unlucky" and get hit hard. In this case I can see some, like V8ScriptValueSerializerTest, ActivityLoggerTest, BindingSecurityTest and V8ScriptValueSerializerTestForModules taking more than 10 seconds each to compile. That will be faster in jumbo, but probably still 5+ seconds each. 

There is a similar situation in v8 where some files take an awful long time to compile and if those end up in the same jumbo unit, that jumbo unit will be slow. For v8 it's extra unfortunate since v8 is often a build bottleneck and everything will be waiting for it to finish. In v8 we excluded those files that took 30s each from jumbo but it's still not great.

A 100% hypothetical system would know expected compilation time of each file and spread them out so that each jumbo unit compiled equally fast.

I hope to land a reduction from 100 to 50 today (we have to fix some things that don't compile with 50 first) which will make incremental builds faster on average, but it will not make it impossible to be grouped with slow-to-compile files and, as you noticed, you can even get more unlucky as the files are spread out differently.

Project Member

Comment 43 by bugdroid1@chromium.org, Oct 27 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/da3587161cce3f1b92c39cab72ee2fe9c482ab0c

commit da3587161cce3f1b92c39cab72ee2fe9c482ab0c
Author: Fredrik Söderquist <fs@opera.com>
Date: Fri Oct 27 17:32:33 2017

Deduplicate PaintScrollbar

CompositedLayerMapping.cpp and PaintLayerCompositor.cpp has almost
identical (differs by a null-check) PaintScrollbar helpers.
Move a cleaned up version to ScrollableAreaPainter (being the least bad
common location.)
This fixes Jumbo compilation.

Bug:  713137 
Cq-Include-Trybots: master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2
Change-Id: Icc36e57ed364b18716fd56b0363431f352ef0d72
Reviewed-on: https://chromium-review.googlesource.com/741595
Commit-Queue: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Xianzhu Wang <wangxianzhu@chromium.org>
Reviewed-by: Philip Rogers <pdr@chromium.org>
Reviewed-by: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#512224}
[modify] https://crrev.com/da3587161cce3f1b92c39cab72ee2fe9c482ab0c/third_party/WebKit/Source/core/paint/ScrollableAreaPainter.cpp
[modify] https://crrev.com/da3587161cce3f1b92c39cab72ee2fe9c482ab0c/third_party/WebKit/Source/core/paint/ScrollableAreaPainter.h
[modify] https://crrev.com/da3587161cce3f1b92c39cab72ee2fe9c482ab0c/third_party/WebKit/Source/core/paint/compositing/CompositedLayerMapping.cpp
[modify] https://crrev.com/da3587161cce3f1b92c39cab72ee2fe9c482ab0c/third_party/WebKit/Source/core/paint/compositing/PaintLayerCompositor.cpp

Comment 44 by brat...@opera.com, Mar 23 2018

Status: Fixed (was: Started)
More or less all of blink has supported jumbo for half a year to nine months. Countless trees, seals and developer minds have been saved in the process. Closing as fixed.
Project Member

Comment 45 by bugdroid1@chromium.org, Apr 3 2018

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/f1c4c94baf582e5dcb2d16619018a89ee5c34312

commit f1c4c94baf582e5dcb2d16619018a89ee5c34312
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Apr 03 09:05:39 2018

Narrowed scope of a UrlWithoutFragment method

In jumbo builds many files are compiled in the same
translation unit and then their anonymous namespaces will also
be one and the same. In blink/inspector two (identical)
UrlWithoutFragment functions end up colliding. This patch
changes it so that one of them has a more narrow scope.

The patch also removes the special casing of these files
from the build system.

Bug:  713137 
Change-Id: If0b8bfb8a13c1191aa4e1c07d2aa1472ed168f29
Reviewed-on: https://chromium-review.googlesource.com/985978
Reviewed-by: Pavel Feldman <pfeldman@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#547653}
[modify] https://crrev.com/f1c4c94baf582e5dcb2d16619018a89ee5c34312/third_party/WebKit/Source/core/inspector/BUILD.gn
[modify] https://crrev.com/f1c4c94baf582e5dcb2d16619018a89ee5c34312/third_party/WebKit/Source/core/inspector/InspectorPageAgent.cpp
[modify] https://crrev.com/f1c4c94baf582e5dcb2d16619018a89ee5c34312/third_party/WebKit/Source/core/inspector/InspectorPageAgent.h

Sign in to add a comment