New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 771153 link

Starred by 1 user

Issue metadata

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

Blocking:
issue 746956



Sign in to add a comment

Support jumbo in mojo generated build targets

Project Member Reported by brat...@opera.com, Oct 3 2017

Issue description

Mojo generates a lot of build targets and code and those targets is a surprisingly large part of the compile time of chrome+content_shell+blink_tests. Adding jumbo to mojo.gni seems to reduce the compilation time by 42 CPU minutes, which is -7% (same reduction in wall time on a 4c/8t machine).

I was surprised by that reduction because I didn't know that mojo generated targets used 42 CPU minutes to begin with. There might be something wrong there. Expensive mojo compilation could also be a contributing factor to the increasing build time since mojo is being used more and more.
 

Comment 1 by brat...@opera.com, Oct 3 2017

I should clarify:
-7% compared to my WIP branch which has jumbo support for code that is not jumbified in master.
It's -5% compared to master with use_jumbo_build=true

Comment 2 by brat...@opera.com, Oct 3 2017

Owner: brat...@opera.com
Project Member

Comment 3 by bugdroid1@chromium.org, Oct 4 2017

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

commit af2403b78b4b9414c00ac6ebcb2fb02e46ce2680
Author: Daniel Bratell <bratell@opera.com>
Date: Wed Oct 04 09:41:50 2017

Remove random string in mojo typemap file

There was a random sources block in image_filter.typemap which
broke jumbo since it has asserts that sources contains nothing
random. Normal builds ignored the sources entry since it didn't
match any toolchain.

Bug:  771153 
Change-Id: I4b801ae350807701740946ca5ea5984c4086b61e
Reviewed-on: https://chromium-review.googlesource.com/697207
Reviewed-by: Stephen White <senorblanco@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#506350}
[modify] https://crrev.com/af2403b78b4b9414c00ac6ebcb2fb02e46ce2680/skia/public/interfaces/image_filter.typemap

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 4 2017

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

commit edb0d9c623226c23f5ec6006cc36ac8544f5ab55
Author: Daniel Bratell <bratell@opera.com>
Date: Wed Oct 04 10:49:43 2017

Removing "using namespace" in a global scope in translate

"using namespace" in a global scope breaks jumbo builds (and
are not allowed in the code style guide). This patch replaces them
with explicit namespaces.

Bug:  771153 
Change-Id: Iea234b0e02636b198af20b0f2b6ad53e735ebf7b
Reviewed-on: https://chromium-review.googlesource.com/697704
Commit-Queue: Daniel Bratell <bratell@opera.com>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Hajime Hoshi <hajimehoshi@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506361}
[modify] https://crrev.com/edb0d9c623226c23f5ec6006cc36ac8544f5ab55/components/translate/content/common/translate_struct_traits.cc

Project Member

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

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

commit 5e58cfdacffae439b76919e7f2ef726cb55ce3f5
Author: Daniel Bratell <bratell@opera.com>
Date: Wed Oct 04 11:48:18 2017

Removing "using namespace" in a global scope in autofill

"using namespace" in a global scope breaks jumbo builds (and
are not allowed in the code style guide). This patch replaces them
with explicit namespaces.

