//tools/binary_size: Group [clone] and ** symbols

//tools/binary_size: Group [clone] and ** symbols

Two types of groups:
1. Those that end with "[clone .part.#]"
2. Star symbols (e.g.: "** symbol gap 3")

To handle grouped symbols:
* Print(recursive=True) added
* Diff() updated to be recursive
* Tests updated

For the .size files that motivated this change, the diff before had:
    355 symbols added (+), 2964 changed (~), 300 removed (-)
and after:
    345 symbols added (+), 2975 changed (~), 290 removed (-)

I was hoping for a bigger reduction, but it turns out the majority of
the "noise" is actually from different functions being inlined.

This change also changes map2size.py from positional arguments to
having: --elf-file, --output-file. It also adds --no-source-paths.
These flags make it obvious that the tool can still be used with only
a .map file.

BUG=681694, 708865
Review-Url: https://codereview.chromium.org/2809043003
Cr-Commit-Position: refs/heads/master@{#463678}
Committed: https://chromium.googlesource.com/chromium/src/+/f70d47db0b462dbc08d4d6ae2e633e4e5b0af518

[agrieve, 2017-04-11 13:40:50]

Description was changed from

==========
//tools/binary_size: Group [clone] and ** symbols

Two types of groups:
1. Those that end with "[clone .part.#]"
2. Star symbols (e.g.: "** symbol gap 3")

To handle grouped symbols:
* Print(recursive=True) added
* Diff() updated to be recursive
* Tests updated

For the .size files that motivated this change, the diff before had:
    355 symbols added (+), 2964 changed (~), 300 removed (-)
and after:
    345 symbols added (+), 2975 changed (~), 290 removed (-)

I was hoping for a bigger reduction, but it turns out the majority of
the "noise" is actually from different functions being inlined.

This change also changes map2size.py from positional arguments to
having: --elf-file, --output-file. It also adds --no-source-paths.
These flags make it obvious that the tool can still be used with only
a .map file.

BUG=681694,708865
==========

to

==========
//tools/binary_size: Group [clone] and ** symbols

Two types of groups:
1. Those that end with "[clone .part.#]"
2. Star symbols (e.g.: "** symbol gap 3")

To handle grouped symbols:
* Print(recursive=True) added
* Diff() updated to be recursive
* Tests updated

For the .size files that motivated this change, the diff before had:
    355 symbols added (+), 2964 changed (~), 300 removed (-)
and after:
    345 symbols added (+), 2975 changed (~), 290 removed (-)

I was hoping for a bigger reduction, but it turns out the majority of
the "noise" is actually from different functions being inlined.

This change also changes map2size.py from positional arguments to
having: --elf-file, --output-file. It also adds --no-source-paths.
These flags make it obvious that the tool can still be used with only
a .map file.

BUG=681694,708865
==========

[agrieve, 2017-04-11 13:40:50]

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

[agrieve, 2017-04-11 13:40:56]

On 2017/04/11 13:40:50, agrieve wrote:
> mailto:agrieve@chromium.org changed reviewers:
> + mailto:estevenson@chromium.org

[estevenson, 2017-04-11 17:11:42]

lgtm

https://codereview.chromium.org/2809043003/diff/1/tools/binary_size/map2size.py
File tools/binary_size/map2size.py (right):

https://codereview.chromium.org/2809043003/diff/1/tools/binary_size/map2size....
tools/binary_size/map2size.py:405: parser.add_argument('--elf-file',
required=True,
nit: won't the --map-file always be --elf-file + '.map.gz'? Can we simplify the
args by just assuming this? 

Also, please update the README to reflect the new args.

https://codereview.chromium.org/2809043003/diff/1/tools/binary_size/models.py
File tools/binary_size/models.py (right):

https://codereview.chromium.org/2809043003/diff/1/tools/binary_size/models.py...
tools/binary_size/models.py:59: 'raw_symbols',
nit: add doc for this field?

[agrieve, 2017-04-11 17:32:50]

https://codereview.chromium.org/2809043003/diff/1/tools/binary_size/map2size.py
File tools/binary_size/map2size.py (right):

https://codereview.chromium.org/2809043003/diff/1/tools/binary_size/map2size....
tools/binary_size/map2size.py:405: parser.add_argument('--elf-file',
required=True,
On 2017/04/11 17:11:42, estevenson wrote:
> nit: won't the --map-file always be --elf-file + '.map.gz'? Can we simplify
the
> args by just assuming this? 
> 
> Also, please update the README to reflect the new args.

That is the default for the arg, so normally you won't need to pass it. The
reason for the flag change is so that to tool can still be used when you don't
have the .so available (which is the case for integration_tests.py)

https://codereview.chromium.org/2809043003/diff/1/tools/binary_size/models.py
File tools/binary_size/models.py (right):

https://codereview.chromium.org/2809043003/diff/1/tools/binary_size/models.py...
tools/binary_size/models.py:59: 'raw_symbols',
On 2017/04/11 17:11:42, estevenson wrote:
> nit: add doc for this field?

Done.

[agrieve, 2017-04-11 17:34:05]

The CQ bit was checked by agrieve@chromium.org

[agrieve, 2017-04-11 17:34:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
estevenson@chromium.org
Link to the patchset: https://codereview.chromium.org/2809043003/#ps20001
(title: "review comments")

[commit-bot: I haz the power, 2017-04-11 17:35:14]

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-11 17:48:25]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1491932045225360,
"parent_rev": "6dd419deb677011673257d555f097cbc944adc28", "commit_rev":
"f70d47db0b462dbc08d4d6ae2e633e4e5b0af518"}

[commit-bot: I haz the power, 2017-04-11 17:49:12]

Description was changed from

==========
//tools/binary_size: Group [clone] and ** symbols

Two types of groups:
1. Those that end with "[clone .part.#]"
2. Star symbols (e.g.: "** symbol gap 3")

To handle grouped symbols:
* Print(recursive=True) added
* Diff() updated to be recursive
* Tests updated

For the .size files that motivated this change, the diff before had:
    355 symbols added (+), 2964 changed (~), 300 removed (-)
and after:
    345 symbols added (+), 2975 changed (~), 290 removed (-)

I was hoping for a bigger reduction, but it turns out the majority of
the "noise" is actually from different functions being inlined.

This change also changes map2size.py from positional arguments to
having: --elf-file, --output-file. It also adds --no-source-paths.
These flags make it obvious that the tool can still be used with only
a .map file.

BUG=681694,708865
==========

to

==========
//tools/binary_size: Group [clone] and ** symbols

Two types of groups:
1. Those that end with "[clone .part.#]"
2. Star symbols (e.g.: "** symbol gap 3")

To handle grouped symbols:
* Print(recursive=True) added
* Diff() updated to be recursive
* Tests updated

For the .size files that motivated this change, the diff before had:
    355 symbols added (+), 2964 changed (~), 300 removed (-)
and after:
    345 symbols added (+), 2975 changed (~), 290 removed (-)

I was hoping for a bigger reduction, but it turns out the majority of
the "noise" is actually from different functions being inlined.

This change also changes map2size.py from positional arguments to
having: --elf-file, --output-file. It also adds --no-source-paths.
These flags make it obvious that the tool can still be used with only
a .map file.

BUG=681694,708865

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

[commit-bot: I haz the power, 2017-04-11 17:49:13]

Committed patchset #2 (id:20001) as
https://chromium.googlesource.com/chromium/src/+/f70d47db0b462dbc08d4d6ae2e63...