//tools/binary_size: Record packed .rel.dyn section size

//tools/binary_size: Record packed .rel.dyn section size

Adds --apk-file flag to archive action. When passed, section sizes are
taken from the .so within the .apk rather than from the one in
lib.unstripped.

Also removes "symbols" from console.py, which was missed in
previous rename commit. The reason to not expose this top-level
is that it in the future, we don't want to also expose java_symbols,
pak_symbols, etc.

BUG=681694
Review-Url: https://codereview.chromium.org/2807343005
Cr-Commit-Position: refs/heads/master@{#464271}
Committed: https://chromium.googlesource.com/chromium/src/+/2b71e084f0d11b7aee47096cbbd2144931760028

[agrieve, 2017-04-12 02:36:13]

Description was changed from

==========
//tools/binary_size: Record packed .rel.dyn section size

Adds --apk-file flag to archive action. When passed, section sizes are
taken from the .so within the .apk rather than from the one in
lib.unstripped.

BUG=681694
==========

to

==========
//tools/binary_size: Record packed .rel.dyn section size

Adds --apk-file flag to archive action. When passed, section sizes are
taken from the .so within the .apk rather than from the one in
lib.unstripped.

Also removes "symbols" from console.py, which was missed in
previous rename commit. The reason to not expose this top-level
is that it in the future, we don't want to also expose java_symbols,
pak_symbols, etc.

BUG=681694
==========

[agrieve, 2017-04-12 14:05:58]

agrieve@chromium.org changed reviewers:
+ estevenson@chromium.org

[agrieve, 2017-04-12 14:06:17]

On 2017/04/12 14:05:58, agrieve wrote:
> mailto:agrieve@chromium.org changed reviewers:
> + mailto:estevenson@chromium.org

🚪

[estevenson, 2017-04-12 16:54:42]

lgtm

https://codereview.chromium.org/2807343005/diff/1/tools/binary_size/libsupers...
File tools/binary_size/libsupersize/archive.py (right):

https://codereview.chromium.org/2807343005/diff/1/tools/binary_size/libsupers...
tools/binary_size/libsupersize/archive.py:465: assert lib_infos, 'APK has not
.so files to measure.'
nit: awkward wording.

https://codereview.chromium.org/2807343005/diff/1/tools/binary_size/libsupers...
tools/binary_size/libsupersize/archive.py:467: #     secondary architectures.
_SectionSizesFromApk() would fail for 64 bit monochrome right now, correct?
Might be worth it to add this note to the README, though I'd assume it's pretty
uncommon for people to regularly build 64 bit chrome..

I wasn't able to verify this properly, I ran:

tools/binary_size/supersize archive output.size --elf-file
out/Release-64/lib.unstripped/libmonochrome.so --apk-file
out/Release-64/apks/MonochromePublic.apk

and hit the "One or more source file paths could not be found" assert.

Most errors were v8 like this:
Could not find source path for obj/v8/v8_base/assembler-arm64.o

And some of these:
W   4201 no obj basename for obj/third_party/libvpx/libvpx.a(loopfilter_neon.o)
W   4201 Could not find source path for
obj/third_party/libvpx/libvpx.a(loopfilter_neon.o)

[agrieve, 2017-04-12 19:37:46]

https://codereview.chromium.org/2807343005/diff/1/tools/binary_size/libsupers...
File tools/binary_size/libsupersize/archive.py (right):

https://codereview.chromium.org/2807343005/diff/1/tools/binary_size/libsupers...
tools/binary_size/libsupersize/archive.py:465: assert lib_infos, 'APK has not
.so files to measure.'
On 2017/04/12 16:54:42, estevenson wrote:
> nit: awkward wording.

Done.

https://codereview.chromium.org/2807343005/diff/1/tools/binary_size/libsupers...
tools/binary_size/libsupersize/archive.py:467: #     secondary architectures.
On 2017/04/12 16:54:42, estevenson wrote:
> _SectionSizesFromApk() would fail for 64 bit monochrome right now, correct?
> Might be worth it to add this note to the README, though I'd assume it's
pretty
> uncommon for people to regularly build 64 bit chrome..
> 
> I wasn't able to verify this properly, I ran:
> 
> tools/binary_size/supersize archive output.size --elf-file
> out/Release-64/lib.unstripped/libmonochrome.so --apk-file
> out/Release-64/apks/MonochromePublic.apk
> 
> and hit the "One or more source file paths could not be found" assert.
> 
> Most errors were v8 like this:
> Could not find source path for obj/v8/v8_base/assembler-arm64.o
> 
> And some of these:
> W   4201 no obj basename for
obj/third_party/libvpx/libvpx.a(loopfilter_neon.o)
> W   4201 Could not find source path for
> obj/third_party/libvpx/libvpx.a(loopfilter_neon.o)

Didn't hit any of those other errors, but good catch on not testing with arm64
(nor with crazy.).

I've not tested on both ChromeModernPublic, and with 64-bit monochrome_public.

[estevenson, 2017-04-12 19:52:11]

still lgtm

[agrieve, 2017-04-13 00:10:17]

The CQ bit was checked by agrieve@chromium.org

[commit-bot: I haz the power, 2017-04-13 00:11:22]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2017-04-13 03:20:43]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1492042217340280,
"parent_rev": "169f23ffda76c9c000b3ff2366a0ff9efa5abb03", "commit_rev":
"2b71e084f0d11b7aee47096cbbd2144931760028"}

[commit-bot: I haz the power, 2017-04-13 03:21:38]

Description was changed from

==========
//tools/binary_size: Record packed .rel.dyn section size

Adds --apk-file flag to archive action. When passed, section sizes are
taken from the .so within the .apk rather than from the one in
lib.unstripped.

Also removes "symbols" from console.py, which was missed in
previous rename commit. The reason to not expose this top-level
is that it in the future, we don't want to also expose java_symbols,
pak_symbols, etc.

BUG=681694
==========

to

==========
//tools/binary_size: Record packed .rel.dyn section size

Adds --apk-file flag to archive action. When passed, section sizes are
taken from the .so within the .apk rather than from the one in
lib.unstripped.

Also removes "symbols" from console.py, which was missed in
previous rename commit. The reason to not expose this top-level
is that it in the future, we don't want to also expose java_symbols,
pak_symbols, etc.

BUG=681694

Review-Url: https://codereview.chromium.org/2807343005
Cr-Commit-Position: refs/heads/master@{#464271}
Committed:
https://chromium.googlesource.com/chromium/src/+/2b71e084f0d11b7aee47096cbbd2...
==========

[commit-bot: I haz the power, 2017-04-13 03:21:39]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/chromium/src/+/2b71e084f0d11b7aee47096cbbd2...