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

Issue 2832333002: Revert of Embed a single sample in histogram metadata. (Closed)

Created:
3 years, 8 months ago by tdanderson
Modified:
3 years, 8 months ago
CC:
chromium-reviews, asvitkine+watch_chromium.org, vmpstr+watch_chromium.org
Target Ref:
refs/heads/master
Project:
chromium
Visibility:
Public.

Description

Revert of Embed a single sample in histogram metadata. (patchset #9 id:240001 of https://codereview.chromium.org/2811713003/ ) Reason for revert: Speculative revert for suspected cause of failing NonThreadSafeDeathTest.DestructorNotAllowedOnDifferentThreadInDebug test on Mac10.9 Tests (dbg). See https://uberchromegw.corp.google.com/i/chromium.mac/builders/Mac10.9%20Tests%20%28dbg%29/builds/39679 Original issue's description: > Embed a single sample in histogram metadata. > > This uses the new DelayedPersistentAllocation to avoid allocating the > full "counts" array when only one value is being stored. That one > value is kept in the HistogramSamples metadata. When a second value > is recorded, the counts-array is created and the single-sample is > moved; it is there-after unused. > > This change only affects SampleVector. A future change may add the > same support for PersistentSampleMap. > > A quick test shows persistent memory use is reduced by 44% (from 924 > KiB to 512 KiB) after a clean start and the loading of three pages: > chrome://histograms > https://www.google.com/ > chrome://histograms (again) > > BUG=705342 > > Review-Url: https://codereview.chromium.org/2811713003 > Cr-Commit-Position: refs/heads/master@{#466401} > Committed: https://chromium.googlesource.com/chromium/src/+/4162bafec82ea11c98d4dc2572337a039b79fdb9 TBR=asvitkine@chromium.org,bcwhite@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true BUG=705342 Review-Url: https://codereview.chromium.org/2832333002 Cr-Commit-Position: refs/heads/master@{#466488} Committed: https://chromium.googlesource.com/chromium/src/+/94191025bc29f325d91565c9520af6495f22be09

Patch Set 1 #

Unified diffs Side-by-side diffs Delta from patch set Stats (+233 lines, -1048 lines) Patch
M base/metrics/histogram.h View 11 chunks +25 lines, -21 lines 0 comments Download
M base/metrics/histogram.cc View 10 chunks +59 lines, -65 lines 0 comments Download
M base/metrics/histogram_samples.h View 4 chunks +13 lines, -106 lines 0 comments Download
M base/metrics/histogram_samples.cc View 6 chunks +17 lines, -166 lines 0 comments Download
M base/metrics/histogram_unittest.cc View 1 chunk +3 lines, -3 lines 0 comments Download
M base/metrics/persistent_histogram_allocator.cc View 5 chunks +23 lines, -30 lines 0 comments Download
M base/metrics/persistent_sample_map.cc View 1 chunk +2 lines, -1 line 0 comments Download
M base/metrics/sample_map.cc View 1 chunk +2 lines, -1 line 0 comments Download
M base/metrics/sample_vector.h View 2 chunks +18 lines, -96 lines 0 comments Download
M base/metrics/sample_vector.cc View 2 chunks +61 lines, -292 lines 0 comments Download
M base/metrics/sample_vector_unittest.cc View 8 chunks +10 lines, -267 lines 0 comments Download

Messages

Total messages: 7 (3 generated)
tdanderson
Created Revert of Embed a single sample in histogram metadata.
3 years, 8 months ago (2017-04-21 23:18:40 UTC) #2
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/2832333002/1
3 years, 8 months ago (2017-04-21 23:19:17 UTC) #3
bcwhite
lgtm
3 years, 8 months ago (2017-04-21 23:20:10 UTC) #5
commit-bot: I haz the power
3 years, 8 months ago (2017-04-21 23:20:13 UTC) #7
Message was sent while issue was closed.
Committed patchset #1 (id:1) as
https://chromium.googlesource.com/chromium/src/+/94191025bc29f325d91565c9520a...

Powered by Google App Engine
This is Rietveld 408576698