Chromium Code Reviews
Help | Chromium Project | Gerrit Changes | Sign in
(306)

Issue 2650863003: [tracing] Switch to new heap dump format. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
8 months ago by DmitrySkiba
Modified:
3 months ago
CC:
chromium-reviews, oilpan-reviews, Mads Ager (chromium), vmpstr+watch_chromium.org, wfh+watch_chromium.org, blink-reviews, tracing+reviews_chromium.org, kinuko+watch, kouhei+heap_chromium.org, erikchen
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

[tracing] Switch to new heap dump format. This CL switches tracing to a new heap dump format, that offers the following advantages: 1. Dumps include all the information collected by Chrome's heap profiler. 2. The format is simpler and more compact. 3. The format can easily be extended to include additional per-entry data. 4. The format allows for post-processing (see recategorization from crrev.com/2906413002 as an example). BUG=708930 Review-Url: https://codereview.chromium.org/2650863003 Cr-Commit-Position: refs/heads/master@{#480580} Committed: https://chromium.googlesource.com/chromium/src/+/d4a5e98235da0cc8c4fb1ae2f67826b0b480ce4c

Patch Set 1 #

Patch Set 2 : Add 'version' field. #

Total comments: 17

Patch Set 3 : Rebase #

Patch Set 4 : Address comments #

Patch Set 5 : Add StringDeduplicator, consolidate exporters #

Patch Set 6 : Fix unittests; describe format #

Patch Set 7 : DCHECK for continuous mode #

Total comments: 50

Patch Set 8 : Fix StringDeduplicator::Insert #

Total comments: 13

Patch Set 9 : Address comments #

Total comments: 7

Patch Set 10 : Address comments (heaps_v2) #

Patch Set 11 : Rebase #

Patch Set 12 : nit #

Patch Set 13 : Rebase #

Patch Set 14 : Fix StringDeduplicator issue #

Patch Set 15 : Mega rebase #

Patch Set 16 : Fix rebase damage #

Patch Set 17 : Sharded rebase #

Total comments: 6

Patch Set 18 : Address comments #

Patch Set 19 : Rebase #

Patch Set 20 : Fix tests #

Patch Set 21 : Rebase #

Unified diffs Side-by-side diffs Delta from patch set Stats (+1212 lines, -1127 lines) Patch
M base/BUILD.gn View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 2 chunks +6 lines, -3 lines 0 comments Download
A base/trace_event/heap_profiler_event_writer.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +104 lines, -0 lines 0 comments Download
A base/trace_event/heap_profiler_event_writer.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +142 lines, -0 lines 0 comments Download
A base/trace_event/heap_profiler_event_writer_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 1 chunk +288 lines, -0 lines 0 comments Download
D base/trace_event/heap_profiler_heap_dump_writer.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +0 lines, -115 lines 0 comments Download
D base/trace_event/heap_profiler_heap_dump_writer.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +0 lines, -324 lines 0 comments Download
D base/trace_event/heap_profiler_heap_dump_writer_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +0 lines, -332 lines 0 comments Download
M base/trace_event/heap_profiler_serialization_state.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +10 lines, -5 lines 0 comments Download
M base/trace_event/heap_profiler_serialization_state.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +11 lines, -10 lines 0 comments Download
M base/trace_event/heap_profiler_stack_frame_deduplicator.h View 1 2 3 4 5 6 7 8 5 chunks +17 lines, -10 lines 0 comments Download
M base/trace_event/heap_profiler_stack_frame_deduplicator.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 3 chunks +31 lines, -32 lines 0 comments Download
M base/trace_event/heap_profiler_stack_frame_deduplicator_unittest.cc View 1 2 3 4 5 6 7 8 5 chunks +163 lines, -47 lines 0 comments Download
A base/trace_event/heap_profiler_string_deduplicator.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 1 chunk +50 lines, -0 lines 0 comments Download
A base/trace_event/heap_profiler_string_deduplicator.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 1 chunk +58 lines, -0 lines 0 comments Download
A base/trace_event/heap_profiler_string_deduplicator_unittest.cc View 1 2 3 4 5 6 7 8 1 chunk +124 lines, -0 lines 0 comments Download
M base/trace_event/heap_profiler_type_name_deduplicator.h View 1 2 3 4 5 6 7 8 1 chunk +22 lines, -10 lines 0 comments Download
M base/trace_event/heap_profiler_type_name_deduplicator.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 3 chunks +21 lines, -27 lines 0 comments Download
M base/trace_event/heap_profiler_type_name_deduplicator_unittest.cc View 1 2 3 4 5 6 7 8 3 chunks +96 lines, -51 lines 0 comments Download
M base/trace_event/malloc_dump_provider.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 3 chunks +16 lines, -7 lines 0 comments Download
M base/trace_event/memory_dump_manager.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 3 chunks +9 lines, -56 lines 0 comments Download
M base/trace_event/process_memory_dump.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 3 chunks +4 lines, -6 lines 0 comments Download
M base/trace_event/process_memory_dump.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 4 chunks +12 lines, -13 lines 0 comments Download
M base/trace_event/process_memory_dump_unittest.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 3 chunks +13 lines, -12 lines 0 comments Download
M base/trace_event/sharded_allocation_register.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 3 chunks +5 lines, -12 lines 0 comments Download
M base/trace_event/sharded_allocation_register.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +4 lines, -13 lines 0 comments Download
M base/trace_event/trace_event_memory_overhead.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +1 line, -0 lines 0 comments Download
M base/trace_event/trace_event_memory_overhead.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 1 chunk +2 lines, -0 lines 0 comments Download
M third_party/WebKit/Source/platform/PartitionAllocMemoryDumpProvider.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +1 line, -11 lines 0 comments Download
M third_party/WebKit/Source/platform/heap/BlinkGCMemoryDumpProvider.cpp View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 1 chunk +1 line, -10 lines 0 comments Download
M third_party/WebKit/Source/platform/instrumentation/tracing/web_process_memory_dump.h View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +0 lines, -10 lines 0 comments Download
M third_party/WebKit/Source/platform/instrumentation/tracing/web_process_memory_dump.cc View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 2 chunks +0 lines, -11 lines 0 comments Download
M tools/gn/bootstrap/bootstrap.py View 1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 1 chunk +1 line, -0 lines 0 comments Download
Commit queue not available (can’t edit this change).

