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

Issue 2786373002: memory-infra: Add peak-detector skeleton. (Closed)

Created:
3 years, 8 months ago by Primiano Tucci (use gerrit)
Modified:
3 years, 8 months ago
Reviewers:
Nico, ssid, hjd
CC:
chromium-reviews, Dirk Pranke, tfarina, wfh+watch_chromium.org, vmpstr+watch_chromium.org, agrieve+watch_chromium.org, tracing+reviews_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

memory-infra: Add peak-detector skeleton. Adds a skeleton and tests for the MemoryPeakDetector. This CL does not yet take over the old peak detector in MemoryDumpScheduler, which will happen in the next CLs. BUG=607533 TBR=thakis@chromium.org Review-Url: https://codereview.chromium.org/2786373002 Cr-Commit-Position: refs/heads/master@{#461678} Committed: https://chromium.googlesource.com/chromium/src/+/f34c3e056c9a2eaa12a41daa2f8e05b579360f28

Patch Set 1 #

Patch Set 2 : . #

Patch Set 3 : . #

Total comments: 35

Patch Set 4 : . #

Patch Set 5 : ssid review #

Patch Set 6 : remove seqchecker #

Total comments: 22

Patch Set 7 : rebase + introduce teardown + reset instance in test #

Total comments: 1

Patch Set 8 : Add generation + test #

Patch Set 9 : minor typos #

Unified diffs Side-by-side diffs Delta from patch set Stats (+692 lines, -3 lines) Patch
M base/BUILD.gn View 1 2 3 4 5 6 2 chunks +3 lines, -0 lines 0 comments Download
M base/trace_event/memory_dump_manager_unittest.cc View 1 2 3 4 5 6 2 chunks +2 lines, -2 lines 0 comments Download
M base/trace_event/memory_dump_provider_info.h View 1 2 chunks +2 lines, -1 line 0 comments Download
A base/trace_event/memory_peak_detector.h View 1 2 3 4 5 6 7 8 1 chunk +139 lines, -0 lines 0 comments Download
A base/trace_event/memory_peak_detector.cc View 1 2 3 4 5 6 7 8 1 chunk +164 lines, -0 lines 0 comments Download
A base/trace_event/memory_peak_detector_unittest.cc View 1 2 3 4 5 6 7 1 chunk +381 lines, -0 lines 0 comments Download
M tools/gn/bootstrap/bootstrap.py View 1 chunk +1 line, -0 lines 0 comments Download

Dependent Patchsets:

Messages

Total messages: 45 (31 generated)
Primiano Tucci (use gerrit)
ssid, hjd WDYT? this should work both in the current model (where we tear down ...
3 years, 8 months ago (2017-03-31 16:51:07 UTC) #12
ssid
On 2017/03/31 16:51:07, Primiano Tucci wrote: > ssid, hjd WDYT? > > this should work ...
3 years, 8 months ago (2017-03-31 18:03:35 UTC) #15
Primiano Tucci (use gerrit)
Thanks! https://codereview.chromium.org/2786373002/diff/40001/base/trace_event/memory_peak_detector.cc File base/trace_event/memory_peak_detector.cc (right): https://codereview.chromium.org/2786373002/diff/40001/base/trace_event/memory_peak_detector.cc#newcode49 base/trace_event/memory_peak_detector.cc:49: DCHECK(state_ == NOT_INITIALIZED || state_ == DISABLED); On ...
3 years, 8 months ago (2017-03-31 20:01:55 UTC) #19
hjd
looks great! Just a few comments: https://codereview.chromium.org/2786373002/diff/100001/base/trace_event/memory_peak_detector.cc File base/trace_event/memory_peak_detector.cc (right): https://codereview.chromium.org/2786373002/diff/100001/base/trace_event/memory_peak_detector.cc#newcode102 base/trace_event/memory_peak_detector.cc:102: void MemoryPeakDetector::ReloadDumpProvidersAndStartPollingIfNeeded() { ...
3 years, 8 months ago (2017-04-03 13:25:31 UTC) #23
Primiano Tucci (use gerrit)
On 2017/04/03 13:25:31, hjd wrote: > looks great! Just a few comments: > > https://codereview.chromium.org/2786373002/diff/100001/base/trace_event/memory_peak_detector.cc ...
3 years, 8 months ago (2017-04-03 15:51:17 UTC) #24
hjd
On 2017/04/03 15:51:17, Primiano Tucci wrote: > On 2017/04/03 13:25:31, hjd wrote: > > looks ...
3 years, 8 months ago (2017-04-03 16:04:34 UTC) #25
ssid
Looks good. I am just not happy about the Initialize being called multiple times without ...
3 years, 8 months ago (2017-04-03 17:55:28 UTC) #26
Primiano Tucci (use gerrit)
Thanks a lot folks, very useful comments. see replies below https://codereview.chromium.org/2786373002/diff/40001/base/trace_event/memory_peak_detector.cc File base/trace_event/memory_peak_detector.cc (right): https://codereview.chromium.org/2786373002/diff/40001/base/trace_event/memory_peak_detector.cc#newcode49 ...
3 years, 8 months ago (2017-04-03 20:28:16 UTC) #30
ssid
lgtm, the code is functionally correct. But I still think we should not be supporting ...
3 years, 8 months ago (2017-04-04 05:08:36 UTC) #34
Primiano Tucci (use gerrit)
> But I still think we should not be > supporting unregistration of all mdps ...
3 years, 8 months ago (2017-04-04 09:14:31 UTC) #35
hjd
On 2017/04/04 09:14:31, Primiano Tucci wrote: > > But I still think we should not ...
3 years, 8 months ago (2017-04-04 09:24:58 UTC) #36
Primiano Tucci (use gerrit)
TBR thakis for the usual adds to base/BUILD.gn
3 years, 8 months ago (2017-04-04 09:29:04 UTC) #39
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/2786373002/200001
3 years, 8 months ago (2017-04-04 09:29:21 UTC) #42
commit-bot: I haz the power
3 years, 8 months ago (2017-04-04 10:29:50 UTC) #45
Message was sent while issue was closed.
Committed patchset #9 (id:200001) as
https://chromium.googlesource.com/chromium/src/+/f34c3e056c9a2eaa12a41daa2f8e...

Powered by Google App Engine
This is Rietveld 408576698