Last Comment Bug 764190 - PLACES_EXPIRATION_STEPS_TO_CLEAN telemetry might be misleading
: PLACES_EXPIRATION_STEPS_TO_CLEAN telemetry might be misleading
Status: RESOLVED FIXED
:
Product: Firefox
Classification: Client Software
Component: Bookmarks & History (show other bugs)
: Trunk
: All All
: -- normal (vote)
: Firefox 17
Assigned To: Nathan Froyd [:froydnj]
:
Mentors:
Depends on:
Blocks: 763351 763359 764260
  Show dependency treegraph
 
Reported: 2012-06-12 15:36 PDT by Brian Smith (:briansmith, :bsmith, use NEEDINFO?)
Modified: 2012-08-23 19:19 PDT (History)
7 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
-
-
-
-


Attachments
patch (5.00 KB, patch)
2012-08-07 12:57 PDT, Nathan Froyd [:froydnj]
taras.mozilla: review+
mak77: review+
Details | Diff | Review

Description Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-06-12 15:36:57 PDT
+++ This bug was initially created as a clone of Bug #763359 +++

Please see my investigation in bug 763351 and Bug 763359.

A linear histogram with range (1, n) specified to have n buckets will actually have (n+1) buckets with a range slightly less than 1, instead of n buckets with range of 1. Consequently, sometimes two distinct values will get mapped into the same bucket. For example, in bug 763351, the values 2 and 3 both got mapped into the 2 bucket.

Since this probe seems to want to measure an integral number of steps, it seems like a good idea to change the probe to avoid this issue.
Comment 1 Devdatta Akhawe [:devd] 2012-06-12 15:43:40 PDT
How is current code handling this issue? Just creating n+1 buckets?
Comment 2 Brian Smith (:briansmith, :bsmith, use NEEDINFO?) 2012-06-13 13:44:53 PDT
(In reply to dev from comment #1)
> How is current code handling this issue? Just creating n+1 buckets?

Yes, if you are storing counts or other integers in buckets that aren't enums.

For enums, (e.g. SSL cipher suites), you can use HISTOGRAM_ENUMERATED_VALUES.
Comment 3 Marco Bonardo [::mak] 2012-06-14 08:16:53 PDT
likely we may just fix the number of buckets here, the thing we care most is that this measure stays low (under 5), so the actual value has less importance. We will just get more precice infos once the fix gets out in the wild.
Comment 4 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-06-19 11:08:16 PDT
Not sure I understand the tracking 14+ flags here - this isn't something that's crucial to fix ASAP, is it?
Comment 5 Marco Bonardo [::mak] 2012-06-20 07:04:07 PDT
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #4)
> Not sure I understand the tracking 14+ flags here - this isn't something
> that's crucial to fix ASAP, is it?

nope, as I said in comment 3, we may just fix the measure and get new results, but not high priority.
Comment 6 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-06-21 15:17:30 PDT
OK, un-tracking for 14.
Comment 7 Lukas Blakk [:lsblakk] use ?needinfo 2012-07-18 16:11:05 PDT
taking off tracking for 15/16 as well since this isn't a crucial fix.
Comment 8 Nathan Froyd [:froydnj] 2012-08-07 12:57:47 PDT
Created attachment 649766 [details] [diff] [review]
patch

Just use HISTOGRAM_ENUMERATED_VALUES.  It doesn't sound like we're terribly attached to the exact number of buckets here.
Comment 9 (dormant account) 2012-08-07 13:10:46 PDT
Comment on attachment 649766 [details] [diff] [review]
patch

looks reasonable
Comment 10 (dormant account) 2012-08-07 13:12:02 PDT
Comment on attachment 649766 [details] [diff] [review]
patch

however mak is the only who needs to r+ this
Comment 11 Marco Bonardo [::mak] 2012-08-23 01:51:11 PDT
Comment on attachment 649766 [details] [diff] [review]
patch

Review of attachment 649766 [details] [diff] [review]:
-----------------------------------------------------------------

thanks
Comment 13 Ryan VanderMeulen [:RyanVM] 2012-08-23 19:19:26 PDT
https://hg.mozilla.org/mozilla-central/rev/f4e742d57ce7

Note You need to log in before you can comment on or make changes to this bug.