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

Issue 323013003: Add histograms to track age of cache entries for web fonts (Closed)

Created:
6 years, 6 months ago by Kunihiko Sakamoto
Modified:
6 years, 6 months ago
CC:
chromium-reviews, cbentzel+watch_chromium.org, gavinp+disk_chromium.org, asvitkine+watch_chromium.org, jar+watch_chromium.org
Visibility:
Public.

Description

Add histograms to track age of cache entries for web fonts This CL adds UMA histograms that tracks age (time since creation) of cache entries for web fonts. Histograms are suffixed with font names: "roboto", "opensans", or "others". Also, fixes a bug that DiskCache.ReuseCount.Evict was recording evictions of non-webfont entries. BUG=379670 Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=276599

Patch Set 1 #

Total comments: 2

Patch Set 2 : Use entry->GetKey() #

Unified diffs Side-by-side diffs Delta from patch set Stats (+31 lines, -5 lines) Patch
M net/disk_cache/blockfile/webfonts_histogram.cc View 1 1 chunk +11 lines, -5 lines 0 comments Download
M tools/metrics/histograms/histograms.xml View 2 chunks +20 lines, -0 lines 0 comments Download

Messages

Total messages: 10 (0 generated)
Kunihiko Sakamoto
6 years, 6 months ago (2014-06-10 10:10:13 UTC) #1
rvargas (doing something else)
lgtm after fixing the key. https://codereview.chromium.org/323013003/diff/1/net/disk_cache/blockfile/webfonts_histogram.cc File net/disk_cache/blockfile/webfonts_histogram.cc (right): https://codereview.chromium.org/323013003/diff/1/net/disk_cache/blockfile/webfonts_histogram.cc#newcode82 net/disk_cache/blockfile/webfonts_histogram.cc:82: const char* label = ...
6 years, 6 months ago (2014-06-10 23:15:34 UTC) #2
Ilya Sherman
+Jesse for histograms review
6 years, 6 months ago (2014-06-10 23:29:23 UTC) #3
Kunihiko Sakamoto
https://codereview.chromium.org/323013003/diff/1/net/disk_cache/blockfile/webfonts_histogram.cc File net/disk_cache/blockfile/webfonts_histogram.cc (right): https://codereview.chromium.org/323013003/diff/1/net/disk_cache/blockfile/webfonts_histogram.cc#newcode82 net/disk_cache/blockfile/webfonts_histogram.cc:82: const char* label = HistogramLabel(info->key); On 2014/06/10 23:15:33, rvargas ...
6 years, 6 months ago (2014-06-11 01:50:10 UTC) #4
jwd
lgtm for histograms
6 years, 6 months ago (2014-06-11 17:22:32 UTC) #5
Ilya Sherman
histograms LGTM2
6 years, 6 months ago (2014-06-11 17:24:20 UTC) #6
Kunihiko Sakamoto
The CQ bit was checked by ksakamoto@chromium.org
6 years, 6 months ago (2014-06-12 01:00:41 UTC) #7
commit-bot: I haz the power
CQ is trying da patch. Follow status at https://chromium-status.appspot.com/cq/ksakamoto@chromium.org/323013003/20001
6 years, 6 months ago (2014-06-12 01:01:36 UTC) #8
commit-bot: I haz the power
FYI, CQ is re-trying this CL (attempt #1). The failing builders are: mac_chromium_rel on tryserver.chromium ...
6 years, 6 months ago (2014-06-12 07:06:12 UTC) #9
commit-bot: I haz the power
6 years, 6 months ago (2014-06-12 09:27:54 UTC) #10
Message was sent while issue was closed.
Change committed as 276599

Powered by Google App Engine
This is Rietveld 408576698