Closed
Bug 682869
Opened 13 years ago
Closed 13 years ago
add Telemetry::AccumulateTimeDelta and use it
Categories
(Toolkit :: Telemetry, defect)
Toolkit
Telemetry
Tracking
()
RESOLVED
FIXED
mozilla10
People
(Reporter: froydnj, Assigned: froydnj)
Details
Attachments
(2 files, 3 obsolete files)
1.43 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
11.27 KB,
patch
|
froydnj
:
review+
|
Details | Diff | Splinter Review |
There are a number of places where telemetry probes are used to accumulate millisecond intervals. We should have a separate function to make it obvious what's going on.
Assignee | ||
Comment 1•13 years ago
|
||
Here's a patch to add the function and update appropriate callers. I was unsure of whether to do the two things as separate patches, or one big ball of mud. I'm sure somebody will straighten me out. I'm also not sure who to pester to look at tweaks like this.
Assignee | ||
Comment 2•13 years ago
|
||
Comment on attachment 556583 [details] [diff] [review] patch to add AccumulateTimeDelta and update callsites Taras, can you review the telemetry parts and/or point me at who else should be reviewing the rest?
Attachment #556583 -
Flags: review?(tglek)
Comment 3•13 years ago
|
||
Comment on attachment 556583 [details] [diff] [review] patch to add AccumulateTimeDelta and update callsites Next time please submit api change + code updates in 2 separate patches. As we discussed on irc, instead of passing TimeStamp::Now() use default parameter value or method overloading that implies Now().
Attachment #556583 -
Flags: review?(tglek) → review-
Assignee | ||
Comment 4•13 years ago
|
||
Attachment #565216 -
Flags: review?(tglek)
Assignee | ||
Comment 5•13 years ago
|
||
Assignee | ||
Updated•13 years ago
|
Attachment #556583 -
Attachment is obsolete: true
Comment 6•13 years ago
|
||
Comment on attachment 565216 [details] [diff] [review] introduce AccumulateTimeDelta >+void AccumulateTimeDelta(ID id, TimeStamp start, TimeStamp end=TimeStamp::Now()); Add spaces around =, r+ with that
Attachment #565216 -
Flags: review?(tglek) → review+
Assignee | ||
Comment 7•13 years ago
|
||
Comment on attachment 565217 [details] [diff] [review] update places to use AccumulateTimeDelta Flagging people for review on the use-it-everywhere patch.
Attachment #565217 -
Flags: review?(tglek)
Attachment #565217 -
Flags: review?(khuey)
Attachment #565217 -
Flags: review?(jduell.mcbugs)
Updated•13 years ago
|
Attachment #565217 -
Flags: review?(tglek) → review+
Comment 8•13 years ago
|
||
Comment on attachment 565217 [details] [diff] [review] update places to use AccumulateTimeDelta Review of attachment 565217 [details] [diff] [review]: ----------------------------------------------------------------- Good stuff. Thanks Nathan.
Attachment #565217 -
Flags: review?(jduell.mcbugs) → review+
Comment on attachment 565217 [details] [diff] [review] update places to use AccumulateTimeDelta Review of attachment 565217 [details] [diff] [review]: ----------------------------------------------------------------- Looks reasonable enough to me.
Attachment #565217 -
Flags: review?(khuey) → review+
Assignee | ||
Comment 10•13 years ago
|
||
Updated with taras's requested spaces and real commit info. Carrying over r+.
Attachment #565216 -
Attachment is obsolete: true
Attachment #565966 -
Flags: review+
Assignee | ||
Comment 11•13 years ago
|
||
Updated patch with r= info and such. Carrying over r+.
Attachment #565217 -
Attachment is obsolete: true
Attachment #565967 -
Flags: review+
Assignee | ||
Comment 12•13 years ago
|
||
Think everything's GTG for checkin-needed. Flagging.
Keywords: checkin-needed
Updated•13 years ago
|
Assignee: nobody → nfroyd
Comment 13•13 years ago
|
||
http://hg.mozilla.org/integration/mozilla-inbound/rev/d6b9a54956c0 http://hg.mozilla.org/integration/mozilla-inbound/rev/e4edd20737fb
Keywords: checkin-needed
Target Milestone: --- → mozilla10
Comment 14•13 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/d6b9a54956c0 https://hg.mozilla.org/mozilla-central/rev/e4edd20737fb
Status: NEW → RESOLVED
Closed: 13 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•