New issue
Advanced search Search tips

Issue 898475 link

Starred by 2 users

Issue metadata

Status: Started
Owner:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 921328

Blocking:
issue 746956



Sign in to add a comment

Support jumbo in //ppapi

Project Member Reported by brat...@opera.com, Oct 24

Issue description

Jumbo is a unity build system for Chromium (see https://chromium.googlesource.com/chromium/src/+/lkgr/docs/jumbo.md ) which merges cc files at the build target level.

The code in //ppapi needs roughly 15 CPU minutes to compile in my measurements, which is 2.3% of the total jumbo build time and one of the largest remaining non-thirdparty code blocks without jumbo support.

A big part of the compiled code is generated sources which do contain clashing symbols. I'm uncertain how easy it will be to fix those, but there is other code as well.
 
Blocking: 746956
Project Member

Comment 2 by bugdroid1@chromium.org, Oct 25

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

commit eb5dcdedd067d2dc7c993687adcbb5d1649b76b1
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Oct 25 08:39:50 2018

[ppapi] Merged three RunCallback methods doing the same thing

In jumbo build experiments, the different RunCallback functions
ended up in the same translation unit and thus the same
anonymous namespace where the symbols clashed. Since they,
and VideoDecoderResource::RunCallbackWithError had the same
implementation, this merges the functions into a shared
SafeRunCallback.

Bug: 898475
Change-Id: I3ee20972794d3c6ef73e49882e1ceba24c487e19
Reviewed-on: https://chromium-review.googlesource.com/c/1297427
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#602646}
[modify] https://crrev.com/eb5dcdedd067d2dc7c993687adcbb5d1649b76b1/ppapi/proxy/audio_encoder_resource.cc
[modify] https://crrev.com/eb5dcdedd067d2dc7c993687adcbb5d1649b76b1/ppapi/proxy/plugin_resource.cc
[modify] https://crrev.com/eb5dcdedd067d2dc7c993687adcbb5d1649b76b1/ppapi/proxy/plugin_resource.h
[modify] https://crrev.com/eb5dcdedd067d2dc7c993687adcbb5d1649b76b1/ppapi/proxy/video_decoder_resource.cc
[modify] https://crrev.com/eb5dcdedd067d2dc7c993687adcbb5d1649b76b1/ppapi/proxy/video_encoder_resource.cc

Project Member

Comment 3 by bugdroid1@chromium.org, Oct 25

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

commit d5850cc240451492fe91ebfee12e4f5dfcf36468
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Oct 25 08:40:55 2018

[ppapi] Renamed two global variables and types in ppapi/proxy

In jumbo build experiments, the different
InstanceToDispatcherMap types and g_instance_to_dispatcher
globals ended up in the same translation unit and thus the same
anonymous namespace where the symbols clashed. This just renames
them as Dispatcher -> PluginDispatcher/HostDispatcher.

Bug: 898475
Change-Id: Ieae4310fdbe7ea27954011964d2bb6fa4d83e338
Reviewed-on: https://chromium-review.googlesource.com/c/1297145
Reviewed-by: Antoine Labour <piman@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#602647}
[modify] https://crrev.com/d5850cc240451492fe91ebfee12e4f5dfcf36468/ppapi/proxy/host_dispatcher.cc
[modify] https://crrev.com/d5850cc240451492fe91ebfee12e4f5dfcf36468/ppapi/proxy/plugin_dispatcher.cc

Project Member

Comment 4 by bugdroid1@chromium.org, Oct 25

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

commit 59d3fd6aa036223e9002b2b215a98cc9fe73bc16
Author: Daniel Bratell <bratell@opera.com>
Date: Thu Oct 25 18:15:13 2018

[ppapi] undef MemoryBarrier on Windows to avoid atomicops clash

MemoryBarrier is a macro in some versions of the Win32 API and it's
also a name in the base/atomicops.h API. If windows.h is included
after atomicops.h, further use of the atomicops API will fail
unless MemoryBarrier is undefined. See base/atomicops.h for more info.

Bug: 898475
Change-Id: I4a4be93d534f54a5758dc09d1b1e52c1f2bee79c
Reviewed-on: https://chromium-review.googlesource.com/c/1299139
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Bill Budge <bbudge@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#602793}
[modify] https://crrev.com/59d3fd6aa036223e9002b2b215a98cc9fe73bc16/ppapi/utility/threading/lock.h
[modify] https://crrev.com/59d3fd6aa036223e9002b2b215a98cc9fe73bc16/ppapi/utility/threading/simple_thread.h

Project Member

Comment 5 by bugdroid1@chromium.org, Nov 13

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

commit 0781150b862721e58fac33a2b7f0ef8c4e734d0a
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Nov 13 16:05:20 2018

[ppapi] Merged two identical interface_name

In jumbo build experiments, two cc files both defined
interface_name<PPB_InputEvent_1_0>() which clashed since in
jumbo builds many cc files are compiled in the same translation unit.

This moves the shared code to a shared file.

Bug: 898475
Change-Id: I9ce58e837d663cb5c15ba2596c22812a41def4c0
Reviewed-on: https://chromium-review.googlesource.com/c/1297148
Reviewed-by: Bill Budge <bbudge@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Sam Clegg <sbc@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#607601}
[modify] https://crrev.com/0781150b862721e58fac33a2b7f0ef8c4e734d0a/native_client_sdk/src/libraries/ppapi_cpp/library.dsc
[modify] https://crrev.com/0781150b862721e58fac33a2b7f0ef8c4e734d0a/ppapi/cpp/BUILD.gn
[modify] https://crrev.com/0781150b862721e58fac33a2b7f0ef8c4e734d0a/ppapi/cpp/input_event.cc
[add] https://crrev.com/0781150b862721e58fac33a2b7f0ef8c4e734d0a/ppapi/cpp/input_event_interface_name.h
[modify] https://crrev.com/0781150b862721e58fac33a2b7f0ef8c4e734d0a/ppapi/cpp/instance.cc
[modify] https://crrev.com/0781150b862721e58fac33a2b7f0ef8c4e734d0a/ppapi/cpp/module_impl.h

Project Member

Comment 6 by bugdroid1@chromium.org, Nov 20

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

commit 96fcc576315260ebb67480a6ef2e38e66503b098
Author: Daniel Bratell <bratell@opera.com>
Date: Tue Nov 20 14:12:39 2018

Jumbo support for ppapi (-7.5 CPU minutes)

Jumbo is a unity build system for Chromium (see
https://chromium.googlesource.com/chromium/src/+/lkgr/docs/jumbo.md )
which merges cc files at the build target level.

The code in //ppapi needs roughly 15 CPU minutes to compile in my
measurements, which is 2.3% of the total jumbo build time and one of
the largest remaining non-thirdparty code blocks without jumbo support.

This saves about 7.5 CPU minutes in a jumbo build, which is
a bit over 1% of the total reference build time. It doesn't cover
thunk which is most of the remaining compile time. There are
too much generated code with reused symbol names in thunk for it
to be trivially supported right now.

Bug: 898475
Change-Id: I1ee592578ac765cbd58cb856fcd5f331d3dd8c50
Reviewed-on: https://chromium-review.googlesource.com/c/1297366
Reviewed-by: Antoine Labour <piman@chromium.org>
Reviewed-by: Bill Budge <bbudge@chromium.org>
Commit-Queue: Daniel Bratell <bratell@opera.com>
Cr-Commit-Position: refs/heads/master@{#609687}
[modify] https://crrev.com/96fcc576315260ebb67480a6ef2e38e66503b098/ppapi/cpp/BUILD.gn
[modify] https://crrev.com/96fcc576315260ebb67480a6ef2e38e66503b098/ppapi/host/BUILD.gn
[modify] https://crrev.com/96fcc576315260ebb67480a6ef2e38e66503b098/ppapi/proxy/BUILD.gn
[modify] https://crrev.com/96fcc576315260ebb67480a6ef2e38e66503b098/ppapi/shared_impl/BUILD.gn

Blockedon: 921328

Sign in to add a comment