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

Issue 1125683004: Add aggregated memory histograms. (Closed)

Created:
5 years, 7 months ago by ulan
Modified:
4 years, 5 months ago
CC:
Hannes Payer (out of office), v8-dev
Base URL:
https://chromium.googlesource.com/v8/v8.git@master
Target Ref:
refs/pending/heads/master
Project:
v8
Visibility:
Public.

Description

Add aggregated memory histograms. This introduces V8.MemoryHeapCommitted and V8.MemoryHeapUsed histograms. In contrast to the existing memory histograms, the new histograms are uniform in time, i.e. their samples happen at regular time intervals. The --histogram-interval specifies the length of the interval. We implement this by linearly interpolating memory stats between GC and idle notification events. BUG=chromium:485472 LOG=NO Committed: https://crrev.com/d77839fd0182d01e6d7864f0312d2b194deea2f0 Cr-Commit-Position: refs/heads/master@{#28292}

Patch Set 1 #

Patch Set 2 : #

Patch Set 3 : Fix git cl format line #

Patch Set 4 : #

Patch Set 5 : Add comments #

Total comments: 1
Unified diffs Side-by-side diffs Delta from patch set Stats (+392 lines, -14 lines) Patch
M src/counters.h View 1 2 3 4 5 chunks +143 lines, -9 lines 1 comment Download
M src/counters.cc View 1 2 3 2 chunks +18 lines, -1 line 0 comments Download
M src/flag-definitions.h View 1 chunk +4 lines, -0 lines 0 comments Download
M src/heap/gc-tracer.cc View 1 3 chunks +15 lines, -2 lines 0 comments Download
M src/heap/heap.h View 1 1 chunk +2 lines, -0 lines 0 comments Download
M src/heap/heap.cc View 1 3 chunks +9 lines, -2 lines 0 comments Download
A test/unittests/counters-unittest.cc View 1 1 chunk +200 lines, -0 lines 0 comments Download
M test/unittests/unittests.gyp View 1 chunk +1 line, -0 lines 0 comments Download

Messages

Total messages: 16 (6 generated)
ulan
ptal
5 years, 7 months ago (2015-05-05 10:31:30 UTC) #2
ulan
Hannes: fyi
5 years, 7 months ago (2015-05-05 10:34:54 UTC) #3
jochen (gone - plz use gerrit)
lgtm please link to the design doc, and include some explanation on how the average ...
5 years, 7 months ago (2015-05-05 14:34:23 UTC) #4
commit-bot: I haz the power
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1125683004/80001
5 years, 7 months ago (2015-05-07 09:30:24 UTC) #7
commit-bot: I haz the power
Dry run: This issue passed the CQ dry run.
5 years, 7 months ago (2015-05-07 09:57:14 UTC) #9
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/patch-status/1125683004/80001
5 years, 7 months ago (2015-05-07 10:01:54 UTC) #11
commit-bot: I haz the power
Committed patchset #5 (id:80001)
5 years, 7 months ago (2015-05-07 10:03:39 UTC) #12
commit-bot: I haz the power
Patchset 5 (id:??) landed as https://crrev.com/d77839fd0182d01e6d7864f0312d2b194deea2f0 Cr-Commit-Position: refs/heads/master@{#28292}
5 years, 7 months ago (2015-05-07 10:03:54 UTC) #13
ulan
> please link to the design doc, and include some explanation on how the average ...
5 years, 7 months ago (2015-05-07 10:45:41 UTC) #14
Alexei Svitkine (slow)
4 years, 5 months ago (2016-06-28 16:54:39 UTC) #16
Message was sent while issue was closed.
https://codereview.chromium.org/1125683004/diff/80001/src/counters.h
File src/counters.h (right):

https://codereview.chromium.org/1125683004/diff/80001/src/counters.h#newcode540
src/counters.h:540: HM(heap_sample_maximum_committed,
V8.MemoryHeapSampleMaximumCommitted)
Hi, this histogram doesn't appear to be in histograms.xml. Can it be added
there?

Additionally, since it's not in the XML, it's data is not visible on UMA
dashboards, which likely means it's not used. If so, please consider removing it
- as it's currently one of the top unused histograms that's wasting users'
bandwidth and memory. Thanks!

Powered by Google App Engine
This is Rietveld 408576698