New issue
Advanced search Search tips

Issue 891061 link

Starred by 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Nov 2
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug

Blocking:
issue 890891



Sign in to add a comment

Support outlining in Supersize (aka, fix supersize for arm64)

Project Member Reported by estevenson@chromium.org, Oct 1

Issue description

Needed for 64 bit Chrome now that we use outlining (see  issue 890891 ).

Added in clang: https://chromium-review.googlesource.com/c/chromium/src/+/1236174

 
Labels: -Pri-2 Pri-1
Bumping priority since this makes arm64 unusable
Summary: Support outlining in Supersize (aka, fix supersize for arm64) (was: Support outlining in Supersize)
This seems problematic only for LLD *without* ThinLTO. Results (arm64) from running supersize archive:

* LLD:     11.5 min to run, creating a 15.05 MB .size file.
* LLD-LTO:  1.3 min to run, creating a 11.56 MB .size file.

And the reason for the difference:
* For the LLD flow, .o files contain OUTERLINED_FUNCTION_* symbols, which are retrieved by obj_analyzer._RunNm() and accumulated into |self._paths_by_name|, then later, |object_paths_by_name| in archive.py.
* For the LLD-LTO flow, named .o files are LLVM BitCode files and don't contain OUTLINED_FUNCTION_*. These symbols appear inside thinlto-cache/llvmcache-* files (ELF), which we don't process.
* Numbers:
  * LLD     => num_aliases_created=159049221
  * LLD-LTO => num_aliases_created=757409
* So LLD-LTO flow is also broken, in that OUTLINED_FUNCTION_* symbols (obtained from .map files) are missing object_path and source_path.

For testing I was thinking of adding support to LLD .map file first (Bug 899337). But this seems rather involved! Maybe it's better to just get the fix out ASAP and get arm64 bots up, and worry about testing later?

Yeah, I think it would be fairly involved. maybe just add a couple OUTLINED symbols to the gold map file for now?
Sure I can update the Gold file.

I've found a complication with my current solution: Some regular symbols disappear! After some tracing, looks like:

* In the .map file, at most 1 OUTLINED_FUNCTION_* appear per address.
* In llvm-nm output on libchrome.so, an address can have multiple OUTLINED_FUNCTION_* field, mixed with named symbols.
* It's possible for OUTLINED_FUNCTION_* symbol in the .map file to mask named symbols.

So if we simply ignore OUTLINED_FUNCTION_* in nm.py, then the masked named symbols would not show up. But if we include everything, we'd have many OUTLINED_FUNCTION_*; for arm64 LLD-LTO, .map file has 61158 OUTLINED_FUNCTION_, whereas llvm-nm output has 127235!

The many copies of OUTLINED_FUNCTION_* occupying the same address likely come from all over the place. I'm not sure if it's worthwhile to keep the .pss for each of them though, so I propose we:

* Keep OUTLINED_FUNCTION_* population limited to one per address: So read them from .map file. Rename these as '** outlined function'
* When reading results from llvm-nm on libchrome.so, for each address, collapse all OUTLINED_FUNCTION_ instances into one '** outlined function' symbol.
  * Knowing the count would be interesting, but realistically it seems we don't need it?
  * If named symbols are present, the PSS will treat '** outlined function' as a single entity, and divided accordingly.

Plan sgtm!
Project Member

Comment 10 by bugdroid1@chromium.org, Nov 2

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

commit cf38354217d8ed125c69d77f1872500f6b9814ca
Author: Samuel Huang <huangs@chromium.org>
Date: Fri Nov 02 15:05:16 2018

[SuperSize] Add support for outlining and fix arm64 flow.

Outlining in LLD creates many symbols with names OUTLINED_FUNCTION_0,
OUTLINED_FUNCTION_1, etc. that appear many times. Problems:
* Unrelated symbols can share the same name. If object path information
  is available in the .map file (true for arm64 without ThinLTO), then
  SuperSize would run 10x as long because these paths get joined in an
  O(n^2) fashion.
* In nm output, an address can have many OUTLINED_FUNCTION_* symbols,
  which adds to the bloat.

This CL adds to SuperSize outlining support to solves the problems above.
Details:
* link_map_parser.py: (for LLD, and also Gold for testing) Detect
  OUTLINED_FUNCTION_* and reassign the name to '** outlined function'.
  The '*' prefix labels these symbols as placeholders, so these
  symbols will be ignored by nm results from .o files, and no object
  path joining occurs.
* nm.py: For the nm call to the main ELF file, combine multiple
  OUTLINED_FUNCTION_* with the same address into a single symbol, and
  assign the name to '** outlined function * (count)'. If combination
  takes place, then "forced passing" occurs, i.e., the name will
  always be passed (rather than discarded to save space) to
  |names_by_address|.
* archive.py: Updated to reconcile the two sources of
  '** outlined function' symbols.

Outlined functions can share address with named symbols. Due to the
combining in nm.py, this causes number of aliased symbols to decrease
for affected named symbols (and increases PSS).

