Stop recording Scheduling.ThreadController.*.Any.* in microseconds [chromium/src : main]
0 views
Skip to first unread message
Olivier Li (Gerrit)
Mar 12, 2024, 4:09:39 PM3/12/24
to Egor Pasko, Tricium, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, scheduler...@chromium.org
Attention needed from Egor Pasko
Olivier Li added 1 comment![]( "Open in Gerrit")
Patchset-level comments
File-level comment, Patchset 2 (Latest):
Olivier Li . resolved
Good catch! Assuming a zero time delta is a special case is indeed not very prudent.
Thank you for this fix I appreciate it.
Validation: Tried the patch locally and all histograms seem to be recorded properly.
Related details
Attention is currently required from:
- Egor Pasko
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Olivier Li (Gerrit)
Mar 12, 2024, 4:09:48 PM3/12/24
to Egor Pasko, Tricium, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, scheduler...@chromium.org
Attention needed from Egor Pasko
Olivier Li voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Egor Pasko
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Egor Pasko (Gerrit)
Mar 13, 2024, 4:49:28 AM3/13/24
to Olivier Li, Tricium, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, scheduler...@chromium.org
Egor Pasko added 1 comment![]( "Open in Gerrit")
Patchset-level comments
Olivier Li . resolved
Good catch! Assuming a zero time delta is a special case is indeed not very prudent.
Thank you for this fix I appreciate it.
Validation: Tried the patch locally and all histograms seem to be recorded properly.
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Francois Pierre Doray (Gerrit)
Mar 13, 2024, 12:11:01 PM3/13/24
to Egor Pasko, Alexander Timin, Olivier Li, Tricium, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, scheduler...@chromium.org
Attention needed from Alexander Timin and Egor Pasko
Francois Pierre Doray voted Code-Review+1![]( "Open in Gerrit")
| Code-Review | +1 |
Related details
Attention is currently required from:
- Alexander Timin
- Egor Pasko
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Andrew Paseltiner (Gerrit)
Mar 13, 2024, 1:48:15 PM3/13/24
to Egor Pasko, Francois Pierre Doray, Alexander Timin, Olivier Li, Tricium, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, scheduler...@chromium.org
Attention needed from Alexander Timin and Egor Pasko
Andrew Paseltiner added 1 comment![]( "Open in Gerrit")
Commit Message
Related details
Attention is currently required from:
- Alexander Timin
- Egor Pasko
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Alexander Timin (Gerrit)
Mar 13, 2024, 2:41:38 PM3/13/24
to Egor Pasko, Andrew Paseltiner, Francois Pierre Doray, Olivier Li, Tricium, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, scheduler...@chromium.org
Attention needed from Egor Pasko
Alexander Timin voted Code-Review+1![]( "Open in Gerrit")
Attention is currently required from:
- Egor Pasko
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Egor Pasko (Gerrit)
Mar 13, 2024, 5:02:05 PM3/13/24
to Alexander Timin, Andrew Paseltiner, Francois Pierre Doray, Olivier Li, Tricium, Chromium LUCI CQ, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, scheduler...@chromium.org
Attention needed from Andrew Paseltiner
Egor Pasko voted and added 1 comment![]( "Open in Gerrit")
Attention is currently required from:
- Andrew Paseltiner
Submit Requirements:
Code-Coverage
Code-Owners
Code-Review
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Chromium LUCI CQ (Gerrit)
Mar 13, 2024, 5:56:09 PM3/13/24
to Egor Pasko, Alexander Timin, Andrew Paseltiner, Francois Pierre Doray, Olivier Li, Tricium, Chromium Metrics Reviews, chromium...@chromium.org, asvitkine...@chromium.org, scheduler...@chromium.org
Chromium LUCI CQ submitted the change with unreviewed changes![]( "Open in Gerrit")
Unreviewed changes
2 is the latest approved patch-set.
No files were changed between the latest approved patch-set and the submitted one.
Change information
Commit message:
Stop recording Scheduling.ThreadController.*.Any.* in microseconds
When running content_unittests test AttributionInteropTest I noticed
errors in output, reading like this:
```
Histogram Scheduling.ThreadController.ActiveIntervalDuration.Any.Other
has mismatched construction arguments
```
This was caused by first calling UmaHistogramTimes() and then
UmaHistogramCustomMicrosecondsTimes() with the same histogram name. I
think the reason is that LogIntervalMetric() calls attempt to record a
zero delta, which seems to be coming from lazy_now. Another reason for
zero deltas could be time mocking in the test.
The GetSuffixForHistogram() uses this particular delta to choose the
time suffix "Any". It should rather be "Short" when recording with
UmaHistogramCustomMicrosecondsTimes().
The fix is to use another suffix formatter function for the
UmaHistogramTimes() catch-all recording.
Bug: 328282277
Change-Id: Iab416c93df012b664e25ae5e6aa4dc0cc159b733
Commit-Queue: Egor Pasko <pa...@chromium.org>
Reviewed-by: Francois Pierre Doray <fdo...@chromium.org>
Reviewed-by: Alexander Timin <alt...@chromium.org>
Reviewed-by: Olivier Li <oliv...@chromium.org>
Cr-Commit-Position: refs/heads/main@{#1272448}
Files:
- M base/task/sequence_manager/thread_controller.cc
- M base/task/sequence_manager/thread_controller.h
- M tools/metrics/histograms/metadata/scheduler/histograms.xml
Change size: M
Delta: 3 files changed, 31 insertions(+), 20 deletions(-)
Branch: refs/heads/main
Submit Requirements:
Code-Review: +1 by Olivier Li, +1 by Alexander Timin, +1 by Francois Pierre Doray
| Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings. |
Reply all
Reply to author
Forward
0 new messages