New issue
Advanced search Search tips

Issue 775547 link

Starred by 3 users

Issue metadata

Status: Started
Owner:
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Android , Windows , iOS , Chrome , Mac , Fuchsia
Pri: 3
Type: Bug

Blocked on:
issue 776313

Blocking:
issue 746956



Sign in to add a comment

compile //base with jumbo

Project Member Reported by thakis@chromium.org, Oct 17 2017

Issue description

It doesn't take a ton of time to build, but everything depends on it, and I build it often to try out various flags.

Also, it's an easy way for me to try out jumbofying a target.
 
Project Member

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

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

commit 2499aee158845e85799f9bbef03de427768bc043
Author: Nico Weber <thakis@chromium.org>
Date: Tue Oct 17 20:56:49 2017

jumboify base

Requires teaching the jumbo scripts about .S files.

Also fix some minor formatting issues in the jumbo docs.

On my Mac laptop, for release component builds:
* increases by 536% (from 3.3s to 21s) incremental build times
  of 'base', after touching base/strings/string16.cc.
* reduces by 65% (from 91s to 32s) full build times of 'base'

So this looks like something that's useful for full builds and
for playing with compiler flags (which requires full builds),
but probably not something for daily development.  I suppose
that's true of jumbo builds in general.

Bug: 775547
Change-Id: I9b5157682ee463e5d60499f4dc8815283a127729
Reviewed-on: https://chromium-review.googlesource.com/721139
Commit-Queue: Nico Weber <thakis@chromium.org>
Reviewed-by: Dirk Pranke <dpranke@chromium.org>
Reviewed-by: oysteine <oysteine@chromium.org>
Cr-Commit-Position: refs/heads/master@{#509524}
[modify] https://crrev.com/2499aee158845e85799f9bbef03de427768bc043/base/BUILD.gn
[modify] https://crrev.com/2499aee158845e85799f9bbef03de427768bc043/base/memory/discardable_memory_allocator.cc
[modify] https://crrev.com/2499aee158845e85799f9bbef03de427768bc043/base/metrics/persistent_histogram_allocator.cc
[modify] https://crrev.com/2499aee158845e85799f9bbef03de427768bc043/base/process/launch_win.cc
[modify] https://crrev.com/2499aee158845e85799f9bbef03de427768bc043/base/sync_socket_win.cc
[modify] https://crrev.com/2499aee158845e85799f9bbef03de427768bc043/base/task_scheduler/post_task.cc
[modify] https://crrev.com/2499aee158845e85799f9bbef03de427768bc043/base/threading/sequenced_task_runner_handle.cc
[modify] https://crrev.com/2499aee158845e85799f9bbef03de427768bc043/base/threading/thread_task_runner_handle.cc
[modify] https://crrev.com/2499aee158845e85799f9bbef03de427768bc043/base/trace_event/common/trace_event_common.h
[modify] https://crrev.com/2499aee158845e85799f9bbef03de427768bc043/base/trace_event/trace_event_argument.cc
[modify] https://crrev.com/2499aee158845e85799f9bbef03de427768bc043/base/trace_event/trace_log.cc
[modify] https://crrev.com/2499aee158845e85799f9bbef03de427768bc043/base/win/scoped_hstring.h
[modify] https://crrev.com/2499aee158845e85799f9bbef03de427768bc043/build/config/jumbo.gni
[modify] https://crrev.com/2499aee158845e85799f9bbef03de427768bc043/build/config/merge_for_jumbo.py
[modify] https://crrev.com/2499aee158845e85799f9bbef03de427768bc043/docs/jumbo.md

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

Cc: brat...@opera.com
Labels: jumbo
Project Member

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

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

commit 1b4aeea2b7a6b9414c99791cb203c9a79a0f8990
Author: Daniel Bratell <bratell@opera.com>
Date: Wed Oct 18 12:38:09 2017

Jumbo for //base - disable jumbo when cross compiling for nacl

Jumbo builds with nacl fails in the linker because of too many
instances of memory allocator functions like valloc, realloc, mallopt
when linking libnacl.a.

TBR=thakis@chromium.org,dpranke@chromium.org,oysteine@chromium.org

Bug: 775547
Change-Id: Ibeeb1caa355aa8014c4b9179f4f26e303f13fbf8
Reviewed-on: https://chromium-review.googlesource.com/725284
Commit-Queue: Daniel Bratell <bratell@opera.com>
Reviewed-by: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#509751}
[modify] https://crrev.com/1b4aeea2b7a6b9414c99791cb203c9a79a0f8990/base/BUILD.gn

Comment 4 by brat...@opera.com, Oct 18 2017

I hope this will make the bots go green again. Someone that knows more about nacl should probably make a better fix, but considering that nobody outside of Google delivers nacl that I know about, this will probably be good enough.

@thakis, if you want to look more at this, //base:i18n is probably trivial. I have it in my experimental tree with no code changes needed at all.

Comment 5 by brat...@opera.com, Oct 18 2017

Another thing, in lieu of cq bots, I often turn on jumbo in the CL, do a test run, and then disable it before checking in (important!). It's caught a few things in configurations I usually don't run myself.

Comment 6 by thakis@chromium.org, Oct 18 2017

I did base_i18n and base:test_support locally but it didn't help full build times at all and hurt incremental times, so I decided against landing those.

Comment 7 by brat...@opera.com, Oct 19 2017

Blockedon: 776313
Project Member

Comment 8 by bugdroid1@chromium.org, Nov 8 2017

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

commit efa1e600a9b3464acdc747c4c273faaf98d5f578
Author: Daniel Bratell <bratell@opera.com>
Date: Wed Nov 08 18:13:14 2017

Fix jumbo compilation for Android in //base

The define of "Long" to "long" ended up changing some "Long"
strings in jni code to "long" which broke compilation.

Bug: 775547
Change-Id: If9dab9f193b0cad5067903f46ade98155254c7de
Reviewed-on: https://chromium-review.googlesource.com/758366
Reviewed-by: danakj <danakj@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#514883}
[modify] https://crrev.com/efa1e600a9b3464acdc747c4c273faaf98d5f578/base/third_party/dmg_fp/README.chromium
[modify] https://crrev.com/efa1e600a9b3464acdc747c4c273faaf98d5f578/base/third_party/dmg_fp/dtoa_wrapper.cc

Project Member

Comment 9 by bugdroid1@chromium.org, Jan 11 2018

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

commit d9e3d22a24be439e91c8af394f3f58c64ae048c7
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Jan 11 19:20:40 2018

Include tcmalloc's config before tcmalloc.h

Certain things in tcmalloc.h depend on tcmalloc's config.h file,
for instance whether struct mallinfo exist or not. Depending
on how the functions are used, it may or may not work correctly
without including config.h, but to be safe, always include it.

This was an issue in jumbo builds if base/process/memory_linux.cc
and base/allocator/allocator_shim_default_dispatch_to_tcmalloc.cc
were compiled in the same translation unit. The wrong includes from
memory_linux.cc caused the other file to fail to compile.

Bug: 775547
Change-Id: Iab9b8172281c55b8cbfbc8c21a79f95d2bae5847
Reviewed-on: https://chromium-review.googlesource.com/860646
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#528704}
[modify] https://crrev.com/d9e3d22a24be439e91c8af394f3f58c64ae048c7/base/process/memory_linux.cc

Project Member

Comment 10 by bugdroid1@chromium.org, Jan 11 2018

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

commit 1108193a7a39b1615ef1dd0579b1e745ae3b36bf
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Jan 11 23:13:16 2018

Restore the UNICODE macro after third_party/ced has undefined it

third_party/ced (more specifically
third_party/ced/src/util/encodings/encodings.h) undefs UNICODE
which is a central define in Windows. It controls whether
Windows SDK headers should have 2 byte or 1 byte strings and must
always be set in a modern application.

In Jumbo builds this include of the dangerous ced header happen
in the same translation unit as the include of Windows headers
which will then fail to do the right thing.

Restoring the UNICODE define is a workaround.

Bug: 775547
Change-Id: I7323cf6f9fff3efe2780c3125b9b099a8f9faa09
Reviewed-on: https://chromium-review.googlesource.com/861883
Reviewed-by: Daniel Cheng <dcheng@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#528803}
[modify] https://crrev.com/1108193a7a39b1615ef1dd0579b1e745ae3b36bf/base/i18n/encoding_detection.cc

Project Member

Comment 11 by bugdroid1@chromium.org, Feb 22 2018

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

commit 87120183eccbb01b244288fd20d42cdaaf16d5b0
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Feb 22 19:07:26 2018

Rename a couple of g_instance to allow extreme jumbo

We just got two g_instance in //base and to allow the whole of
base to compile in a single translation unit (not a default
configuration even among jumbo users) it's better if they
have unique names.

Bug: 775547
Change-Id: Ie1a1a856932ae46bde754878108097a94e6e00c6
Reviewed-on: https://chromium-review.googlesource.com/931121
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#538512}
[modify] https://crrev.com/87120183eccbb01b244288fd20d42cdaaf16d5b0/base/feature_list.cc
[modify] https://crrev.com/87120183eccbb01b244288fd20d42cdaaf16d5b0/base/sampling_heap_profiler/sampling_heap_profiler.cc

Project Member

Comment 12 by bugdroid1@chromium.org, Feb 27 2018

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

commit 4b4bfc5848baa03343cba8c470df845968e42559
Author: Mostyn Bramley-Moore <mostynb@vewd.com>
Date: Tue Feb 27 13:08:58 2018

[jumbo] followup after 'tracing: Singleton -> NoDestructor and fix TSan test violation'

There are multiple g_instance_for_testing globals after
https://chromium-review.googlesource.com/c/chromium/src/+/935841 - let's
rename one of them to avoid jumbo build breakage.

TBR=primiano@chromium.org

Bug:  815234 ,775547
Change-Id: I184578b694e59dad8b06643db199f32820fd5887
Reviewed-on: https://chromium-review.googlesource.com/938821
Commit-Queue: Mostyn Bramley-Moore <mostynb@vewd.com>
Reviewed-by: Mostyn Bramley-Moore <mostynb@vewd.com>
Cr-Commit-Position: refs/heads/master@{#539424}
[modify] https://crrev.com/4b4bfc5848baa03343cba8c470df845968e42559/base/trace_event/trace_log.cc

Project Member

Comment 13 by bugdroid1@chromium.org, Feb 27 2018

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

commit c4e42a5f61e4728a06b85250a32b89f5821dc99f
Author: Hwanseung Lee <hs1217.lee@samsung.com>
Date: Tue Feb 27 17:25:23 2018

[jumbo] rename variables name for jumbo build.

a patch[1] which is to fix jumbo build error was merged.
but g_instance_for_testing still keep in memory_dump_manager.cc.
that variable name looks like generic name which is some potential
to make same issue.
so it should be rename in order to void the same problem.

[1]https://chromium-review.googlesource.com/938821

Bug: 775547
Change-Id: Id7f3fc30a8118fb6f24f0e920ab530f44a27fa82
Reviewed-on: https://chromium-review.googlesource.com/938803
Reviewed-by: Primiano Tucci <primiano@chromium.org>
Reviewed-by: Lei Zhang <thestig@chromium.org>
Commit-Queue: Lei Zhang <thestig@chromium.org>
Cr-Commit-Position: refs/heads/master@{#539480}
[modify] https://crrev.com/c4e42a5f61e4728a06b85250a32b89f5821dc99f/base/trace_event/memory_dump_manager.cc
[modify] https://crrev.com/c4e42a5f61e4728a06b85250a32b89f5821dc99f/base/trace_event/trace_log.cc

Sign in to add a comment