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

Issue 856628 link

Starred by 7 users

Issue metadata

Status: WontFix
Owner:
Closed: Jul 10
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 3
Type: Bug

Blocked on:
issue 858903
issue 859936

Blocking:
issue 752720
issue 860762



Sign in to add a comment

Remove NaCl toolchains and pull (P)NaCl binaries from GCS

Project Member Reported by thakis@chromium.org, Jun 26 2018

Issue description

We currently build some files with nacl compilers. Those are old and unlikely to be updated, and nacl is going away eventually, so let's try see if it's possible to fork the files built with nacl compilers into the nacl repo, and then only build the (frozen) forked files with the nacl compilers. Then we could C++17 in src to our heart's content.

Rough plan:

1. I think a necessary first step is to figure out exactly which files are getting built with nacl compilers. Maybe there's some neat way to query gn to print all uses of toolchains, but the low-tech way is to add a "echo $in" to the compile commands in the nacl toolchain.
(Here https://cs.chromium.org/chromium/src/build/toolchain/nacl_toolchain.gni?q=nacl_toolchain+file:%5C.gni&sq=package:chromium&dr=C&l=16 => https://cs.chromium.org/chromium/src/build/toolchain/gcc_toolchain.gni?type=cs&q=gcc_toolchain+file:%5C.gni&sq=package:chromium&g=0&l=280 (and cxx a few lines down). With some luck doing something like `echo = "" if (is_nacl) { echo = "echo {{source}}; " and then inserting "$echo" at the front of `command` works; if not I'd copy gcc_toolchain.gni to gcc_nacl_toolchain.gni , make the change there, and make nacl_toolchain use that new gni file). Also, verify which of the nacl compilers is used for each file (maybe echo $cc too.)

2. Then, figure out which targets these are in (base, net, more? I think maybe webrtc?) and which targets depend on these getting build via nacl (hangouts plugin? chromoting?). `gn path` can do this for us, https://groups.google.com/a/chromium.org/forum/#!searchin/gn-dev/nico$20weber%7Csort:date/gn-dev/n_Mm5Y0is2E/9cMfJ9DgBgAJ has a full example). 

3. At this point, we should have a good enough idea what exactly would need to happen that we can probably write a short design doc: "Problem: We want to use c++17. NaCL uses compilers A/B/C that don't support that and that we won't update since NaCL is going away. They're used to build chrome targets T/U/V, so we can't use C++17 in these targets (and using different C++ standards in different targets is confusing.) Solution: Fork targets T/U/V into the nacl repo, and make things depending on these nacl targets depend on the forked targets, and then never touch those forked targets again. Now nothing in src/ is build by nacl compilers, we can use C++17, and the forked files will go away eventually. Consequences: Things depending on the nacl irt won't get new changes in these targets (say, quic evolution)"

3a. Prototype copying the targets over just to make sure we can actually make this work.

4. Once we have that, get sign-off of the projects having the nacl dependencies that they can live with this, and we can send out an intent-to-implement mail with a link to it

5. land change from 3a: add new files in nacl repo, roll nacl, change chrome build files to use new location
 
Here's step 1 done.  Attached is:
build_log.txt.gz: Full build log with gn args "is_component_build = true enable_nacl = true is_debug = false use_goma = true".  Uncompressed 192M.
nacl.txt: Only the nacl parts (things that use native_client/toolchain).
files.txt: The deduplicated list of source files built by nacl toolchains.
nexe.txt: List of all built *.nexe files.

There's 3012 source files and 156 nexe's.  I have a proposal.  Rather than fork all of these source files, how about we instead upload all of the *.nexe's to GCS (looks like they're all statically linked, even on component builds)?  We could do this by creating a nacl branch in Chromium (from where master is right now) that's dedicated just to building these files.  If we ever want to update the nacl binaries, we would have to do the work on this branch (which hopefully should be never) and reupload the *.nexe's.  Back on the master branch, we would get rid of all the nacl toolchains and just pull the required binaries from GCS.

