New issue
Advanced search Search tips

Issue 891576 link

Starred by 1 user

Issue metadata

Status: Available
Owner: ----
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 3
Type: Bug



Sign in to add a comment

SuperSize: Attribute inline symbols to .h files

Project Member Reported by agrieve@chromium.org, Oct 3

Issue description

SuperSize is currently only able to use .cc files as source_paths. 
To deal with symbols defined in .h files, it creates a symbol alias for each .cc file that includes the symbol.

>>> s = size_info.symbols.WhereIsNative().Filter(lambda s:s.num_aliases > 2 and s.full_name == s.aliases[0].full_name and s.full_name == s.aliases[-1].full_name)
>>> s.size
606352
>>> len(s)
88146
>>> Print(s.Sorted(key=lambda s:-s.size)[:4])
...
0)        637 (25.0%) t@0x19438fc  637 (size=5102) $root_gen_dir/chrome/common/page_load_metrics/page_load_metrics.mojom.cc
             blink::mojom::internal::WebFeature_Data::IsKnownValue (num_aliases=8)
1)       1275 (50.0%) t@0x19438fc  637 (size=5102) $root_gen_dir/chrome/common/page_load_metrics/page_load_metrics.mojom-shared.cc
             blink::mojom::internal::WebFeature_Data::IsKnownValue (num_aliases=8)
2)       1913 (75.0%) t@0x19438fc  637 (size=5102) $root_gen_dir/content/common/service_worker/service_worker_container.mojom.cc
             blink::mojom::internal::WebFeature_Data::IsKnownValue (num_aliases=8)
3)       2551 (100.0%) t@0x19438fc  637 (size=5102) $root_gen_dir/content/common/shared_worker/shared_worker_client.mojom.cc
             blink::mojom::internal::WebFeature_Data::IsKnownValue (num_aliases=8)




Rather than store these symbols as aliases, we should consider trying to set the source_path to the .h file for symbols with > ## aliases (maybe > 3?).

To do this, we could run addr2line (or llvm-symbolizer?) for each alias group to retrieve the .h file from the debug information. e.g.:

../../third_party/android_ndk/toolchains/arm-linux-androideabi-4.9/prebuilt/linux-x86_64/bin/arm-linux-androideabi-addr2line -e lib.unstripped/libmonochrome.so 19438fc
./gen/third_party/blink/public/platform/web_feature.mojom-shared-internal.h:28


Given that these symbols make up only 600kb of size, this task is probably low priority, but wanted to get the idea down.
 
Project Member

Comment 1 by bugdroid1@chromium.org, Dec 4

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

commit 76da8c7057d89bee93be5828790eb27119d2e056
Author: Samuel Huang <huangs@chromium.org>
Date: Tue Dec 04 19:38:07 2018

[Supersize] Simplify linker_map_parser.py by removing _SymbolMaker.

MapFileParserLld.Parse() traverses 3 indent levels:
* Level 1 with indent 0: Sections names.
* Level 2 with indent 8: Object path (may be '<internal>') and .o
  section name.
* Level 3 with indent 16: Symbol names.

Previously _SymbolMaker manages LLD .map file parsing by providing the
following services:
* Maintain a 'current symbol' that's modified when new data (perhaps
  at a deeper layer) are found.
* Flush(): On detecting new symbol, stores the old 'current symbol' to
  to start anew (perhaps on emerging to a shallower layer).
* Create(): Creates new 'current symbol'.

Unfortunately, the code assumes that each contiguous Level 3 block
corresponds to only one symbol. But to fix Bug 891576, we will need to
break this assumption, and _SymbolMaker gets in the way!

This CL remove _SymbolMaker without changing Supersize output. It turns
out that since Create() is only called once (in Level 2), we don't
really need Flush(). The code can be simplified by keeping a |syms|
list, and directly modifying |syms[-1]|.

Also add --dump argument to linker_map_parser.py's stand-alone parsing
mode, to ensure identical results before and after this CL.

Bug: 891576
Change-Id: I34b879c7e816049ec55f90d30f84213f6eb1acf9
Reviewed-on: https://chromium-review.googlesource.com/c/1361591
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@{#613647}
[modify] https://crrev.com/76da8c7057d89bee93be5828790eb27119d2e056/tools/binary_size/libsupersize/linker_map_parser.py

Oops the CL in #1 should refer to 892648.

Sign in to add a comment