Add the core classification infrastructure to memory_inspector.

Add the core classification infrastructure to memory_inspector.

This CL introduce the core classes for handling classification of
data in a hierarchical fashion. It introduces two concepts:
- A rule tree: hierarchy of rules defined by the end-user.
- A result tree: the corresponding tree which aggregates the
memory counters.

BUG=340294
NOTRY=true

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=255352

[Primiano Tucci (use gerrit), 2014-02-27 15:15:04]

Thanks as usual :)

Just as a progress update: this CL adds the core classes for classification, but
not the actual classifiers, i.e. the guy that takes a core.memory_map.Map and
turns it into a AggreatedResults instance (and similar for native allocs).
I plan to do that in the next CL.

After these two (this cl + next) the only remaining part for the v1. prototype
is the HTML frontend ...unless I missed something macroscopic, hopefully not :)

[Philippe, 2014-02-28 14:43:49]

Thanks Primiano! This LGTM overall. We might be able to improve a few tiny
things on clarity/naming though.

https://codereview.chromium.org/183173003/diff/1/tools/memory_inspector/memor...
File tools/memory_inspector/memory_inspector/classification/results.py (right):

https://codereview.chromium.org/183173003/diff/1/tools/memory_inspector/memor...
tools/memory_inspector/memory_inspector/classification/results.py:13: - The
rules define how to "match" a subject (e.g., a mmap line or a native
Nit: I wonder if 'subject' is the best term we can use here. 'data' is also very
generic but maybe less intriguing IMO. I have no better suggestion yet but I
will think about it :)

A subject here is a piece of the snapshot data, right?

I just think it would be slightly clearer to use less abstract terms but at the
same time we don't want to restrict ourselves :)

https://codereview.chromium.org/183173003/diff/1/tools/memory_inspector/memor...
tools/memory_inspector/memory_inspector/classification/results.py:42:
"""Percolates down data in the bucket tree, incrementing the matching nodes.
Nit: Adds the provided values to the nodes that match the provided subject?

I'm wondering also if s/FilterData/MatchSubject would be clearer? MatchData()
would be nice(r) too IMO. Wdyt? AddValuesToMatchingNodes() is a bit verbose also
but maybe less mysterious. You could keep this more verbose name for the
internal helper function at least traversing the tree.


"Percolate" is a bit new to me :)

https://codereview.chromium.org/183173003/diff/1/tools/memory_inspector/memor...
tools/memory_inspector/memory_inspector/classification/results.py:44: The tree
is traversed in breadth-first matching the |subject| with each of
Nit: university is a bit far now, but isn't _PercolateDownAndFilterData()
currently doing a DFS as opposed to a BFS? :)

[Primiano Tucci (use gerrit), 2014-03-03 14:34:58]

Thanks for the useful comments.
I reworded some names according to your indications (and added some beautiful
ASCII art :) )

https://codereview.chromium.org/183173003/diff/1/tools/memory_inspector/memor...
File tools/memory_inspector/memory_inspector/classification/results.py (right):

https://codereview.chromium.org/183173003/diff/1/tools/memory_inspector/memor...
tools/memory_inspector/memory_inspector/classification/results.py:13: - The
rules define how to "match" a subject (e.g., a mmap line or a native
On 2014/02/28 14:43:50, Philippe wrote:
> Nit: I wonder if 'subject' is the best term we can use here. 'data' is also
very
> generic but maybe less intriguing IMO. I have no better suggestion yet but I
> will think about it :)

Agree. What about trace_record here? Should give the idea.


> A subject here is a piece of the snapshot data, right?
Yeah, exactly.

> I just think it would be slightly clearer to use less abstract terms but at
the
> same time we don't want to restrict ourselves :)
Agree.

https://codereview.chromium.org/183173003/diff/1/tools/memory_inspector/memor...
tools/memory_inspector/memory_inspector/classification/results.py:42:
"""Percolates down data in the bucket tree, incrementing the matching nodes.
On 2014/02/28 14:43:50, Philippe wrote:
> Nit: Adds the provided values to the nodes that match the provided subject?
> 
> I'm wondering also if s/FilterData/MatchSubject would be clearer? MatchData()
> would be nice(r) too IMO. Wdyt? AddValuesToMatchingNodes() is a bit verbose
also
> but maybe less mysterious. You could keep this more verbose name for the
> internal helper function at least traversing the tree.

AddToMatchingNodes (without values, a bit redundant) sounds very good to me!


> "Percolate" is a bit new to me :)
Isn't it the general term you use to express vertical tree operations (i.e. as
opposite to bubble-down)?

Anyways. Removed and make it more explicit.
Also there is no reason why _PercolateDownAndFilterData should have a different
name w.r.t. of this method. The static one is simply the recursive version. It's
just confusing having two different names.
So I'm just renaming them to AddToMatchingNodes and _AddToMatchingNodes.

https://codereview.chromium.org/183173003/diff/1/tools/memory_inspector/memor...
tools/memory_inspector/memory_inspector/classification/results.py:44: The tree
is traversed in breadth-first matching the |subject| with each of
On 2014/02/28 14:43:50, Philippe wrote:
> Nit: university is a bit far now, but isn't _PercolateDownAndFilterData()
> currently doing a DFS as opposed to a BFS? :)
I've just reworded it. Yeah that was really confusing. The real problem is that
the method is not doing a search at all, it is incrementing mathcing nodes (and
yes, looking them definitely in DFS :) ) but more nodes can match on multiple
levels (e.g. Ashmem and Ashmem::DalvikHeap) that was leading to confusion.
Hopefully the new wording and the tree diagram should be more clear now.

[Philippe, 2014-03-05 14:05:33]

Still LGTM, thanks a lot Primiano for improving those things!

https://codereview.chromium.org/183173003/diff/1/tools/memory_inspector/memor...
File tools/memory_inspector/memory_inspector/classification/results.py (right):

https://codereview.chromium.org/183173003/diff/1/tools/memory_inspector/memor...
tools/memory_inspector/memory_inspector/classification/results.py:13: - The
rules define how to "match" a subject (e.g., a mmap line or a native
On 2014/03/03 14:34:59, Primiano Tucci wrote:
> On 2014/02/28 14:43:50, Philippe wrote:
> > Nit: I wonder if 'subject' is the best term we can use here. 'data' is also
> very
> > generic but maybe less intriguing IMO. I have no better suggestion yet but I
> > will think about it :)
> 
> Agree. What about trace_record here? Should give the idea.
> 
> 
> > A subject here is a piece of the snapshot data, right?
> Yeah, exactly.
> 
> > I just think it would be slightly clearer to use less abstract terms but at
> the
> > same time we don't want to restrict ourselves :)
> Agree.

trace_record sounds great!

[Primiano Tucci (use gerrit), 2014-03-06 10:52:09]

The CQ bit was checked by primiano@chromium.org

[commit-bot: I haz the power, 2014-03-06 10:52:28]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/primiano@chromium.org/183173003/130001

[commit-bot: I haz the power, 2014-03-06 10:54:58]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2014-03-06 10:54:59]

Try jobs failed on following builders: mac_chromium_compile_dbg

[Primiano Tucci (use gerrit), 2014-03-06 11:13:55]

The CQ bit was checked by primiano@chromium.org

[commit-bot: I haz the power, 2014-03-06 11:14:00]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/primiano@chromium.org/183173003/130001

[commit-bot: I haz the power, 2014-03-06 14:40:39]

Change committed as 255352