Bug:  771153 
Change-Id: Ic45f740d8081b9f4e531b30ac05f6578a404de11
Reviewed-on: https://chromium-review.googlesource.com/697371
Commit-Queue: Daniel Bratell <bratell@opera.com>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Sebastien Seguin-Gagnon <sebsg@chromium.org>
Cr-Commit-Position: refs/heads/master@{#506367}
[modify] https://crrev.com/5e58cfdacffae439b76919e7f2ef726cb55ce3f5/components/autofill/content/common/autofill_types_struct_traits.cc

Project Member

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

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

commit 470dd1cc00034c05f906e886e2e30ecef6cbb73b
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Oct 05 18:33:31 2017

Removing strange sources from mojo typemap

This patch removes a random sources block that was ignored
in normal builds but triggered an assert in jumbo builds.

This also changes the OWNERS file according to presubmit instructions.

Bug:  771153 
Change-Id: Ic32f92db25b1b4bd6e2087b95785b4934a94e120
Reviewed-on: https://chromium-review.googlesource.com/702258
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Steven Bennetts <stevenjb@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#506801}
[modify] https://crrev.com/470dd1cc00034c05f906e886e2e30ecef6cbb73b/ui/message_center/mojo/OWNERS
[modify] https://crrev.com/470dd1cc00034c05f906e886e2e30ecef6cbb73b/ui/message_center/mojo/notification.typemap

Project Member

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

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

commit bc165a2c103f77c117b9092a2fa6d504b8e77ce2
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Oct 05 20:00:08 2017

Removing "using namespace" in a global scope in password_manager

"using namespace" in a global scope breaks jumbo builds (and
are not allowed in the code style guide). This patch replaces them
with explicit namespaces.

Bug:  771153 
Change-Id: If9c4de9e115c5531708cf06f64cb0088daca2d57
Reviewed-on: https://chromium-review.googlesource.com/697370
Reviewed-by: Vadym Doroshenko <dvadym@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#506837}
[modify] https://crrev.com/bc165a2c103f77c117b9092a2fa6d504b8e77ce2/components/password_manager/content/common/credential_manager_struct_traits.cc

Project Member

Comment 8 by bugdroid1@chromium.org, Oct 6 2017

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

commit 62ef46de73fe5714147b5af24947dff9d015e887
Author: Daniel Bratell <bratell@opera.com>
Date: Fri Oct 06 15:05:45 2017

Qualify the blink namespace in mojo so that mojo::blink is not used

There is a mojo::blink namespace generated from
ui/platform_window/mojo/text_input_state.mojom and if that one is
visible then the string "blink" will be resolved to the wrong
namespace inside namespace mojo.

This fixes android jumbo compilation (only android generates
blink bindings from the mojom file mentioned above).

Bug:  771153 
Change-Id: I923e3bebe4fac3e02fe27c60b4472275f9613df8
Reviewed-on: https://chromium-review.googlesource.com/703654
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#507070}
[modify] https://crrev.com/62ef46de73fe5714147b5af24947dff9d015e887/third_party/WebKit/Source/platform/mojo/FetchAPIRequestStructTraits.h
[modify] https://crrev.com/62ef46de73fe5714147b5af24947dff9d015e887/third_party/WebKit/public/platform/AddressSpaceStructTraits.h
[modify] https://crrev.com/62ef46de73fe5714147b5af24947dff9d015e887/third_party/WebKit/public/platform/ContentSecurityPolicyStructTraits.h

Project Member

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

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

commit 6190165700c744187b3de8082341705f2f58be5e
Author: Daniel Bratell <bratell@opera.com>
Date: Mon Oct 09 16:53:30 2017

Support jumbo for mojo generated code (-42 CPU minutes)

Mojo generates a lot of code that is expensive to compile and
compiling them through jumbo saves roughly 80-90% of that effort,
which is 42 CPU minutes on the reference machine (surprising that 
mojo code needs that long to compile).

42 CPU minutes is -7% of the current "fastest" compilation, or
-4.5% of current master use_jumbo_build=true. The wall clock effect
will be similar for a 4c/8t machine but smaller the more cores
the computer have.

This patch includes a change to jumbo.gni to handle targets 
with no sources, and streamlines targets with only one file, 
because both happen through the mojo template.

Bug:  771153 
Change-Id: Ia11a709711600f1b6eedd098bf47c1df2239a7a5
Reviewed-on: https://chromium-review.googlesource.com/697664
Reviewed-by: Bruce Dawson <brucedawson@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#507393}
[modify] https://crrev.com/6190165700c744187b3de8082341705f2f58be5e/build/config/jumbo.gni
[modify] https://crrev.com/6190165700c744187b3de8082341705f2f58be5e/mojo/public/tools/bindings/mojom.gni
[modify] https://crrev.com/6190165700c744187b3de8082341705f2f58be5e/third_party/WebKit/Source/core/mojo/BUILD.gn

Comment 10 by brat...@opera.com, Oct 10 2017

Status: Fixed (was: Started)
mojo generated sources now support jumbo compilation and it should have saved another 10 minutes of compilation on a 4 core machine.
Project Member

Comment 11 by bugdroid1@chromium.org, Oct 13 2017

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

commit 65c9ac8db53ee49af6ba853ac69a4d9cff502bff
Author: Daniel Bratell <bratell@opera.com>
Date: Fri Oct 13 07:24:24 2017

Change mojo module from "mojo" to "ui.mojom" because of conflicts

Having the mojo module be "mojo" means that code appear inside the
C++ namespace "mojo" which then can collide with other code.

This happened while preparing for jumbo compiling mojo code and
caused compilation errors in jumbo because of a mojo::blink
namespace colliding with the ::blink namespace.

Since there already was an ui.mojom.TextInputType from ime, this changes
the code to merge the two TextInputType types.

Bug:  771153 
Change-Id: Ic88ed6932a2d64f35ffb7661eedde09dc4619691
Reviewed-on: https://chromium-review.googlesource.com/704581
Reviewed-by: Scott Violet <sky@chromium.org>
Reviewed-by: Sadrul Chowdhury <sadrul@chromium.org>
Reviewed-by: Yuzhu Shen <yzshen@chromium.org>
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Reviewed-by: Ken Rockot <rockot@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#508634}
[modify] https://crrev.com/65c9ac8db53ee49af6ba853ac69a4d9cff502bff/services/ui/public/interfaces/ime/BUILD.gn
[modify] https://crrev.com/65c9ac8db53ee49af6ba853ac69a4d9cff502bff/services/ui/public/interfaces/ime/ime.mojom
[modify] https://crrev.com/65c9ac8db53ee49af6ba853ac69a4d9cff502bff/services/ui/public/interfaces/ime/ime_struct_traits.cc
[modify] https://crrev.com/65c9ac8db53ee49af6ba853ac69a4d9cff502bff/services/ui/public/interfaces/ime/ime_struct_traits_test.mojom
[modify] https://crrev.com/65c9ac8db53ee49af6ba853ac69a4d9cff502bff/services/ui/public/interfaces/window_tree.mojom
[modify] https://crrev.com/65c9ac8db53ee49af6ba853ac69a4d9cff502bff/services/ui/ws/window_tree.cc
[modify] https://crrev.com/65c9ac8db53ee49af6ba853ac69a4d9cff502bff/services/ui/ws/window_tree.h
[modify] https://crrev.com/65c9ac8db53ee49af6ba853ac69a4d9cff502bff/ui/aura/mus/input_method_mus.cc
[modify] https://crrev.com/65c9ac8db53ee49af6ba853ac69a4d9cff502bff/ui/aura/mus/window_port_mus.cc
[modify] https://crrev.com/65c9ac8db53ee49af6ba853ac69a4d9cff502bff/ui/aura/mus/window_port_mus.h
[modify] https://crrev.com/65c9ac8db53ee49af6ba853ac69a4d9cff502bff/ui/aura/mus/window_tree_client.cc
[modify] https://crrev.com/65c9ac8db53ee49af6ba853ac69a4d9cff502bff/ui/aura/mus/window_tree_client.h
[modify] https://crrev.com/65c9ac8db53ee49af6ba853ac69a4d9cff502bff/ui/aura/test/mus/test_window_tree.cc
[modify] https://crrev.com/65c9ac8db53ee49af6ba853ac69a4d9cff502bff/ui/aura/test/mus/test_window_tree.h
[modify] https://crrev.com/65c9ac8db53ee49af6ba853ac69a4d9cff502bff/ui/platform_window/mojo/ime_type_converters.cc
[modify] https://crrev.com/65c9ac8db53ee49af6ba853ac69a4d9cff502bff/ui/platform_window/mojo/ime_type_converters.h
[modify] https://crrev.com/65c9ac8db53ee49af6ba853ac69a4d9cff502bff/ui/platform_window/mojo/text_input_state.mojom

Sign in to add a comment