Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(111)

Issue 2582453002: [tracing] Implement polling in MemoryDumpManager (Closed)

Created:
4 years ago by ssid
Modified:
3 years, 10 months ago
CC:
chromium-reviews, tracing+reviews_chromium.org, wfh+watch_chromium.org, vmpstr+watch_chromium.org
Target Ref:
refs/pending/heads/master
Project:
chromium
Visibility:
Public.

Description

[tracing] Implement polling in MemoryDumpManager Implement polling for memory totals when peak triggers are given in trace config. It also triggers dumps if there is a big increase in memory total. BUG=607533 Review-Url: https://codereview.chromium.org/2582453002 Cr-Commit-Position: refs/heads/master@{#451386} Committed: https://chromium.googlesource.com/chromium/src/+/5047017bdca8641764919bcbb25b60b8d89c5684

Patch Set 1 #

Total comments: 18

Patch Set 2 : reorganize. #

Patch Set 3 : Keep config to MDM. #

Patch Set 4 : nit. #

Total comments: 22

Patch Set 5 : fixes. #

Patch Set 6 : Nit. #

Total comments: 28

Patch Set 7 : Address comments. #

Total comments: 2

Patch Set 8 : doc link and fix test. #

Unified diffs Side-by-side diffs Delta from patch set Stats (+469 lines, -171 lines) Patch
M base/BUILD.gn View 1 2 3 4 5 6 7 1 chunk +2 lines, -0 lines 0 comments Download
M base/trace_event/heap_profiler_heap_dump_writer.cc View 1 2 1 chunk +1 line, -2 lines 0 comments Download
M base/trace_event/memory_dump_manager.h View 1 2 3 4 5 6 7 6 chunks +6 lines, -29 lines 0 comments Download
M base/trace_event/memory_dump_manager.cc View 1 2 3 4 5 6 7 12 chunks +55 lines, -90 lines 0 comments Download
M base/trace_event/memory_dump_manager_unittest.cc View 1 2 3 4 5 6 7 3 chunks +38 lines, -30 lines 0 comments Download
A base/trace_event/memory_dump_scheduler.h View 1 2 3 4 5 6 1 chunk +121 lines, -0 lines 0 comments Download
A base/trace_event/memory_dump_scheduler.cc View 1 2 3 4 5 6 7 1 chunk +222 lines, -0 lines 0 comments Download
M base/trace_event/memory_dump_session_state.h View 1 2 3 chunks +14 lines, -11 lines 0 comments Download
M base/trace_event/memory_dump_session_state.cc View 1 2 2 chunks +9 lines, -9 lines 0 comments Download
M tools/gn/bootstrap/bootstrap.py View 1 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 58 (42 generated)
ssid
This CL adds polling based on config. This does not implement any peak detection, but ...
4 years ago (2016-12-20 00:31:56 UTC) #6
Primiano Tucci (use gerrit)
Ok I did a first pass. I like a lot the idea of splitting out ...
3 years, 11 months ago (2017-01-18 16:16:07 UTC) #7
ssid
I have changed this CL to have more cleaner api. I still think trace config ...
3 years, 11 months ago (2017-01-20 23:07:28 UTC) #9
ssid
Primiano, PTAL. It seems difficult to write separate unit tests for this scheduler. WDYT?
3 years, 11 months ago (2017-01-25 22:13:56 UTC) #15
Primiano Tucci (use gerrit)
Some comments, will do a final pass tomorrow. Design looks great now, thanks a lot ...
3 years, 11 months ago (2017-01-26 02:52:23 UTC) #22
ssid
please see replies inline https://codereview.chromium.org/2582453002/diff/200001/base/trace_event/memory_dump_manager.cc File base/trace_event/memory_dump_manager.cc (right): https://codereview.chromium.org/2582453002/diff/200001/base/trace_event/memory_dump_manager.cc#newcode436 base/trace_event/memory_dump_manager.cc:436: << "All polling MDPs cannot ...
3 years, 11 months ago (2017-01-26 21:48:57 UTC) #23
ssid
https://codereview.chromium.org/2582453002/diff/200001/base/trace_event/memory_dump_manager.cc File base/trace_event/memory_dump_manager.cc (right): https://codereview.chromium.org/2582453002/diff/200001/base/trace_event/memory_dump_manager.cc#newcode790 base/trace_event/memory_dump_manager.cc:790: const TraceConfig::MemoryDumpConfig memory_dump_config = On 2017/01/26 21:48:57, ssid wrote: ...
3 years, 11 months ago (2017-01-27 00:43:25 UTC) #25
ssid
Primiano, ping.
3 years, 10 months ago (2017-02-02 20:08:37 UTC) #29
Primiano Tucci (use gerrit)
sorry for the delay on this. Everything looks good (% some minor comment here and ...
3 years, 10 months ago (2017-02-03 20:08:20 UTC) #30
ssid
Made changes, ptal thanks! https://codereview.chromium.org/2582453002/diff/240001/base/trace_event/memory_dump_scheduler.cc File base/trace_event/memory_dump_scheduler.cc (right): https://codereview.chromium.org/2582453002/diff/240001/base/trace_event/memory_dump_scheduler.cc#newcode103 base/trace_event/memory_dump_scheduler.cc:103: DCHECK_EQ(0u, light_dump_period_ms % min_timer_period_ms_); On ...
3 years, 10 months ago (2017-02-14 02:05:40 UTC) #41
Primiano Tucci (use gerrit)
LGTM thanks https://codereview.chromium.org/2582453002/diff/340001/base/trace_event/memory_dump_scheduler.cc File base/trace_event/memory_dump_scheduler.cc (right): https://codereview.chromium.org/2582453002/diff/340001/base/trace_event/memory_dump_scheduler.cc#newcode16 base/trace_event/memory_dump_scheduler.cc:16: // Threshold on increase in memory from ...
3 years, 10 months ago (2017-02-15 18:13:44 UTC) #44
ssid
Thanks. https://codereview.chromium.org/2582453002/diff/340001/base/trace_event/memory_dump_scheduler.cc File base/trace_event/memory_dump_scheduler.cc (right): https://codereview.chromium.org/2582453002/diff/340001/base/trace_event/memory_dump_scheduler.cc#newcode16 base/trace_event/memory_dump_scheduler.cc:16: // Threshold on increase in memory from last ...
3 years, 10 months ago (2017-02-17 16:53:02 UTC) #45
ssid
+Nico for the BUILD file edit. Thanks
3 years, 10 months ago (2017-02-17 16:55:43 UTC) #49
Nico
lgtm
3 years, 10 months ago (2017-02-17 17:37:10 UTC) #50
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/2582453002/360001
3 years, 10 months ago (2017-02-17 20:04:19 UTC) #55
commit-bot: I haz the power
3 years, 10 months ago (2017-02-17 22:27:06 UTC) #58
Message was sent while issue was closed.
Committed patchset #8 (id:360001) as
https://chromium.googlesource.com/chromium/src/+/5047017bdca8641764919bcbb25b...

Powered by Google App Engine
This is Rietveld 408576698