Messages

Total messages: 51 (20 generated)
DmitrySkiba
7 months, 3 weeks ago (2017-01-30 19:44:11 UTC) #2
DmitrySkiba
PTAL
7 months, 1 week ago (2017-02-14 18:31:15 UTC) #4
Primiano Tucci (use gerrit)
On 2017/02/14 18:31:15, DmitrySkiba wrote: > PTAL sorry for the delay, manage to do *almost* ...
7 months, 1 week ago (2017-02-15 18:55:02 UTC) #5
Primiano Tucci (use gerrit)
Thanks a lot for this. I'm very excited by the code being ditched here. Everything ...
7 months ago (2017-02-17 17:07:05 UTC) #6
DmitrySkiba
Overall things TODO: 1. Tests 2. Optimizing format for size 3. Public format doc (use/extend ...
7 months ago (2017-02-23 07:17:19 UTC) #8
lpy
On 2017/02/23 07:17:19, DmitrySkiba wrote: > 3. Public format doc (use/extend Hector's, or add a ...
7 months ago (2017-02-23 07:23:42 UTC) #9
hjd
https://codereview.chromium.org/2650863003/diff/20001/base/trace_event/memory_dump_manager.cc File base/trace_event/memory_dump_manager.cc (right): https://codereview.chromium.org/2650863003/diff/20001/base/trace_event/memory_dump_manager.cc#newcode725 base/trace_event/memory_dump_manager.cc:725: heap_dump_value->SetInteger("version", 1); On 2017/02/23 07:17:19, DmitrySkiba wrote: > On ...
7 months ago (2017-02-23 12:33:28 UTC) #11
DmitrySkiba
PTAL Changes at a glance: 1. Added StringDeduplicator, and made both StackFrameDeduplicator / TypeNameDeduplicator depend ...
6 months, 3 weeks ago (2017-03-01 08:20:48 UTC) #12
DmitrySkiba
On 2017/03/01 08:20:48, DmitrySkiba wrote: > PTAL > > Changes at a glance: > > ...
6 months, 3 weeks ago (2017-03-02 19:56:55 UTC) #13
hjd
On 2017/03/02 19:56:55, DmitrySkiba wrote: > On 2017/03/01 08:20:48, DmitrySkiba wrote: > > PTAL > ...
6 months, 3 weeks ago (2017-03-02 19:58:43 UTC) #14
DmitrySkiba
PTAL. I added unittests and DCHECK for continuous mode.
6 months, 2 weeks ago (2017-03-07 22:45:09 UTC) #15
Primiano Tucci (use gerrit)
Thanks! I looked through all the code (a bit less tests) and it looks all ...
6 months, 2 weeks ago (2017-03-09 11:47:45 UTC) #17
ssid
I have 2 concerns: Does the trace size increase in pseudo mode because of string ...
6 months, 1 week ago (2017-03-13 03:38:01 UTC) #18
DmitrySkiba
https://codereview.chromium.org/2650863003/diff/120001/base/BUILD.gn File base/BUILD.gn (right): https://codereview.chromium.org/2650863003/diff/120001/base/BUILD.gn#newcode981 base/BUILD.gn:981: "trace_event/heap_profiler_string_deduplicator.cc", On 2017/03/09 11:47:44, Primiano Tucci (slow - perf) ...
6 months, 1 week ago (2017-03-14 22:12:48 UTC) #19
Primiano Tucci (use gerrit)
Ok LGTM (plz fix the description though), so you can also get other owners approvalz. ...
6 months, 1 week ago (2017-03-15 20:41:40 UTC) #20
ssid
lgtm, please look at the comments, that should be done in all dumps. https://codereview.chromium.org/2650863003/diff/140001/base/trace_event/heap_profiler_event_writer.cc File ...
6 months, 1 week ago (2017-03-16 02:09:20 UTC) #21
Primiano Tucci (use gerrit)
So I talked today with hjd: let's keep having the "heaps" as part of the ...
6 months, 1 week ago (2017-03-16 14:23:56 UTC) #22
DmitrySkiba
https://codereview.chromium.org/2650863003/diff/160001/base/trace_event/process_memory_dump.cc File base/trace_event/process_memory_dump.cc (right): https://codereview.chromium.org/2650863003/diff/160001/base/trace_event/process_memory_dump.cc#newcode254 base/trace_event/process_memory_dump.cc:254: allocation_register.EstimateTraceMemoryOverhead(&overhead); On 2017/03/16 02:09:20, ssid wrote: > This part ...
6 months, 1 week ago (2017-03-17 04:01:38 UTC) #23
DmitrySkiba
On 2017/03/16 14:23:56, Primiano Tucci (slow - perf) wrote: > So I talked today with ...
6 months, 1 week ago (2017-03-17 04:03:03 UTC) #24
hjd
On 2017/03/17 04:03:03, DmitrySkiba wrote: > On 2017/03/16 14:23:56, Primiano Tucci (slow - perf) wrote: ...
6 months, 1 week ago (2017-03-17 12:24:26 UTC) #25
Primiano Tucci (use gerrit)
Great thanks both. dskiba@ can you add the other OWNERS here? So as as soon ...
6 months, 1 week ago (2017-03-17 19:00:30 UTC) #26
DmitrySkiba
haraken@: please review third_party/WebKit/Source/* mark@: please review base/BUILD.gn
5 months, 1 week ago (2017-04-12 00:52:42 UTC) #29
Mark Mentovai
base/BUILD.gn LGTM
5 months, 1 week ago (2017-04-12 02:40:17 UTC) #30
haraken
WebKit LGTM
5 months, 1 week ago (2017-04-12 07:21:56 UTC) #31
DmitrySkiba
Primiano, please review changes related to ShardedAllocationRegister: 1. ShardedAllocationRegister::VisitAllocations is introduced. 2. ShardedAllocationRegister doesn't calculate ...
3 months, 4 weeks ago (2017-05-26 23:56:38 UTC) #36
Primiano Tucci (use gerrit)
Not fully sure I understand the need of VisitAllocations, seems overly complicated for what we ...
3 months, 3 weeks ago (2017-05-30 18:50:53 UTC) #37
DmitrySkiba
We can't update UpdateAndReturnsMetrics() because the thing it would update is now an internal detail ...
3 months, 2 weeks ago (2017-06-05 00:50:18 UTC) #38
Primiano Tucci (use gerrit)
LGTM, please send a psa to memory-dev just as a heads up in case some ...
3 months, 2 weeks ago (2017-06-07 19:43:58 UTC) #40
commit-bot: I haz the power
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.org/2650863003/390001
3 months ago (2017-06-19 18:27:44 UTC) #47
commit-bot: I haz the power
Committed patchset #21 (id:390001) as https://chromium.googlesource.com/chromium/src/+/d4a5e98235da0cc8c4fb1ae2f67826b0b480ce4c
3 months ago (2017-06-19 21:10:34 UTC) #50
pdr.
3 months ago (2017-06-19 23:43:15 UTC) #51
Message was sent while issue was closed.
On 2017/06/19 at 21:10:34, commit-bot wrote:
> Committed patchset #21 (id:390001) as
https://chromium.googlesource.com/chromium/src/+/d4a5e98235da0cc8c4fb1ae2f678...

I think this patch may have caused a failure:
https://bugs.chromium.org/p/chromium/issues/detail?id=734810
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld b40b6558b