Closed
Bug 732053
Opened 12 years ago
Closed 12 years ago
make linear histograms harder to get wrong
Categories
(Toolkit :: Telemetry, defect)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
mozilla14
People
(Reporter: froydnj, Assigned: froydnj)
Details
Attachments
(1 file, 1 obsolete file)
2.49 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
Due to a quirk (?) in linear histograms, if you want a histogram with a single bucket for each value in 1..N, you need to declare it: HISTOGRAM(NAME, 1, N, N+1, LINEAR, EXPLANATION) and not: HISTOGRAM(NAME, 1, N, N, LINEAR, EXPLANATION) because you need to account for the implicit zero bucket at the bottom. This has come up a couple times for new histograms, and at least GC_REASON seems to suffer from a wrong declaration. SSL telemetry was/is going to use histograms like this, too.
Assignee | ||
Comment 1•12 years ago
|
||
The dead simple patch. Bikeshedding about the name welcome.
Attachment #604111 -
Flags: review?(taras.mozilla)
Assignee | ||
Comment 2•12 years ago
|
||
Histograms that could benefit from this macro: GC_REASON HTTP_*_DISPOSITION CACHE_*_SEARCH (odd to have a linear histogram for ms histogram, especially one so finely bucketed) CACHE_SERVICE_LOCK_WAIT* (same) PLACES_EXPIRATION_STEPS_TO_CLEAN DNS_LOOKUP_METHOD UPDATE_STATUS but most of those would involve a small tweak to the histogram (either in # of buckets or upper bound); doesn't seem worth the effort
Comment 3•12 years ago
|
||
Comment on attachment 604111 [details] [diff] [review] patch HISTOGRAM_SINGLE_VALUE_BUCKET is the best you can come up with? How about HISTOGRAM_CONTINUOUS?
Attachment #604111 -
Flags: review?(taras.mozilla)
Assignee | ||
Comment 4•12 years ago
|
||
HISTOGRAM_CONTINUOUS doesn't sound right. HISTOGRAM_ENUMERATED_VALUES (since most people seem to be sticking enums in these anyway)?
Comment 5•12 years ago
|
||
(In reply to Nathan Froyd (:froydnj) from comment #4) > HISTOGRAM_CONTINUOUS doesn't sound right. HISTOGRAM_ENUMERATED_VALUES > (since most people seem to be sticking enums in these anyway)? r+
Assignee | ||
Comment 6•12 years ago
|
||
Use HISTOGRAM_ENUMERATED_VALUES. Adding r+ per Taras's earlier comment.
Assignee: nobody → nfroyd
Attachment #604111 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #606203 -
Flags: review+
Assignee | ||
Updated•12 years ago
|
Whiteboard: [autoland-try:-b do -p all -u none -t none]
Updated•12 years ago
|
Whiteboard: [autoland-try:-b do -p all -u none -t none] → [autoland-in-queue]
Comment 7•12 years ago
|
||
Autoland Patchset: Patches: 606203 Branch: mozilla-central => try Destination: http://hg.mozilla.org/try/pushloghtml?changeset=a9acecdb826d Try run started, revision a9acecdb826d. To cancel or monitor the job, see: https://tbpl.mozilla.org/?tree=Try&rev=a9acecdb826d
Comment 8•12 years ago
|
||
Try run for a9acecdb826d is complete. Detailed breakdown of the results available here: https://tbpl.mozilla.org/?tree=Try&rev=a9acecdb826d Results (out of 14 total builds): success: 14 Builds (or logs if builds failed) available at: http://ftp.mozilla.org/pub/mozilla.org/firefox/try-builds/autolanduser@mozilla.com-a9acecdb826d
Updated•12 years ago
|
Whiteboard: [autoland-in-queue]
Assignee | ||
Updated•12 years ago
|
Keywords: checkin-needed
Comment 10•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/da0a5a4b0519
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•