Chromium Code Reviews
|
|
Chromium Code Reviews
Issue
2831543002:
[UMA] Expand the comment for UMA_HISTOGRAM_SPARSE_SLOWLY (Closed)
|
Created:
3 years, 8 months ago by jkrcal Modified:
3 years, 8 months ago Reviewers:
Ilya Sherman CC:
chromium-reviews, danakj+watch_chromium.org, asvitkine+watch_chromium.org, vmpstr+watch_chromium.org Target Ref:
refs/heads/master Project:
chromium Visibility:
Public. |
Description[UMA] Expand the comment for UMA_HISTOGRAM_SPARSE_SLOWLY
This CL expands the comment for the UMA_HISTOGRAM_SPARSE_SLOWLY macro.
It is inspired by a recent code review discussion with isherman@.
BUG=none
Review-Url: https://codereview.chromium.org/2831543002
Cr-Commit-Position: refs/heads/master@{#467602}
Committed: https://chromium.googlesource.com/chromium/src/+/9591f480aaee3bd50c50e027f028730213ca4a8f
Patch Set 1 #
Total comments: 3
Patch Set 2 : Ilya's comments #Messages
Total messages: 18 (10 generated)
The CQ bit was checked by jkrcal@chromium.org to run a CQ dry run
Dry run: CQ is trying da patch. Follow status at https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
jkrcal@chromium.org changed reviewers: + isherman@chromium.org
Ilya, could you PTAL? https://codereview.chromium.org/2831543002/diff/1/base/metrics/histogram_macr... File base/metrics/histogram_macros.h (right): https://codereview.chromium.org/2831543002/diff/1/base/metrics/histogram_macr... base/metrics/histogram_macros.h:254: // histograms, i.e. uploading many many distinct values to the server (across Maybe we can make the "many many" more concrete. Roughly how many is "many many" in this context?
The CQ bit was unchecked by commit-bot@chromium.org
Dry run: This issue passed the CQ dry run.
Thanks! It would also be great to mention the idea of capping the recorded value, possibly by providing a sample invocation that includes capping. https://codereview.chromium.org/2831543002/diff/1/base/metrics/histogram_macr... File base/metrics/histogram_macros.h (right): https://codereview.chromium.org/2831543002/diff/1/base/metrics/histogram_macr... base/metrics/histogram_macros.h:254: // histograms, i.e. uploading many many distinct values to the server (across On 2017/04/19 07:58:06, jkrcal wrote: > Maybe we can make the "many many" more concrete. Roughly how many is "many many" > in this context? <= 100 is best, and <= 1000 is okay (but more expensive, and overkill in many cases).
Thanks, PTAL, again! https://codereview.chromium.org/2831543002/diff/1/base/metrics/histogram_macr... File base/metrics/histogram_macros.h (right): https://codereview.chromium.org/2831543002/diff/1/base/metrics/histogram_macr... base/metrics/histogram_macros.h:254: // histograms, i.e. uploading many many distinct values to the server (across On 2017/04/19 17:22:07, Ilya Sherman wrote: > On 2017/04/19 07:58:06, jkrcal wrote: > > Maybe we can make the "many many" more concrete. Roughly how many is "many > many" > > in this context? > > <= 100 is best, and <= 1000 is okay (but more expensive, and overkill in many > cases). Done.
LGTM. Thanks for taking the time to updating the docs!
The CQ bit was checked by isherman@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
The CQ bit was unchecked by commit-bot@chromium.org
Try jobs failed on following builders: mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED, http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)
The CQ bit was checked by jkrcal@chromium.org
CQ is trying da patch. Follow status at: https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...
CQ is committing da patch.
Bot data: {"patchset_id": 20001, "attempt_start_ts": 1493272816672280,
"parent_rev": "9ebaba7e8e9888ff16ee524ca4fb18db77224fd1", "commit_rev":
"9591f480aaee3bd50c50e027f028730213ca4a8f"}
Message was sent while issue was closed.
Description was changed from ========== [UMA] Expand the comment for UMA_HISTOGRAM_SPARSE_SLOWLY This CL expands the comment for the UMA_HISTOGRAM_SPARSE_SLOWLY macro. It is inspired by a recent code review discussion with isherman@. BUG=none ========== to ========== [UMA] Expand the comment for UMA_HISTOGRAM_SPARSE_SLOWLY This CL expands the comment for the UMA_HISTOGRAM_SPARSE_SLOWLY macro. It is inspired by a recent code review discussion with isherman@. BUG=none Review-Url: https://codereview.chromium.org/2831543002 Cr-Commit-Position: refs/heads/master@{#467602} Committed: https://chromium.googlesource.com/chromium/src/+/9591f480aaee3bd50c50e027f028... ==========
Message was sent while issue was closed.
Committed patchset #2 (id:20001) as https://chromium.googlesource.com/chromium/src/+/9591f480aaee3bd50c50e027f028... |
