Reland: add dump provider for malloc heap profiler (https://codereview.chromium.org/1675183006/)

Reland: add dump provider for malloc heap profiler (https://codereview.chromium.org/1675183006/)

Reason for reland:
the previous CL forgot about the fact that nacl has its own special
base target (base_nacl) which is a fork of the real base one.
That caused a revert in crrev.com/1822013002 because:
 - The original CL introduced a dependency from a translation unit in
   base (malloc_dump_provider.cc) to allocator_features.h, which is a
   generated header.
 - The original CL was relying on the fact that there is an existing
   dependency from base to allocator_features declared in base.gyp.
 - The original CL forgot about the need of the same dependency from
   base_nacl.
This cl is fixing the latter point.

Original breakage:
https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Mac/builds/8808

Original issue's description:
> tracing: add dump provider for malloc heap profiler
>
> Now that the allocator shim is landing (the base API landed in
> crre.com/1675143004, Linux support in crrev.com/1781573002) we
> can use that to implement the malloc heap profiler on top of that.
> This CL leverages the new shim API to collect information about
> malloc/free and injecting that into the heap profiler, leveraging
> the same infrastructure we are already using for PartitionAlloc
> and BlinkGC (oilpan).
>
> Next steps
> ----------
> Currently this heap profiler reports also the memory used by the
> tracing subsystem itself, which is not desirable. Will come up
> with some scoped suppression semantic to bypass the reporting.
>
> BUG=550886
> TBR=haraken
>
> Committed: https://crrev.com/14c8b52dac1285bbf23514d05e6109aa7edc427f
> Cr-Commit-Position: refs/heads/master@{#382505}

BUG=550886, 596873
TBR=ssid@chromium.org,petrcermak@chromium.org,dskiba@google.com,haraken@chromium.org

Committed: https://crrev.com/fd907216bc422c737219832038e5fc0a5fb66488
Cr-Commit-Position: refs/heads/master@{#383225}

[Primiano Tucci (use gerrit), 2016-03-24 17:40:25]

Patchset #2 (id:20001) has been deleted

[Primiano Tucci (use gerrit), 2016-03-24 17:41:36]

Description was changed from

==========
Reland: add dump provider for malloc heap profiler
(https://codereview.chromium.org/1675183006/)

Reason for reland:
the previous CL forgot about the fact that nacl has its own special
base target (base_nacl) which is a fork of the real base one.
That caused a revert in crrev.com/1822013002 because:
 - The original CL introduced a dependency from a translation unit in
   base to allocator_features.h, which is a generated header.
 - The original CL was relying on the fact that there is a dependency
   from base to allocator_features.
 - The original CL forgot about the need of the same dependency from
   base_nacl.

Original breakage:
https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Mac/b...

Original issue's description:
> tracing: add dump provider for malloc heap profiler
>
> Now that the allocator shim is landing (the base API landed in
> crre.com/1675143004, Linux support in crrev.com/1781573002) we
> can use that to implement the malloc heap profiler on top of that.
> This CL leverages the new shim API to collect information about
> malloc/free and injecting that into the heap profiler, leveraging
> the same infrastructure we are already using for PartitionAlloc
> and BlinkGC (oilpan).
>
> Next steps
> ----------
> Currently this heap profiler reports also the memory used by the
> tracing subsystem itself, which is not desirable. Will come up
> with some scoped suppression semantic to bypass the reporting.
>
> BUG=550886
> TBR=haraken
>
> Committed: https://crrev.com/14c8b52dac1285bbf23514d05e6109aa7edc427f
> Cr-Commit-Position: refs/heads/master@{#382505}

BUG=550886, 596873
TBR=ssid@chromium.org,petrcermak@chromium.org,dskiba@google.com,haraken@chrom...
==========

to

==========
Reland: add dump provider for malloc heap profiler
(https://codereview.chromium.org/1675183006/)

Reason for reland:
the previous CL forgot about the fact that nacl has its own special
base target (base_nacl) which is a fork of the real base one.
That caused a revert in crrev.com/1822013002 because:
 - The original CL introduced a dependency from a translation unit in
   base (malloc_dump_provider.cc) to allocator_features.h, which is a
   generated header.
 - The original CL was relying on the fact that there is an existing
   dependency from base to allocator_features declared in base.gyp.
 - The original CL forgot about the need of the same dependency from
   base_nacl.
This cl is fixing the latter point.

Original breakage:
https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Mac/b...

Original issue's description:
> tracing: add dump provider for malloc heap profiler
>
> Now that the allocator shim is landing (the base API landed in
> crre.com/1675143004, Linux support in crrev.com/1781573002) we
> can use that to implement the malloc heap profiler on top of that.
> This CL leverages the new shim API to collect information about
> malloc/free and injecting that into the heap profiler, leveraging
> the same infrastructure we are already using for PartitionAlloc
> and BlinkGC (oilpan).
>
> Next steps
> ----------
> Currently this heap profiler reports also the memory used by the
> tracing subsystem itself, which is not desirable. Will come up
> with some scoped suppression semantic to bypass the reporting.
>
> BUG=550886
> TBR=haraken
>
> Committed: https://crrev.com/14c8b52dac1285bbf23514d05e6109aa7edc427f
> Cr-Commit-Position: refs/heads/master@{#382505}

BUG=550886, 596873
TBR=ssid@chromium.org,petrcermak@chromium.org,dskiba@google.com,haraken@chrom...
==========

[Primiano Tucci (use gerrit), 2016-03-24 17:41:36]

primiano@chromium.org changed reviewers:
+ thakis@chromium.org
- dskiba@google.com, haraken@chromium.org, petrcermak@chromium.org,
ssid@chromium.org

[Primiano Tucci (use gerrit), 2016-03-24 17:42:42]

Nico, can I ask you to take a look to base/base_nacl.gyp , which is the only
diff of this reland?
I am never sure what is the proper way of dealing with these shadow copies of
base in nacl.

[Primiano Tucci (use gerrit), 2016-03-24 17:43:26]

primiano@chromium.org changed reviewers:
+ danakj@chromium.org

[Primiano Tucci (use gerrit), 2016-03-24 17:43:26]

nico -> danakj, as nico seems to be away for a bit.

[danakj, 2016-03-24 21:03:38]

rubberstamp LGTM on the new gyp changes

[haraken, 2016-03-24 22:23:00]

WebKit LGTM

[Dmitry Skiba, 2016-03-24 22:58:06]

https://codereview.chromium.org/1831763003/diff/40001/base/trace_event/malloc...
File base/trace_event/malloc_dump_provider.cc (right):

https://codereview.chromium.org/1831763003/diff/40001/base/trace_event/malloc...
base/trace_event/malloc_dump_provider.cc:239: AllocationContext context =
tracker->GetContextSnapshot();
BTW, you might want to move this after if (!allocation_register_) because
otherwise it's useless work if we're not profiling.

[Primiano Tucci (use gerrit), 2016-03-25 02:05:03]

https://codereview.chromium.org/1831763003/diff/40001/base/trace_event/malloc...
File base/trace_event/malloc_dump_provider.cc (right):

https://codereview.chromium.org/1831763003/diff/40001/base/trace_event/malloc...
base/trace_event/malloc_dump_provider.cc:239: AllocationContext context =
tracker->GetContextSnapshot();
On 2016/03/24 22:58:06, Dmitry Skiba wrote:
> BTW, you might want to move this after if (!allocation_register_) because
> otherwise it's useless work if we're not profiling.

Hmm I think what is missing here is a speculative check on
heap_profiler_enabled_.
GetConextSnapshot ahs a  memcpy and I don't want to do that under the lock,
which serializes all allocations.
Currently this slow path is going to happen only in the case when tracing gets
disabled, but I agree with you it can be improved.
I'll just do that on a subsequent CL. I'm on a flight and git operations are not
an option here, and ssid has CLs that need to be based on top of this.
I filed crbug.com/597885 to remember about this

[Primiano Tucci (use gerrit), 2016-03-25 02:05:09]

The CQ bit was checked by primiano@chromium.org

[commit-bot: I haz the power, 2016-03-25 02:05:21]

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

[commit-bot: I haz the power, 2016-03-25 02:13:40]

Description was changed from

==========
Reland: add dump provider for malloc heap profiler
(https://codereview.chromium.org/1675183006/)

Reason for reland:
the previous CL forgot about the fact that nacl has its own special
base target (base_nacl) which is a fork of the real base one.
That caused a revert in crrev.com/1822013002 because:
 - The original CL introduced a dependency from a translation unit in
   base (malloc_dump_provider.cc) to allocator_features.h, which is a
   generated header.
 - The original CL was relying on the fact that there is an existing
   dependency from base to allocator_features declared in base.gyp.
 - The original CL forgot about the need of the same dependency from
   base_nacl.
This cl is fixing the latter point.

Original breakage:
https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Mac/b...

Original issue's description:
> tracing: add dump provider for malloc heap profiler
>
> Now that the allocator shim is landing (the base API landed in
> crre.com/1675143004, Linux support in crrev.com/1781573002) we
> can use that to implement the malloc heap profiler on top of that.
> This CL leverages the new shim API to collect information about
> malloc/free and injecting that into the heap profiler, leveraging
> the same infrastructure we are already using for PartitionAlloc
> and BlinkGC (oilpan).
>
> Next steps
> ----------
> Currently this heap profiler reports also the memory used by the
> tracing subsystem itself, which is not desirable. Will come up
> with some scoped suppression semantic to bypass the reporting.
>
> BUG=550886
> TBR=haraken
>
> Committed: https://crrev.com/14c8b52dac1285bbf23514d05e6109aa7edc427f
> Cr-Commit-Position: refs/heads/master@{#382505}

BUG=550886, 596873
TBR=ssid@chromium.org,petrcermak@chromium.org,dskiba@google.com,haraken@chrom...
==========

to

==========
Reland: add dump provider for malloc heap profiler
(https://codereview.chromium.org/1675183006/)

Reason for reland:
the previous CL forgot about the fact that nacl has its own special
base target (base_nacl) which is a fork of the real base one.
That caused a revert in crrev.com/1822013002 because:
 - The original CL introduced a dependency from a translation unit in
   base (malloc_dump_provider.cc) to allocator_features.h, which is a
   generated header.
 - The original CL was relying on the fact that there is an existing
   dependency from base to allocator_features declared in base.gyp.
 - The original CL forgot about the need of the same dependency from
   base_nacl.
This cl is fixing the latter point.

Original breakage:
https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Mac/b...

Original issue's description:
> tracing: add dump provider for malloc heap profiler
>
> Now that the allocator shim is landing (the base API landed in
> crre.com/1675143004, Linux support in crrev.com/1781573002) we
> can use that to implement the malloc heap profiler on top of that.
> This CL leverages the new shim API to collect information about
> malloc/free and injecting that into the heap profiler, leveraging
> the same infrastructure we are already using for PartitionAlloc
> and BlinkGC (oilpan).
>
> Next steps
> ----------
> Currently this heap profiler reports also the memory used by the
> tracing subsystem itself, which is not desirable. Will come up
> with some scoped suppression semantic to bypass the reporting.
>
> BUG=550886
> TBR=haraken
>
> Committed: https://crrev.com/14c8b52dac1285bbf23514d05e6109aa7edc427f
> Cr-Commit-Position: refs/heads/master@{#382505}

BUG=550886, 596873
TBR=ssid@chromium.org,petrcermak@chromium.org,dskiba@google.com,haraken@chrom...
==========

[commit-bot: I haz the power, 2016-03-25 02:13:41]

Committed patchset #2 (id:40001)

[commit-bot: I haz the power, 2016-03-25 02:15:17]

Description was changed from

==========
Reland: add dump provider for malloc heap profiler
(https://codereview.chromium.org/1675183006/)

Reason for reland:
the previous CL forgot about the fact that nacl has its own special
base target (base_nacl) which is a fork of the real base one.
That caused a revert in crrev.com/1822013002 because:
 - The original CL introduced a dependency from a translation unit in
   base (malloc_dump_provider.cc) to allocator_features.h, which is a
   generated header.
 - The original CL was relying on the fact that there is an existing
   dependency from base to allocator_features declared in base.gyp.
 - The original CL forgot about the need of the same dependency from
   base_nacl.
This cl is fixing the latter point.

Original breakage:
https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Mac/b...

Original issue's description:
> tracing: add dump provider for malloc heap profiler
>
> Now that the allocator shim is landing (the base API landed in
> crre.com/1675143004, Linux support in crrev.com/1781573002) we
> can use that to implement the malloc heap profiler on top of that.
> This CL leverages the new shim API to collect information about
> malloc/free and injecting that into the heap profiler, leveraging
> the same infrastructure we are already using for PartitionAlloc
> and BlinkGC (oilpan).
>
> Next steps
> ----------
> Currently this heap profiler reports also the memory used by the
> tracing subsystem itself, which is not desirable. Will come up
> with some scoped suppression semantic to bypass the reporting.
>
> BUG=550886
> TBR=haraken
>
> Committed: https://crrev.com/14c8b52dac1285bbf23514d05e6109aa7edc427f
> Cr-Commit-Position: refs/heads/master@{#382505}

BUG=550886, 596873
TBR=ssid@chromium.org,petrcermak@chromium.org,dskiba@google.com,haraken@chrom...
==========

to

==========
Reland: add dump provider for malloc heap profiler
(https://codereview.chromium.org/1675183006/)

Reason for reland:
the previous CL forgot about the fact that nacl has its own special
base target (base_nacl) which is a fork of the real base one.
That caused a revert in crrev.com/1822013002 because:
 - The original CL introduced a dependency from a translation unit in
   base (malloc_dump_provider.cc) to allocator_features.h, which is a
   generated header.
 - The original CL was relying on the fact that there is an existing
   dependency from base to allocator_features declared in base.gyp.
 - The original CL forgot about the need of the same dependency from
   base_nacl.
This cl is fixing the latter point.

Original breakage:
https://build.chromium.org/p/chromium.chrome/builders/Google%20Chrome%20Mac/b...

Original issue's description:
> tracing: add dump provider for malloc heap profiler
>
> Now that the allocator shim is landing (the base API landed in
> crre.com/1675143004, Linux support in crrev.com/1781573002) we
> can use that to implement the malloc heap profiler on top of that.
> This CL leverages the new shim API to collect information about
> malloc/free and injecting that into the heap profiler, leveraging
> the same infrastructure we are already using for PartitionAlloc
> and BlinkGC (oilpan).
>
> Next steps
> ----------
> Currently this heap profiler reports also the memory used by the
> tracing subsystem itself, which is not desirable. Will come up
> with some scoped suppression semantic to bypass the reporting.
>
> BUG=550886
> TBR=haraken
>
> Committed: https://crrev.com/14c8b52dac1285bbf23514d05e6109aa7edc427f
> Cr-Commit-Position: refs/heads/master@{#382505}

BUG=550886, 596873
TBR=ssid@chromium.org,petrcermak@chromium.org,dskiba@google.com,haraken@chrom...

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

[commit-bot: I haz the power, 2016-03-25 02:15:18]

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