Reland of Add main lib section sizes to resource_sizes.py.

Reland of Add main lib section sizes to resource_sizes.py.

Reverted in: https://codereview.chromium.org/2673043002/.

Reason for reland: |_ExtractMainLibSectionSizesFromApk()| now handles
multiline readelf output and missing section headers.

This CL adds more detailed metrics for the main native library section
sizes. The new metrics are added under a new subtest: "_MainLibInfo".

BUG=681153
Review-Url: https://codereview.chromium.org/2673023003
Cr-Commit-Position: refs/heads/master@{#448229}
Committed: https://chromium.googlesource.com/chromium/src/+/b57a134073e5e119fdd498025050feda8c0c5353

[estevenson, 2017-02-03 16:44:06]

Description was changed from

==========
Reland of Add main lib section sizes to resource_sizes.py.

This CL adds more detailed metrics for the main native library section
sizes. The new metrics are added under a new subtest: "_MainLibInfo".

BUG=681153
==========

to

==========
Reland of Add main lib section sizes to resource_sizes.py.

Reverted in: https://codereview.chromium.org/2673043002/.

Reason for reland: |_ExtractMainLibSectionSizesFromApk()| now handles
multiline readelf output and missing section headers.

This CL adds more detailed metrics for the main native library section
sizes. The new metrics are added under a new subtest: "_MainLibInfo".

BUG=681153
==========

[estevenson, 2017-02-03 16:46:29]

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

[estevenson, 2017-02-03 16:46:30]

ptal Andrew!

I was thinking of adding more error handling but it probably isn't a good idea
since then we'll likely never notice places where the script doesn't work
properly right?

https://codereview.chromium.org/2673023003/diff/20001/build/android/resource_...
File build/android/resource_sizes.py (right):

https://codereview.chromium.org/2673023003/diff/20001/build/android/resource_...
build/android/resource_sizes.py:128: if section_name in section_sizes:
Another option would be to just skip the grouping and measure each section
independently, wdyt?

[agrieve, 2017-02-03 17:01:52]

https://codereview.chromium.org/2673023003/diff/20001/build/android/resource_...
File build/android/resource_sizes.py (right):

https://codereview.chromium.org/2673023003/diff/20001/build/android/resource_...
build/android/resource_sizes.py:128: if section_name in section_sizes:
On 2017/02/03 16:46:29, estevenson wrote:
> Another option would be to just skip the grouping and measure each section
> independently, wdyt?

This is actually already more broken out than what the other sizes.py does (it
has just two groups). Groups are important I think in case section names change
between GCC and clang (or per-architecture).

https://codereview.chromium.org/2673023003/diff/20001/build/android/resource_...
build/android/resource_sizes.py:141: # Matches [ 2] .dynsym DYNSYM 00000158
nit: add a comment saying multiline happens for 64 bit.

[estevenson, 2017-02-03 20:00:49]

ptal Andrew!

https://codereview.chromium.org/2673023003/diff/20001/build/android/resource_...
File build/android/resource_sizes.py (right):

https://codereview.chromium.org/2673023003/diff/20001/build/android/resource_...
build/android/resource_sizes.py:128: if section_name in section_sizes:
On 2017/02/03 17:01:52, agrieve wrote:
> On 2017/02/03 16:46:29, estevenson wrote:
> > Another option would be to just skip the grouping and measure each section
> > independently, wdyt?
> 
> This is actually already more broken out than what the other sizes.py does (it
> has just two groups). Groups are important I think in case section names
change
> between GCC and clang (or per-architecture).

Acknowledged.

https://codereview.chromium.org/2673023003/diff/20001/build/android/resource_...
build/android/resource_sizes.py:141: # Matches [ 2] .dynsym DYNSYM 00000158
On 2017/02/03 17:01:52, agrieve wrote:
> nit: add a comment saying multiline happens for 64 bit.

Added --wide instead.

https://codereview.chromium.org/2673023003/diff/40001/build/android/resource_...
File build/android/resource_sizes.py (right):

https://codereview.chromium.org/2673023003/diff/40001/build/android/resource_...
build/android/resource_sizes.py:111: 'other': ['.hash', '.init_array',
'.fini_array', '.comment',
If .hash is a symbol hash table, should it be in symbols?
http://www.skyfree.org/linux/references/ELF_Format.pdf

[agrieve, 2017-02-03 20:12:47]

lgtm!

https://codereview.chromium.org/2673023003/diff/40001/build/android/resource_...
File build/android/resource_sizes.py (right):

https://codereview.chromium.org/2673023003/diff/40001/build/android/resource_...
build/android/resource_sizes.py:111: 'other': ['.hash', '.init_array',
'.fini_array', '.comment',
On 2017/02/03 20:00:49, estevenson wrote:
> If .hash is a symbol hash table, should it be in symbols?
> http://www.skyfree.org/linux/references/ELF_Format.pdf

Yes! Great point!

https://codereview.chromium.org/2673023003/diff/40001/build/android/resource_...
build/android/resource_sizes.py:133: print "Unknown elf section header:",
section_header
Great idea!

[estevenson, 2017-02-03 20:31:27]

On 2017/02/03 20:12:47, agrieve wrote:
> lgtm!
> 
>
https://codereview.chromium.org/2673023003/diff/40001/build/android/resource_...
> File build/android/resource_sizes.py (right):
> 
>
https://codereview.chromium.org/2673023003/diff/40001/build/android/resource_...
> build/android/resource_sizes.py:111: 'other': ['.hash', '.init_array',
> '.fini_array', '.comment',
> On 2017/02/03 20:00:49, estevenson wrote:
> > If .hash is a symbol hash table, should it be in symbols?
> > http://www.skyfree.org/linux/references/ELF_Format.pdf
> 
> Yes! Great point!
> 
>
https://codereview.chromium.org/2673023003/diff/40001/build/android/resource_...
> build/android/resource_sizes.py:133: print "Unknown elf section header:",
> section_header
> Great idea!

Thanks for the reviews! Going to wait until Monday to submit this as well :p.

[estevenson, 2017-02-06 10:24:09]

The CQ bit was checked by estevenson@chromium.org

[estevenson, 2017-02-06 10:24:10]

The patchset sent to the CQ was uploaded after l-g-t-m from agrieve@chromium.org
Link to the patchset: https://codereview.chromium.org/2673023003/#ps60001
(title: ".hash -> symbols")

[commit-bot: I haz the power, 2017-02-06 10:24:30]

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-02-06 11:34:26]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1486376649831900,
"parent_rev": "bce01de43097d69f127f2637d9c3398ff780462a", "commit_rev":
"b57a134073e5e119fdd498025050feda8c0c5353"}

[commit-bot: I haz the power, 2017-02-06 11:35:02]

Description was changed from

==========
Reland of Add main lib section sizes to resource_sizes.py.

Reverted in: https://codereview.chromium.org/2673043002/.

Reason for reland: |_ExtractMainLibSectionSizesFromApk()| now handles
multiline readelf output and missing section headers.

This CL adds more detailed metrics for the main native library section
sizes. The new metrics are added under a new subtest: "_MainLibInfo".

BUG=681153
==========

to

==========
Reland of Add main lib section sizes to resource_sizes.py.

Reverted in: https://codereview.chromium.org/2673043002/.

Reason for reland: |_ExtractMainLibSectionSizesFromApk()| now handles
multiline readelf output and missing section headers.

This CL adds more detailed metrics for the main native library section
sizes. The new metrics are added under a new subtest: "_MainLibInfo".

BUG=681153

Review-Url: https://codereview.chromium.org/2673023003
Cr-Commit-Position: refs/heads/master@{#448229}
Committed:
https://chromium.googlesource.com/chromium/src/+/b57a134073e5e119fdd498025050...
==========

[commit-bot: I haz the power, 2017-02-06 11:35:03]

Committed patchset #4 (id:60001) as
https://chromium.googlesource.com/chromium/src/+/b57a134073e5e119fdd498025050...