Closed
Bug 673837
Opened 13 years ago
Closed 13 years ago
Telemeterize js-compartment-count
Categories
(Toolkit :: Telemetry, defect)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
mozilla8
People
(Reporter: justin.lebar+bug, Assigned: sandervv)
References
Details
(Whiteboard: [MemShrink:P2])
Attachments
(1 file)
2.92 KB,
patch
|
n.nethercote
:
review+
|
Details | Diff | Splinter Review |
Split off from bug 672731 (see bug 672731 comment 4).
That bug is a follow-up to bug 672694, which added the js-compartment-count reporter. From bug 672694 comment 3.
This bug is about hooking this count up to telemetry. njn, do you want to do it?
Updated•13 years ago
|
Whiteboard: [MemShrink]
Updated•13 years ago
|
Whiteboard: [MemShrink] → [MemShrink:P2]
Assignee | ||
Comment 1•13 years ago
|
||
I chose {min: 1, max: 1000, scale: LINEAR} for both compartment statistics. Do you agree? I'm not sure about the scale. Note that low numbers will dominate for these statistics.
What happens if the gathered statistics exceeds the maximum value (1000 in this case); will the data be lost?
Attachment #550583 -
Flags: review?(tglek)
Comment 2•13 years ago
|
||
1000 is way too high. I'd browse around for a bit see what kind of numbers you get then base the range on that. If the stats later show too much on the maximum end of the scale can always increase it later
Comment 3•13 years ago
|
||
I think the data just ends up in the largest bucket, so you don't lose it altogether, you just don't see exactly how much it was.
Comment 4•13 years ago
|
||
I actually think 1000 is reasonable. A decent number of people have hundreds of tabs open, and it's not hard to have multiple compartments per tab. Also, JetPack-based add-ons create compartments like it's going out of style (bug 672443); I saw 305 compartments one time with Bugzilla Tweaks installed when I had only 12 tabs open.
But I suspect it should be EXPONENTIAL, since there's a strong bias towards the lower numbers. That's what EXPONENTIAL is for, right?
Comment 5•13 years ago
|
||
Hmm, but maybe they're not biased enough for EXPONENTIAL... the expected range is only 10^3. So maybe LINEAR is best after all.
Comment 6•13 years ago
|
||
I am guessing that the number of tabs will cluster at <= 100 so EXPONENTIAL seems like a better fit, but that's just a guess. I'll leave this up to the domain experts.
Comment 7•13 years ago
|
||
If we have a min of 1 and max of 1000, what do the bucket ranges look like for LINEAR vs. EXPONENTIAL?
Comment 8•13 years ago
|
||
Comment on attachment 550583 [details] [diff] [review]
Added the histograms for "js-compartments-{user,system}"
I'm going to approve this with LINEAR changed to EXPONENTIAL. I'll land it. Thanks, Sander!
Attachment #550583 -
Flags: review?(tglek) → review+
Comment 9•13 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/30bdc4e72672
That's your first code in a Mozilla repo, Sander :) Assuming there are no test failures, it'll be merged into mozilla-central in the next 24 hours or so.
Whiteboard: [MemShrink:P2] → [MemShrink:P2][inbound]
Comment 10•13 years ago
|
||
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
Whiteboard: [MemShrink:P2][inbound] → [MemShrink:P2]
Target Milestone: --- → mozilla8
Version: unspecified → Trunk
Updated•13 years ago
|
Assignee: nnethercote → sandervv
You need to log in
before you can comment on or make changes to this bug.
Description
•