[PartitionAlloc] Annotate common Blink types for heap profiling

[PartitionAlloc] Annotate common Blink types for heap profiling

This annotates a few common types in Blink and WTF to unconditionally
provide type information. This will enable getting type info from the
heap profiler even in official builds, albeit with less detail. It will
not be possible to get the long tail in official builds. Depending on
the page, the annotated types can make up 30 to 70 percent of the
PartitionAlloc heap.

The types annotated are:

 - char
 - blink::CSSSelector
 - blink::CSSValue
 - blink::ImmutableStylePropertySet
 - blink::InvalidationSet
 - blink::Node
 - blink::Resource
 - blink::SharedBuffer
 - blink::StyleRule
 - WTF::StringImpl

Type info about |char| is available only when allocated via variants of
|PartitionAllocator::allocateVectorBacking|. This is a large portion of
all char's but not 100%.

The annotated types are the ten types that on average make up the
largest portion of the PartitionAlloc heap, and which can be annotated
easily. Types like |WTF::StringImpl*| are in the top ten too, but most
of these are not stored in vector backing.

This is part of the heap profiler in chrome://tracing.

BUG=524631
Committed: https://crrev.com/ba2eebd19776527566625c066495dd31565b0002
Cr-Commit-Position: refs/heads/master@{#362687}

[Ruud van Asseldonk, 2015-11-26 20:17:06]

ruuda@google.com changed reviewers:
+ bashi@chromium.org, haraken@chromium.org, primiano@chromium.org,
tasak@google.com

[haraken, 2015-11-27 00:22:29]

https://codereview.chromium.org/1474213002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/CSSSelector.h (right):

https://codereview.chromium.org/1474213002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/CSSSelector.h:78:
USING_FAST_MALLOC_WITH_TYPE_NAME(CSSSelector, "blink::CSSSelector");

It looks redundant to write "CSSSelector" twice in the macro. Would there be any
way to avoid duplicating the type name twice in the macro?

[Ruud van Asseldonk, 2015-11-27 10:41:44]

https://codereview.chromium.org/1474213002/diff/1/third_party/WebKit/Source/c...
File third_party/WebKit/Source/core/css/CSSSelector.h (right):

https://codereview.chromium.org/1474213002/diff/1/third_party/WebKit/Source/c...
third_party/WebKit/Source/core/css/CSSSelector.h:78:
USING_FAST_MALLOC_WITH_TYPE_NAME(CSSSelector, "blink::CSSSelector");
On 2015/11/27 00:22:28, haraken wrote:
> 
> It looks redundant to write "CSSSelector" twice in the macro. Would there be
any
> way to avoid duplicating the type name twice in the macro?

Actually there is! Fixed in new patchset.

[haraken, 2015-11-27 10:46:26]

On 2015/11/27 10:41:44, Ruud van Asseldonk wrote:
>
https://codereview.chromium.org/1474213002/diff/1/third_party/WebKit/Source/c...
> File third_party/WebKit/Source/core/css/CSSSelector.h (right):
> 
>
https://codereview.chromium.org/1474213002/diff/1/third_party/WebKit/Source/c...
> third_party/WebKit/Source/core/css/CSSSelector.h:78:
> USING_FAST_MALLOC_WITH_TYPE_NAME(CSSSelector, "blink::CSSSelector");
> On 2015/11/27 00:22:28, haraken wrote:
> > 
> > It looks redundant to write "CSSSelector" twice in the macro. Would there be
> any
> > way to avoid duplicating the type name twice in the macro?
> 
> Actually there is! Fixed in new patchset.

Great!

Is there any benefit in mixing USING_FAST_MALLOC and
USING_FAST_MALLOC_WITH_TYPE_NAME in the code base? I guess we can just make
USING_FAST_MALLOC take the qualified type name (e.g., blink::CSSValue). Then we
can get type names of all objects in PartitionAlloc.

[Ruud van Asseldonk, 2015-11-27 12:13:46]

On 2015/11/27 10:46:26, haraken wrote:
> On 2015/11/27 10:41:44, Ruud van Asseldonk wrote:
> >
>
https://codereview.chromium.org/1474213002/diff/1/third_party/WebKit/Source/c...
> > File third_party/WebKit/Source/core/css/CSSSelector.h (right):
> > 
> >
>
https://codereview.chromium.org/1474213002/diff/1/third_party/WebKit/Source/c...
> > third_party/WebKit/Source/core/css/CSSSelector.h:78:
> > USING_FAST_MALLOC_WITH_TYPE_NAME(CSSSelector, "blink::CSSSelector");
> > On 2015/11/27 00:22:28, haraken wrote:
> > > 
> > > It looks redundant to write "CSSSelector" twice in the macro. Would there
be
> > any
> > > way to avoid duplicating the type name twice in the macro?
> > 
> > Actually there is! Fixed in new patchset.
> 
> Great!
> 
> Is there any benefit in mixing USING_FAST_MALLOC and
> USING_FAST_MALLOC_WITH_TYPE_NAME in the code base? I guess we can just make
> USING_FAST_MALLOC take the qualified type name (e.g., blink::CSSValue). Then
we
> can get type names of all objects in PartitionAlloc.

Where |USING_FAST_MALLOC_WITH_TYPE_NAME| is used, type info is available even in
official builds. With |USING_FAST_MALLOC| the type name is available only in
development builds. Also, |USING_FAST_MALLOC| correctly expands templates, the
|_WITH_TYPE_NAME| stringifies the argument, so it can be used only for concrete
types.

[haraken, 2015-11-27 12:15:44]

On 2015/11/27 12:13:46, Ruud van Asseldonk wrote:
> On 2015/11/27 10:46:26, haraken wrote:
> > On 2015/11/27 10:41:44, Ruud van Asseldonk wrote:
> > >
> >
>
https://codereview.chromium.org/1474213002/diff/1/third_party/WebKit/Source/c...
> > > File third_party/WebKit/Source/core/css/CSSSelector.h (right):
> > > 
> > >
> >
>
https://codereview.chromium.org/1474213002/diff/1/third_party/WebKit/Source/c...
> > > third_party/WebKit/Source/core/css/CSSSelector.h:78:
> > > USING_FAST_MALLOC_WITH_TYPE_NAME(CSSSelector, "blink::CSSSelector");
> > > On 2015/11/27 00:22:28, haraken wrote:
> > > > 
> > > > It looks redundant to write "CSSSelector" twice in the macro. Would
there
> be
> > > any
> > > > way to avoid duplicating the type name twice in the macro?
> > > 
> > > Actually there is! Fixed in new patchset.
> > 
> > Great!
> > 
> > Is there any benefit in mixing USING_FAST_MALLOC and
> > USING_FAST_MALLOC_WITH_TYPE_NAME in the code base? I guess we can just make
> > USING_FAST_MALLOC take the qualified type name (e.g., blink::CSSValue). Then
> we
> > can get type names of all objects in PartitionAlloc.
> 
> Where |USING_FAST_MALLOC_WITH_TYPE_NAME| is used, type info is available even
in
> official builds. With |USING_FAST_MALLOC| the type name is available only in
> development builds. Also, |USING_FAST_MALLOC| correctly expands templates, the
> |_WITH_TYPE_NAME| stringifies the argument, so it can be used only for
concrete
> types.

What happens if we always use the USING_FAST_MALLOC_WITH_TYPE_NAME specifying
concrete types? (Then we can just rename USING_FAST_MALLOC_WITH_TYPE_NAME to
USING_FAST_MALLOC.)

[Ruud van Asseldonk, 2015-11-27 12:28:56]

On 2015/11/27 12:15:44, haraken wrote:
> On 2015/11/27 12:13:46, Ruud van Asseldonk wrote:
> > On 2015/11/27 10:46:26, haraken wrote:
> > > On 2015/11/27 10:41:44, Ruud van Asseldonk wrote:
> > > >
> > >
> >
>
https://codereview.chromium.org/1474213002/diff/1/third_party/WebKit/Source/c...
> > > > File third_party/WebKit/Source/core/css/CSSSelector.h (right):
> > > > 
> > > >
> > >
> >
>
https://codereview.chromium.org/1474213002/diff/1/third_party/WebKit/Source/c...
> > > > third_party/WebKit/Source/core/css/CSSSelector.h:78:
> > > > USING_FAST_MALLOC_WITH_TYPE_NAME(CSSSelector, "blink::CSSSelector");
> > > > On 2015/11/27 00:22:28, haraken wrote:
> > > > > 
> > > > > It looks redundant to write "CSSSelector" twice in the macro. Would
> there
> > be
> > > > any
> > > > > way to avoid duplicating the type name twice in the macro?
> > > > 
> > > > Actually there is! Fixed in new patchset.
> > > 
> > > Great!
> > > 
> > > Is there any benefit in mixing USING_FAST_MALLOC and
> > > USING_FAST_MALLOC_WITH_TYPE_NAME in the code base? I guess we can just
make
> > > USING_FAST_MALLOC take the qualified type name (e.g., blink::CSSValue).
Then
> > we
> > > can get type names of all objects in PartitionAlloc.
> > 
> > Where |USING_FAST_MALLOC_WITH_TYPE_NAME| is used, type info is available
even
> in
> > official builds. With |USING_FAST_MALLOC| the type name is available only in
> > development builds. Also, |USING_FAST_MALLOC| correctly expands templates,
the
> > |_WITH_TYPE_NAME| stringifies the argument, so it can be used only for
> concrete
> > types.
> 
> What happens if we always use the USING_FAST_MALLOC_WITH_TYPE_NAME specifying
> concrete types? (Then we can just rename USING_FAST_MALLOC_WITH_TYPE_NAME to
> USING_FAST_MALLOC.)

Then the type name strings will be included even in official builds, and we
increase binary size too much. We need a way to distinguish “always on” type
info and “only developer build type info”, because making all types “always on”
is too big a hit for binary size.

On a side note, |USING_FAST_MALLOC_WITH_TYPE_NAME| could be made to work with
non-concrete types too by using |CompileTimeTypeInfo| instead of stringifying
the macro argument, but I don’t think it is necessary here.

[haraken, 2015-11-27 15:36:10]

Thanks for the clarification! LGTM

https://codereview.chromium.org/1474213002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/wtf/Allocator.h (right):

https://codereview.chromium.org/1474213002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/wtf/Allocator.h:134: #define
USING_FAST_MALLOC_WITH_TYPE_NAME(type) USING_FAST_MALLOC_INTERNAL(type, #type)

Add a comment and explain the difference of these macros.

[Ruud van Asseldonk, 2015-11-27 16:31:19]

https://codereview.chromium.org/1474213002/diff/20001/third_party/WebKit/Sour...
File third_party/WebKit/Source/wtf/Allocator.h (right):

https://codereview.chromium.org/1474213002/diff/20001/third_party/WebKit/Sour...
third_party/WebKit/Source/wtf/Allocator.h:134: #define
USING_FAST_MALLOC_WITH_TYPE_NAME(type) USING_FAST_MALLOC_INTERNAL(type, #type)
On 2015/11/27 15:36:10, haraken wrote:
> 
> Add a comment and explain the difference of these macros.

Done.

[Ruud van Asseldonk, 2015-12-02 11:56:25]

The CQ bit was checked by ruuda@google.com

[Ruud van Asseldonk, 2015-12-02 11:56:25]

The patchset sent to the CQ was uploaded after l-g-t-m from haraken@chromium.org
Link to the patchset: https://codereview.chromium.org/1474213002/#ps60001
(title: "Mark template specialisations as WTF_EXPORT")

[commit-bot: I haz the power, 2015-12-02 11:56:34]

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

[commit-bot: I haz the power, 2015-12-02 12:22:24]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-12-02 12:23:11]

Description was changed from

==========
[PartitionAlloc] Annotate common Blink types for heap profiling

This annotates a few common types in Blink and WTF to unconditionally
provide type information. This will enable getting type info from the
heap profiler even in official builds, albeit with less detail. It will
not be possible to get the long tail in official builds. Depending on
the page, the annotated types can make up 30 to 70 percent of the
PartitionAlloc heap.

The types annotated are:

 - char
 - blink::CSSSelector
 - blink::CSSValue
 - blink::ImmutableStylePropertySet
 - blink::InvalidationSet
 - blink::Node
 - blink::Resource
 - blink::SharedBuffer
 - blink::StyleRule
 - WTF::StringImpl

Type info about |char| is available only when allocated via variants of
|PartitionAllocator::allocateVectorBacking|. This is a large portion of
all char's but not 100%.

The annotated types are the ten types that on average make up the
largest portion of the PartitionAlloc heap, and which can be annotated
easily. Types like |WTF::StringImpl*| are in the top ten too, but most
of these are not stored in vector backing.

This is part of the heap profiler in chrome://tracing.

BUG=524631
==========

to

==========
[PartitionAlloc] Annotate common Blink types for heap profiling

This annotates a few common types in Blink and WTF to unconditionally
provide type information. This will enable getting type info from the
heap profiler even in official builds, albeit with less detail. It will
not be possible to get the long tail in official builds. Depending on
the page, the annotated types can make up 30 to 70 percent of the
PartitionAlloc heap.

The types annotated are:

 - char
 - blink::CSSSelector
 - blink::CSSValue
 - blink::ImmutableStylePropertySet
 - blink::InvalidationSet
 - blink::Node
 - blink::Resource
 - blink::SharedBuffer
 - blink::StyleRule
 - WTF::StringImpl

Type info about |char| is available only when allocated via variants of
|PartitionAllocator::allocateVectorBacking|. This is a large portion of
all char's but not 100%.

The annotated types are the ten types that on average make up the
largest portion of the PartitionAlloc heap, and which can be annotated
easily. Types like |WTF::StringImpl*| are in the top ten too, but most
of these are not stored in vector backing.

This is part of the heap profiler in chrome://tracing.

BUG=524631

Committed: https://crrev.com/ba2eebd19776527566625c066495dd31565b0002
Cr-Commit-Position: refs/heads/master@{#362687}
==========

[commit-bot: I haz the power, 2015-12-02 12:23:12]

Patchset 4 (id:??) landed as
https://crrev.com/ba2eebd19776527566625c066495dd31565b0002
Cr-Commit-Position: refs/heads/master@{#362687}