'** outlined function' symbols can have object_path assigned, but the
data are obtained from .map files only, since these symbols are ignored
for nm results from .o files. Furthermore, if an outlined function
symbol shares an address with any other such symbol (as found by nm
results for the main ELF file), then it won't have object_path assigned
(since "forced passing" above causes _AddNmAliases() to process the
symbol, but this discards object_path data from .map files).

Bug:  891061 
Change-Id: Ie0109c7fc876749c10a87b5a7673af2be49d38c4
Reviewed-on: https://chromium-review.googlesource.com/c/1312561
Reviewed-by: Samuel Huang <huangs@chromium.org>
Reviewed-by: agrieve <agrieve@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Cr-Commit-Position: refs/heads/master@{#604918}
[modify] https://crrev.com/cf38354217d8ed125c69d77f1872500f6b9814ca/tools/binary_size/README.md
[modify] https://crrev.com/cf38354217d8ed125c69d77f1872500f6b9814ca/tools/binary_size/libsupersize/archive.py
[modify] https://crrev.com/cf38354217d8ed125c69d77f1872500f6b9814ca/tools/binary_size/libsupersize/linker_map_parser.py
[modify] https://crrev.com/cf38354217d8ed125c69d77f1872500f6b9814ca/tools/binary_size/libsupersize/nm.py
[modify] https://crrev.com/cf38354217d8ed125c69d77f1872500f6b9814ca/tools/binary_size/libsupersize/testdata/Archive.golden
[modify] https://crrev.com/cf38354217d8ed125c69d77f1872500f6b9814ca/tools/binary_size/libsupersize/testdata/Archive_Apk.golden
[modify] https://crrev.com/cf38354217d8ed125c69d77f1872500f6b9814ca/tools/binary_size/libsupersize/testdata/Archive_Elf.golden
[modify] https://crrev.com/cf38354217d8ed125c69d77f1872500f6b9814ca/tools/binary_size/libsupersize/testdata/Archive_OutputDirectory.golden
[modify] https://crrev.com/cf38354217d8ed125c69d77f1872500f6b9814ca/tools/binary_size/libsupersize/testdata/Archive_Pak_Files.golden
[modify] https://crrev.com/cf38354217d8ed125c69d77f1872500f6b9814ca/tools/binary_size/libsupersize/testdata/Console.golden
[modify] https://crrev.com/cf38354217d8ed125c69d77f1872500f6b9814ca/tools/binary_size/libsupersize/testdata/Csv.golden
[modify] https://crrev.com/cf38354217d8ed125c69d77f1872500f6b9814ca/tools/binary_size/libsupersize/testdata/Diff_Basic.golden
[modify] https://crrev.com/cf38354217d8ed125c69d77f1872500f6b9814ca/tools/binary_size/libsupersize/testdata/Diff_NullDiff.golden
[modify] https://crrev.com/cf38354217d8ed125c69d77f1872500f6b9814ca/tools/binary_size/libsupersize/testdata/FullDescription.golden
[modify] https://crrev.com/cf38354217d8ed125c69d77f1872500f6b9814ca/tools/binary_size/libsupersize/testdata/SymbolGroupMethods.golden
[modify] https://crrev.com/cf38354217d8ed125c69d77f1872500f6b9814ca/tools/binary_size/libsupersize/testdata/mock_toolchain/mock_nm.py
[modify] https://crrev.com/cf38354217d8ed125c69d77f1872500f6b9814ca/tools/binary_size/libsupersize/testdata/test.map

Status: Fixed (was: Assigned)
Project Member

Comment 12 by bugdroid1@chromium.org, Nov 6

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

commit 193d7c53fbea604b92ebdcf19562e95400f5a7e3
Author: Samuel Huang <huangs@chromium.org>
Date: Tue Nov 06 18:51:39 2018

[Build Bots] Re-enable SuperSize for arm64 build bots.

This CL essentialy reverts
  https://chromium-review.googlesource.com/c/chromium/tools/build/+/1255550/ ,
and reruns './recipes.py test train'.

Previously SuperSize didn't support outlined symbols for LLD (891061),
thereby causing arm64 (no ThinLTO) to run 10x as long and time out
(890891). Now that outlined symbol support has been added to SuperSize,
we can re-enable running it for arm64 build bots.

Bug:  891061 , 890891 
Change-Id: I1e83506576c07d14fab51b2ed9dc1a606aec327f
Reviewed-on: https://chromium-review.googlesource.com/c/1318290
Reviewed-by: John Budorick <jbudorick@chromium.org>
Commit-Queue: Samuel Huang <huangs@chromium.org>
Auto-Submit: Samuel Huang <huangs@chromium.org>

[modify] https://crrev.com/193d7c53fbea604b92ebdcf19562e95400f5a7e3/scripts/slave/README.recipes.md
[modify] https://crrev.com/193d7c53fbea604b92ebdcf19562e95400f5a7e3/scripts/slave/recipes/android/builder.py
[modify] https://crrev.com/193d7c53fbea604b92ebdcf19562e95400f5a7e3/scripts/slave/recipes/android/builder.expected/full_chromium_perf_Android_arm64_Builder_Perf.json

Sign in to add a comment