I feel like this would be easier than copying a bunch of source and build files and figuring out dependencies and such.  It could also be done in a more incremental way, eg. one nexe at a time.  But I'd like to hear what you guys think.
build_log.txt.gz
3.6 MB Download
nacl.txt
2.8 MB View Download
files.txt
145 KB View Download
nexe.txt
10.6 KB View Download

Comment 2 by thakis@chromium.org, Jun 26 2018

Awesome, thanks!

Looks like:
base/
chrome/test/data/nacl/
components/nacl
2 files in content
crypto
2 files in device
gpu/command_buffer
ipc
jingle
media/base (just a few files)
mojo/edk and mojo/public
net/
ppapi/
remoting
sandbox
services/service_manager
testing
third_party/boringssl
third_party/ced
third_party/expat
third_party/googletest
third_party/icu
third_party/libjingle
third_party/libjpeg_turbo
third_party/libvpx
third_party/libyuv
third_party/modp_b64 (1 file)
third_party/opus
third_party/protobuf
third_party/webrtc
ui/events (1 file)
ui/gfx
url

Urgh, that's more than I expected, especially more third_party stuff. Is that all pulled in from multiple nexes, or just one? Which?


Re your proposal: That's what I thought we would do, but I talked to jschuh a bit and he had this forking idea, and it seems a bit nicer to me since then it's still possible to change things if needed. With the prebuilts, you basically have to sync to mid-2018 code to rebuild nacl stuff if we change things, as you say. Do you think copying files over is the difficult part? That seems reasonably straightforward to me (and we need to do the build untangling when we delete nacl one day anyways).
> Is that all pulled in from multiple nexes, or just one? Which?

I'll get back to you on that soon.

> With the prebuilts, you basically have to sync to mid-2018 code to rebuild nacl stuff if we change things, as you say.

Yup, but I think it won't actually be that bad since
1. We hope not to do this.
2. My plan was to have a script on the nacl branch that builds and uploads all of the nexe's so it wouldn't be that painful to change things.  I think this is cleaner than having duplicates of base etc that are also out-of-sync.  It also means devs and the bots would get to skip building up to 3012 object files and DSOs, a potentially significant speedup.

> Do you think copying files over is the difficult part?

My main concern is the build files.  We would have to figure out which targets the source files correspond to (the easy part) and copy just those targets into the new build files.  This partitioning sounds like the difficult part to me.  It sounds like a very manual process.  If we were to pull the binaries from GCS, it turns from an additive process into a subtractive one: we would simply delete the old targets rather than copying the targets + dependencies + gni files etc.

It would also allow us to clean up a lot of the nacl build code today instead of in 2020.  Every is_nacl condition could be removed.  This is appealing to me since nacl always seems to be a pain point when making build config changes.

I'd still be open to forking if you think that's the right way to go, but plmk what you think.
> Every is_nacl condition could be removed.

Never mind that.  We'd be able to do that with the forking method as well.
One more correction, it's not 156 nexe's since 48 of those were just the unstripped versions.  So really 108 nexe's.
> Is that all pulled in from multiple nexes, or just one? Which?

Looks like the big one is nacl_irt which has 1280 source files and a bunch of deps here
https://cs.chromium.org/chromium/src/ppapi/native_client/BUILD.gn?rcl=c5d723af5052482663ff2e3df9caceef187e7390&l=16

But there's also several other nexe's that have >100 source files.

attachments:
num_sources.txt: List of nexe's and how many source files it takes to build it
nexe_sources.tar.gz: For each nexe, a list of its source files
num_sources.txt
9.1 KB View Download
nexe_sources.tar.gz
35.7 KB Download

Comment 7 by thakis@chromium.org, Jun 27 2018

nacl_irt doesn't depend on webrtc as far as I can see, what pulls that in?

The main thing I worry about is that we're going to freeze some protocols and whoever ships things depending on nacl_irt / these protocols may not be fine with that. This includes quic and maybe webrtc; I can't think of much else. This likely isn't an issue, but it's the only interesting part I can think of that might sink this endeavor.

