Supersize diff rewrite + tweaks

supersize: Rewrite diff logic and a few display tweaks

The prior diff logic tried to make sense of symbol aliases by
putting the diffed symbols in the same alias groups as the
after_symbols. However, this approach does not work when
a symbol from one alias group changes to have a different
set of aliases in the after group.

Now, all symbols within a diff have no aliases, but instead
contain pointers to the before & after symbols. Additionally,
each diff'ed symbol now also tracks its own diff_status (rather
than this information being stored in the containing group.

Both of these changes simplify the logic, as well as allow for some
more advanced queries. (e.g. filter for symbols where number of
aliases has changed)

BUG=733053
Review-Url: https://codereview.chromium.org/2936033002
Cr-Commit-Position: refs/heads/master@{#479805}
Committed: https://chromium.googlesource.com/chromium/src/+/89f066e7d382f7f2f9e74b1afbe1fcf1649615cf

[agrieve, 2017-06-14 02:08:02]

Description was changed from

==========
Supersize diff rewrite + tweaks

BUG=
==========

to

==========
supersize: Rewrite diff logic and a few display tweaks

The prior diff logic tried to make sense of symbol aliases by
putting the diffed symbols in the same alias groups as the
after_symbols. However, this approach does not work when
a symbol from one alias group changes to have a different
set of aliases in the after group.

Now, all symbols within a diff have no aliases, but instead
contain pointers to the before & after symbols. Additionally,
each diff'ed symbol now also tracks its own diff_status (rather
than this information being stored in the containing group.

Both of these changes simplify the logic, as well as allow for some
more advanced queries. (e.g. filter for symbols where number of
aliases has changed)

BUG=733053
==========

[agrieve, 2017-06-14 02:08:03]

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

[estevenson, 2017-06-15 16:10:45]

lgtm

https://codereview.chromium.org/2936033002/diff/60001/tools/binary_size/libsu...
File tools/binary_size/libsupersize/describe.py (right):

https://codereview.chromium.org/2936033002/diff/60001/tools/binary_size/libsu...
tools/binary_size/libsupersize/describe.py:133: if pss_with_sign[0] not in '~-':
nit: does it make sense for _FormatPss() to always include the sign?

https://codereview.chromium.org/2936033002/diff/60001/tools/binary_size/libsu...
tools/binary_size/libsupersize/describe.py:251: # causes a some symbols to be
UNCHANGED, yet have pss != 0.
nit: remove 'a'

https://codereview.chromium.org/2936033002/diff/60001/tools/binary_size/libsu...
tools/binary_size/libsupersize/describe.py:297: unqiue_counts =
delta_group.CountUniqueSymbols()
nit: spelling. Might be a little more readable to just unpack these into their
own variable.

[agrieve, 2017-06-15 19:57:04]

https://codereview.chromium.org/2936033002/diff/60001/tools/binary_size/libsu...
File tools/binary_size/libsupersize/describe.py (right):

https://codereview.chromium.org/2936033002/diff/60001/tools/binary_size/libsu...
tools/binary_size/libsupersize/describe.py:133: if pss_with_sign[0] not in '~-':
On 2017/06/15 16:10:45, estevenson wrote:
> nit: does it make sense for _FormatPss() to always include the sign?

The idea is to only include the explicit + when showing a diff, so I don't think
it makes sense with that in mind.

https://codereview.chromium.org/2936033002/diff/60001/tools/binary_size/libsu...
tools/binary_size/libsupersize/describe.py:251: # causes a some symbols to be
UNCHANGED, yet have pss != 0.
On 2017/06/15 16:10:45, estevenson wrote:
> nit: remove 'a'

Done.

https://codereview.chromium.org/2936033002/diff/60001/tools/binary_size/libsu...
tools/binary_size/libsupersize/describe.py:297: unqiue_counts =
delta_group.CountUniqueSymbols()
On 2017/06/15 16:10:45, estevenson wrote:
> nit: spelling. Might be a little more readable to just unpack these into their
> own variable.

Done.

[agrieve, 2017-06-15 19:57:24]

The CQ bit was checked by agrieve@chromium.org

[agrieve, 2017-06-15 19:57:27]

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/2936033002/#ps70001
(title: "review comnts")

[commit-bot: I haz the power, 2017-06-15 19:58:15]

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-06-15 20:14:58]

CQ is committing da patch.

Bot data: {"patchset_id": 70001, "attempt_start_ts": 1497556644668610,
"parent_rev": "02527bb2e9eee1c6dd97b3ec2133d0011db28965", "commit_rev":
"89f066e7d382f7f2f9e74b1afbe1fcf1649615cf"}

[commit-bot: I haz the power, 2017-06-15 20:15:14]

Description was changed from

==========
supersize: Rewrite diff logic and a few display tweaks

The prior diff logic tried to make sense of symbol aliases by
putting the diffed symbols in the same alias groups as the
after_symbols. However, this approach does not work when
a symbol from one alias group changes to have a different
set of aliases in the after group.

Now, all symbols within a diff have no aliases, but instead
contain pointers to the before & after symbols. Additionally,
each diff'ed symbol now also tracks its own diff_status (rather
than this information being stored in the containing group.

Both of these changes simplify the logic, as well as allow for some
more advanced queries. (e.g. filter for symbols where number of
aliases has changed)

BUG=733053
==========

to

==========
supersize: Rewrite diff logic and a few display tweaks

The prior diff logic tried to make sense of symbol aliases by
putting the diffed symbols in the same alias groups as the
after_symbols. However, this approach does not work when
a symbol from one alias group changes to have a different
set of aliases in the after group.

Now, all symbols within a diff have no aliases, but instead
contain pointers to the before & after symbols. Additionally,
each diff'ed symbol now also tracks its own diff_status (rather
than this information being stored in the containing group.

Both of these changes simplify the logic, as well as allow for some
more advanced queries. (e.g. filter for symbols where number of
aliases has changed)

BUG=733053

Review-Url: https://codereview.chromium.org/2936033002
Cr-Commit-Position: refs/heads/master@{#479805}
Committed:
https://chromium.googlesource.com/chromium/src/+/89f066e7d382f7f2f9e74b1afbe1...
==========

[commit-bot: I haz the power, 2017-06-15 20:15:19]

Committed patchset #5 (id:70001) as
https://chromium.googlesource.com/chromium/src/+/89f066e7d382f7f2f9e74b1afbe1...