New issue
Advanced search Search tips

Issue 886902 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Sep 28
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocking:
issue 746953



Sign in to add a comment

Support jumbo builds in content/shell

Project Member Reported by brat...@opera.com, Sep 19

Issue description

jumbo builds are more efficient by compiling many files in a single translation unit. Most of the code (in compile time) now supports it, but there are still some meaningful exceptions. The code in content/shell compiles in about 13 CPU minutes, which could be much less with jumbo, saving 1-2% of the total compile effort.
 
Blocking: 746953
Project Member

Comment 2 by bugdroid1@chromium.org, Sep 20

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

commit 6da6478c56416ae6e45406516c8a265e7cb4bbe5
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Sep 20 07:55:55 2018

Remove "using namespace" from content_shell code

"using namespace" is not allowed per the Code Style Guide, but
worse, much much much worse, it won't work in jumbo builds because
they will be in a different context where a compiler error
will be triggered.

Bug:  886902 
Change-Id: I635ceb8ec5d65a9d599ecd38bf0c150c90edf056
Reviewed-on: https://chromium-review.googlesource.com/1233837
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#592717}
[modify] https://crrev.com/6da6478c56416ae6e45406516c8a265e7cb4bbe5/content/shell/renderer/layout_test/test_media_stream_renderer_factory.cc
[modify] https://crrev.com/6da6478c56416ae6e45406516c8a265e7cb4bbe5/content/shell/test_runner/test_runner.cc
[modify] https://crrev.com/6da6478c56416ae6e45406516c8a265e7cb4bbe5/content/shell/test_runner/test_runner_for_specific_view.cc
[modify] https://crrev.com/6da6478c56416ae6e45406516c8a265e7cb4bbe5/content/shell/test_runner/web_test_interfaces.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Sep 20

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

commit ffecbc0f99048d45af37a8b8dc018299bb8b4d4d
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Sep 20 19:37:13 2018

Give content_shell message headers include guards

This is in preparation for jumbo compiling content/shell.

IPC headers generate code by being included multiple times with
different macros. This requires careful juggling of inclusions
and jumbo is not careful with when headers are included. In most
of the code this has already been solved by adding include guards
but not in content/shell until this patch.

Bug:  886902 
Change-Id: I84644fba628a827dfd7825d97844aacca3a9cd8e
Reviewed-on: https://chromium-review.googlesource.com/1233743
Commit-Queue: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Cr-Commit-Position: refs/heads/master@{#592905}
[modify] https://crrev.com/ffecbc0f99048d45af37a8b8dc018299bb8b4d4d/content/shell/common/layout_test/OWNERS
[modify] https://crrev.com/ffecbc0f99048d45af37a8b8dc018299bb8b4d4d/content/shell/common/layout_test/layout_test_messages.cc
[modify] https://crrev.com/ffecbc0f99048d45af37a8b8dc018299bb8b4d4d/content/shell/common/layout_test/layout_test_messages.h
[modify] https://crrev.com/ffecbc0f99048d45af37a8b8dc018299bb8b4d4d/content/shell/common/shell_messages.cc
[modify] https://crrev.com/ffecbc0f99048d45af37a8b8dc018299bb8b4d4d/content/shell/common/shell_messages.h

Project Member

Comment 4 by bugdroid1@chromium.org, Sep 21

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

commit 74789b0b5edbade95d81579a344bd6de005dba33
Author: Daniel Bratell <bratell@opera.com>
Date: Fri Sep 21 08:50:40 2018

[jumbo] Resolve symbol clashes in content/shell

Rename two TestServiceImpl to not clash in jumbo build experiments,
and change a kIllegalString constant to be shared.

Bug:  886902 
Change-Id: I513e0641834476bb198cf24d71ef088634458313
Reviewed-on: https://chromium-review.googlesource.com/1233709
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#593125}
[modify] https://crrev.com/74789b0b5edbade95d81579a344bd6de005dba33/content/shell/renderer/shell_content_renderer_client.cc
[modify] https://crrev.com/74789b0b5edbade95d81579a344bd6de005dba33/content/shell/test_runner/test_common.cc
[modify] https://crrev.com/74789b0b5edbade95d81579a344bd6de005dba33/content/shell/test_runner/test_common.h
[modify] https://crrev.com/74789b0b5edbade95d81579a344bd6de005dba33/content/shell/test_runner/web_frame_test_client.cc
[modify] https://crrev.com/74789b0b5edbade95d81579a344bd6de005dba33/content/shell/utility/shell_content_utility_client.cc

Project Member

Comment 5 by bugdroid1@chromium.org, Sep 21

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

commit 9aa4227330b98d6cc72613c20b35d281f2686803
Author: Daniel Bratell <bratell@opera.com>
Date: Fri Sep 21 10:54:27 2018

Gave some content_shell constants unique names and better docs

content_shell wants to replicate a chome constant at several
places which caused collisions in jumbo build experiments. This
give the replicas unique names and fixes the documentation which
was a bit outdated.

Bug:  886902 
Change-Id: Ibf65de50bb2f938392fcecc910d70a4c4182c87d
Reviewed-on: https://chromium-review.googlesource.com/1233715
Commit-Queue: Daniel Bratell <bratell@opera.com>
Reviewed-by: Dmitry Gozman <dgozman@chromium.org>
Reviewed-by: Peter Beverloo <peter@chromium.org>
Cr-Commit-Position: refs/heads/master@{#593143}
[modify] https://crrev.com/9aa4227330b98d6cc72613c20b35d281f2686803/chrome/browser/devtools/devtools_ui_bindings.cc
[modify] https://crrev.com/9aa4227330b98d6cc72613c20b35d281f2686803/content/shell/browser/layout_test/devtools_protocol_test_bindings.cc
[modify] https://crrev.com/9aa4227330b98d6cc72613c20b35d281f2686803/content/shell/browser/shell_devtools_bindings.cc

Project Member

Comment 6 by bugdroid1@chromium.org, Sep 25

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

commit a7f3c5f4381470735f0618be6a45f294ee54032b
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Sep 25 12:58:27 2018

Support jumbo compilation for content/shell (-11 CPU minutes)

Compiling content/shell does in tests consume 13.5 CPU minutes which
is brought down to 2 CPU minutes with jumbo compilation. Jumbo
compilation works by combining many cc files in a single translation
unit, thereby avoiding recompilations of headers and re-instantiation
of templates, as well as by reducing the amount of work needed by the
linker.

Unfortunately IPC message generators can not be combined with other
code since they use some headers under very special circumstances.

Bug:  886902 
Change-Id: I4ceee107ea64b774c9f540047182dbcfcf3c487f
Reviewed-on: https://chromium-review.googlesource.com/1238413
Reviewed-by: Peter Beverloo <peter@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#593905}
[modify] https://crrev.com/a7f3c5f4381470735f0618be6a45f294ee54032b/content/shell/BUILD.gn
[modify] https://crrev.com/a7f3c5f4381470735f0618be6a45f294ee54032b/content/shell/test_runner/BUILD.gn

Status: Fixed (was: Started)

Sign in to add a comment