I don't think it matters too much if we do prebuilts or copy files over; if you prefer prebuilts a lot that works for me.
Blockedon: 858903
Blockedon: 859936
Blocking: 860762
Cc: thakis@chromium.org
Owner: thomasanderson@chromium.org
Status: Started (was: Untriaged)
Summary: Remove NaCl toolchains and pull (P)NaCl binaries from GCS (was: Try to fork files in src compiled by nacl compilers into nacl repo)
Project Member

Comment 12 by bugdroid1@chromium.org, Jul 9

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/tools/build/+/d85ecf2447eb1c29d92627a69a070f68cebbb5c7

commit d85ecf2447eb1c29d92627a69a070f68cebbb5c7
Author: Tom Anderson <thomasanderson@chromium.org>
Date: Mon Jul 09 16:59:41 2018

Apply arm config to chromeos-daisy-rel

Needed for:
https://chromium-review.googlesource.com/c/chromium/src/+/1124968

BUG= 856628 

Change-Id: Ida33210f3da05920665e82067664d0f4c5011e05
Reviewed-on: https://chromium-review.googlesource.com/1125092
Reviewed-by: Ben Pastene <bpastene@chromium.org>
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Thomas Anderson <thomasanderson@chromium.org>

[modify] https://crrev.com/d85ecf2447eb1c29d92627a69a070f68cebbb5c7/scripts/slave/recipe_modules/chromium_tests/chromium_chromiumos.py

@thakis: When you say "whoever ships things depending on nacl_irt / these protocols may not be fine with that", you might be misunderstanding what NaCl IRT does.  It's not the people shipping things depending on the IRT or on these protocols (such as Chrome IPC interfaces) who might object, it's the people maintaining the implementations of those protocols in Chrome who might object.

The IRT contains a copy of the PPAPI proxy, which depends on various Chrome internals, including interfaces exposed via Chrome IPC.  It provides the PPAPI interface (a stable ABI, function-call-based) to NaCl apps.  All NaCl apps depend on the IRT.

The IRT is basically the only nexe that needs to be versioned in sync with the rest of Chrome.  NaCl apps don't need to be versioned in sync with Chrome, so they could be turned into prebuilt nexes much more easily.
How does the IRT talk to chrome?
Chrome IPC and shared memory.
So it's

nacl_app <==ppapi==> IRT <==chrome ipc==> chrome

where nacl_app is a nexe, the IRT is also a nexe, and chrome is a native binary?

Doesn't the PPAPI code live in chrome too? Does the IRT have its own copy? Why isn't the IRT <=> chrome communication done via ppapi too? Is there a doc somewhere that explains this the overall picture in some more detail? 
Think of the IRT as a DLL injected by chrome in the nacl_app to provide the ppapi implementation by proxying to other chrome processes, it is effectively part of chrome (the same way the blink v8 bindings are part of chrome and provide the implementation of the web API, by proxying to other processes).

PPAPI is an API in the traditional sense, a set of functions called by the nacl app. To implement the API it needs to proxy to other processes (renderer, browser), and this is what the ppapi proxy does. To answer your questions:
- "Doesn't the PPAPI code live in chrome too?". The ppapi code (the chrome implementation of the ppapi API) is in the chrome repository. It has plugin-side code and host-side code which talk to each other over chrome IPC / shm. The host side code is built exactly once, natively, and runs in the renderer and browser processes. The plugin-side code is built both natively (for native plugins such as Flash) and for NaCl (as part of the IRT), and runs in the plugin processes.
- "Does the IRT have its own copy?" it has its own build of the plugin-side code from the same source, the code itself is identical (with maybe a few ifdefs).
- "Why isn't the IRT <=> chrome communication done via ppapi too?" They live in different processes (well, with different architectures too), so they need to talk over IPC. PPAPI is not an IPC.
- "Is there a doc somewhere that explains this the overall picture in some more detail?" There is this: http://dev.chromium.org/developers/design-documents/pepper-plugin-implementation , I'm not sure if it covers the grounds you care about.
Status: WontFix (was: Started)
Based on recent comments, it seems forking and/or prebuilt binaries is a non-option for various folks.  So I'm closing this issue out.

Sign in to add a comment