New issue
Advanced search Search tips

Issue 786926 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2017
Cc:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 746956



Sign in to add a comment

Support jumbo in //cc

Project Member Reported by brat...@opera.com, Nov 20 2017

Issue description

The code in cc needs approximately 16 CPU minutes to compile when compiling chrome+content_shell+blink_tests (~2% of current master jumbo compilation). That could be 2.5 CPU minutes or less if the code supported jumbo compilation, making the build 1-2% faster. About 2 minutes on a 4 core/8 thread machine.


 

Comment 1 by brat...@opera.com, Nov 20 2017

Labels: jumbo

Comment 2 by brat...@opera.com, Nov 20 2017

I know of 5 code changes that are necessary to avoid having to exclude files from jumbo:

* Duplicate constants kNormalMaxItemsInCache, kThrottledMaxItemsInCache, kSuspendedMaxItemsInCache
 
* 2 RasterBufferImpl (suggest renaming one of them to ZeroCopyRasterBufferImpl)
 
* 2 ImageDecodeTaskImpl
 
* Identical functions named Children() in layer_list_iterator.cc and property_tree_builder.cc
 
* Too many kInitialResourceId
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 21 2017

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

commit 09412d8d0b82140c0941094c2058010607b5df6b
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Nov 21 10:44:39 2017

Inlined two RoundOutRect to avoid jumbo clash

Two identical helper functions named RoundOutRect clashed
in jumbo builds where they ended up in the same
translation unit. If they had been larger it would have been
better to find a common place to define the, then shared, function
but since they are small, inlining works.

Bug:  786926 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I11fdeb9ab1a9d063cb4f8b91f17c12ab250627a3
Reviewed-on: https://chromium-review.googlesource.com/779198
Reviewed-by: enne <enne@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#518207}
[modify] https://crrev.com/09412d8d0b82140c0941094c2058010607b5df6b/cc/paint/paint_op_buffer.cc
[modify] https://crrev.com/09412d8d0b82140c0941094c2058010607b5df6b/cc/paint/scoped_image_flags.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Nov 21 2017

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

commit 1bddcb33048b81c10729f406d1e184c1a1c2b3c6
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Nov 21 11:08:29 2017

Rename two RasterBufferImpl to make the names more unique

In jumbo builds (use_jumbo_build=true), cc files are merged so local
symbols, such as these RasterBufferImpl, will end up in the same
translation unit and prevent the code from compiling.

Having unique names have other benefits as well in debugging and
documentation so this CL renames them ZeroCopyRasterBufferImpl
and BitmapRasterBufferImpl.

Bug:  786926 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I0b0af89cc1c0b3581a18a685742ad3e8c6e6b429
Reviewed-on: https://chromium-review.googlesource.com/779119
Reviewed-by: enne <enne@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#518210}
[modify] https://crrev.com/1bddcb33048b81c10729f406d1e184c1a1c2b3c6/cc/raster/bitmap_raster_buffer_provider.cc
[modify] https://crrev.com/1bddcb33048b81c10729f406d1e184c1a1c2b3c6/cc/raster/zero_copy_raster_buffer_provider.cc

Comment 5 by brat...@opera.com, Nov 22 2017

Owner: brat...@opera.com
Status: Started (was: Untriaged)
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 22 2017

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

commit ad55d51dd41919ff7b04b1702e01173f32fdcdcb
Author: Daniel Bratell <bratell@opera.com>
Date: Wed Nov 22 20:37:02 2017

Support jumbo in //cc

Jumbo is the Chromium implementation of a unity build system where cc files are combined
before they are compiled. This adds support for that in //cc which can make it compile 8-9x
times "faster" by saving 10-15 CPU minutes (1-2% of the effort to
compile chrome+content_shell+blink_tests).

The mapping to wall time depends on the exact hardware. More parallel computers benefit less.

Bug:  786926 

Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I687873c09abbd27f7525426c943e5bb4e64748b4
Reviewed-on: https://chromium-review.googlesource.com/776801
Reviewed-by: enne <enne@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#518730}
[modify] https://crrev.com/ad55d51dd41919ff7b04b1702e01173f32fdcdcb/cc/cc.gni

Comment 7 by brat...@opera.com, Dec 7 2017

Status: Fixed (was: Started)
Code seems to be working fine. Closing as Fixed.
Project Member

Comment 8 by bugdroid1@chromium.org, Jan 2 2018

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

commit d0e666e982629b018b6cc13e7ab81a2e1da1bfc7
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Jan 02 19:56:51 2018

Rename ChildAt and Parent to get unique names.

To help jumbo compilations, this code has already renamed
Children -> LayerChildren so this follows the same pattern
to help some other jumbo configurations where the plurality
of ChildAt and Parent methods caused problems.

Bug:  786926 
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel
Change-Id: I086022931622b1bb670bc1313987ff69182c0990
Reviewed-on: https://chromium-review.googlesource.com/847473
Reviewed-by: enne <enne@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#526521}
[modify] https://crrev.com/d0e666e982629b018b6cc13e7ab81a2e1da1bfc7/cc/trees/property_tree_builder.cc

Sign in to add a comment