Show type names in Oilpan heap profiler

Show type names in Oilpan heap profiler

This CL enables to show object type names in Oilpan heap profiler.

BUG=585063
Committed: https://crrev.com/8727d5daa81ec334c5ac99a2223fe13ff1595710
Cr-Commit-Position: refs/heads/master@{#376146}

[hajimehoshi, 2016-02-18 09:15:56]

hajimehoshi@chromium.org changed reviewers:
+ haraken@chromium.org, primiano@chromium.org

[hajimehoshi, 2016-02-18 09:15:56]

PTAL

BTW, I tested this and saw the object names in 'Heap Details' as a flat list. I
expected a pseudo-stack tree constructed by TRACE_EVENT. Am I missing something?

[haraken, 2016-02-18 09:49:22]

LGTM

On 2016/02/18 09:15:56, hajimehoshi wrote:
> PTAL
> 
> BTW, I tested this and saw the object names in 'Heap Details' as a flat list.
I
> expected a pseudo-stack tree constructed by TRACE_EVENT. Am I missing
something?

Trace events and WTF_HEAP_PROFILER_TYPE_NAME are different things.
WTF_HEAP_PROFILER_TYPE_NAME tells the object type to the heap profiler (so it's
visualized as a flat list). Trace events tell a stack trace of the object
allocation to the heap profiler (so it's visualized as a pseudo stack).

BTW, now that we have WTF_HEAP_PROFILER_TYPE_NAME, can we remove
GCInfo::m_className
(https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
I guess WTF_HEAP_PROFILER_TYPE_NAME provides the same thing as the
GCInfo::m_className.

primiano@?

[hajimehoshi, 2016-02-18 10:08:30]

On 2016/02/18 09:49:22, haraken wrote:
> LGTM
> 
> On 2016/02/18 09:15:56, hajimehoshi wrote:
> > PTAL
> > 
> > BTW, I tested this and saw the object names in 'Heap Details' as a flat
list.
> I
> > expected a pseudo-stack tree constructed by TRACE_EVENT. Am I missing
> something?
> 
> Trace events and WTF_HEAP_PROFILER_TYPE_NAME are different things.
> WTF_HEAP_PROFILER_TYPE_NAME tells the object type to the heap profiler (so
it's
> visualized as a flat list). Trace events tell a stack trace of the object
> allocation to the heap profiler (so it's visualized as a pseudo stack).

I found that I needed to check 'blink' in addition to 'memory-infra' before
measuring with chrome://tracing.  That's why I couldn't find the pseudo-tree
stack. Now I can see the tree!

> 
> BTW, now that we have WTF_HEAP_PROFILER_TYPE_NAME, can we remove
> GCInfo::m_className
>
(https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> I guess WTF_HEAP_PROFILER_TYPE_NAME provides the same thing as the
> GCInfo::m_className.
> 
> primiano@?

[Primiano Tucci (use gerrit), 2016-02-18 11:50:02]

\o/
LGTM 

I think you solved the pseudo-stack mystery, right?

> BTW, now that we have WTF_HEAP_PROFILER_TYPE_NAME, can we remove
> GCInfo::m_className
>
(https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> I guess WTF_HEAP_PROFILER_TYPE_NAME provides the same thing as the
> GCInfo::m_className.
> 
> primiano@?
Ehm, I am missing something.
In principle yes, that should be the same thing.
In practice, I cannot see where m_className is set in the codebase :-?
WTF_HEAP_PROFILER_TYPE_NAME will give you essentially the expansion of <T>,
which I think is what you want, right?

[haraken, 2016-02-18 11:54:18]

On 2016/02/18 11:50:02, Primiano Tucci wrote:
> \o/
> LGTM 
> 
> I think you solved the pseudo-stack mystery, right?
> 
> > BTW, now that we have WTF_HEAP_PROFILER_TYPE_NAME, can we remove
> > GCInfo::m_className
> >
>
(https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> > I guess WTF_HEAP_PROFILER_TYPE_NAME provides the same thing as the
> > GCInfo::m_className.
> > 
> > primiano@?
> Ehm, I am missing something.
> In principle yes, that should be the same thing.
> In practice, I cannot see where m_className is set in the codebase :-?

It is set here:

https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...

> WTF_HEAP_PROFILER_TYPE_NAME will give you essentially the expansion of <T>,
> which I think is what you want, right?

Yes, so I think we can just remove GCInfo::m_className.

[Primiano Tucci (use gerrit), 2016-02-18 12:00:20]

On 2016/02/18 11:54:18, haraken wrote:
> On 2016/02/18 11:50:02, Primiano Tucci wrote:
> > \o/
> > LGTM 
> > 
> > I think you solved the pseudo-stack mystery, right?
> > 
> > > BTW, now that we have WTF_HEAP_PROFILER_TYPE_NAME, can we remove
> > > GCInfo::m_className
> > >
> >
>
(https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> > > I guess WTF_HEAP_PROFILER_TYPE_NAME provides the same thing as the
> > > GCInfo::m_className.
> > > 
> > > primiano@?
> > Ehm, I am missing something.
> > In principle yes, that should be the same thing.
> > In practice, I cannot see where m_className is set in the codebase :-?
> 
> It is set here:
> 
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> 
> > WTF_HEAP_PROFILER_TYPE_NAME will give you essentially the expansion of <T>,
> > which I think is what you want, right?
> 
> Yes, so I think we can just remove GCInfo::m_className.

Oh right.
Yes, It looks that that TypenameStringTrait do the same trickery of
WTF_HEAP_PROFILER_NAME.
I think the only difference is that TypenameStringTrait does the string
massaging (see extractTypeNameFromFunctionName) at runtime in chrome, to
transform 
TypenameStringTrait<NameOfTheClass>::get() [T=NameOfTheClass]  
into
NameOfTheClass

the tracing approach of WTF_HEAP_PROFILER_NAME instead is that we put the full
long string in the trace, and do the string processing in traceviewer at import
time.
That should avoid runtime overhead in chrome and makes it easier for us to
support different compilers, as the full expansion string depends on the actual
compiler.
See the code here
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/catapu...

[hajimehoshi, 2016-02-18 12:01:36]

The CQ bit was checked by hajimehoshi@chromium.org

[commit-bot: I haz the power, 2016-02-18 12:02:55]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1711643002/20001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1711643002/20001

[hajimehoshi, 2016-02-18 12:06:48]

On 2016/02/18 12:00:20, Primiano Tucci wrote:
> On 2016/02/18 11:54:18, haraken wrote:
> > On 2016/02/18 11:50:02, Primiano Tucci wrote:
> > > \o/
> > > LGTM 
> > > 
> > > I think you solved the pseudo-stack mystery, right?

Yes!

> > > 
> > > > BTW, now that we have WTF_HEAP_PROFILER_TYPE_NAME, can we remove
> > > > GCInfo::m_className
> > > >
> > >
> >
>
(https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> > > > I guess WTF_HEAP_PROFILER_TYPE_NAME provides the same thing as the
> > > > GCInfo::m_className.
> > > > 
> > > > primiano@?
> > > Ehm, I am missing something.
> > > In principle yes, that should be the same thing.
> > > In practice, I cannot see where m_className is set in the codebase :-?
> > 
> > It is set here:
> > 
> >
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/WebKit...
> > 
> > > WTF_HEAP_PROFILER_TYPE_NAME will give you essentially the expansion of
<T>,
> > > which I think is what you want, right?
> > 
> > Yes, so I think we can just remove GCInfo::m_className.
> 
> Oh right.
> Yes, It looks that that TypenameStringTrait do the same trickery of
> WTF_HEAP_PROFILER_NAME.
> I think the only difference is that TypenameStringTrait does the string
> massaging (see extractTypeNameFromFunctionName) at runtime in chrome, to
> transform 
> TypenameStringTrait<NameOfTheClass>::get() [T=NameOfTheClass]  
> into
> NameOfTheClass
> 
> the tracing approach of WTF_HEAP_PROFILER_NAME instead is that we put the full
> long string in the trace, and do the string processing in traceviewer at
import
> time.
> That should avoid runtime overhead in chrome and makes it easier for us to
> support different compilers, as the full expansion string depends on the
actual
> compiler.
> See the code here
>
https://code.google.com/p/chromium/codesearch#chromium/src/third_party/catapu...

I don't have a strong opinion regarding m_className, so that's fine with me to
remove this. BTW, DETAILED_MEMORY_INFRA is not maintained well so that I
couldn't compile with this macro... Removing m_className would make it possible
to remove this mode?

[commit-bot: I haz the power, 2016-02-18 12:08:01]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-02-18 12:09:18]

Description was changed from

==========
Show type names in Oilpan heap profiler

This CL enables to show object type names in Oilpan heap profiler.

BUG=585063
==========

to

==========
Show type names in Oilpan heap profiler

This CL enables to show object type names in Oilpan heap profiler.

BUG=585063

Committed: https://crrev.com/8727d5daa81ec334c5ac99a2223fe13ff1595710
Cr-Commit-Position: refs/heads/master@{#376146}
==========

[commit-bot: I haz the power, 2016-02-18 12:09:19]

Patchset 2 (id:??) landed as
https://crrev.com/8727d5daa81ec334c5ac99a2223fe13ff1595710
Cr-Commit-Position: refs/heads/master@{#376146}

[haraken, 2016-02-18 12:13:46]

Yes, just remove GCInfo::m_className and the DETAILED_MEMORY_INFRA flag in a
follow up.