1063561 - Convert Addon Manager telemetry timings to use Components.utils.now()

Status: RESOLVED FIXED

(Toolkit :: Add-ons Manager, defect)

Description

[:Irving Reid (No longer working on Firefox)]

Bug 969490 gave us a better time base to use for interval measurements, we should switch to it for our telemetry time interval measurements in the Addon Manager, XPI Provider, etc. Anywhere we record two "new Date()" or "Date.now()" values and subtract them, we should switch to Components.utils.now().

Note that Components.utils.now() returns floating point milliseconds with microsecond precision, and our existing Telemetry expects an integer milliseconds, so the time interval should be rounded with Math.round()

Comment 1

[Gopal Singh Meena]

I am new to bugzilla and I will like to work on this bug. I have setup mozilla central. Can you give me further instructions ?

Comment 2

[:Irving Reid (No longer working on Firefox)]

Thanks for taking a look at this. I may have set this bug for mentoring a bit too soon, because I'm in the middle of making changes to some of the Addon Manager files, in ways that overlap with what needs to be fixed for this bug. You're welcome to go ahead and work on this, but it might be easier to work on bug 1063559, which is the same sort of change but in a different module.

Once you can build and run your own copy of Firefox (see the instructions at https://developer.mozilla.org/en/docs/Simple_Firefox_build), the files that need to be changed are in the toolkit/mozapps/extensions directory of mozilla-central.

In toolkit/mozapps/extensions/AddonManager.jsm we use Date.now() to calculate a time interval in the 'simpleTimer' method (around line 2433). The Date.now() calls need to be replaced with Components.utils.now() and the calculated time interval needs to be rounded as described in comment 1.

toolkit/mozapps/extensions/internal/XPIProvider.jsm is the other file in this module where we calculate time intervals to record as Telemetry measurements, but I'm in the middle of changing all of the places we do this, as part of bug 1049142, so you should wait until those changes have been made before editing XPIProvider.jsm.

Comment 3

[Gopal Singh Meena]

Ok,cool
No problem

Thanks for a good suggestion

Comment 4

[:Irving Reid (No longer working on Firefox)]

The bug that was blocking this has now landed, so if someone wants to start working on this it's ready to go.

Comment 5

[Manu Jain]

Attachment: bug1063561.patch

I've attached the patch. Please check it. Thanks!!

Comment 6

[:Irving Reid (No longer working on Firefox)]

Comment on attachment 8491462 [details] [diff] [review]
bug1063561.patch

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

::: toolkit/mozapps/extensions/AddonManager.jsm
@@ +2430,5 @@
>  
>    // Start a timer, record a simple measure of the time interval when
>    // timer.done() is called
>    simpleTimer: function(aName) {
> +    let startTime = Components.utils.now();

I didn't say this in the bug description, but in most Firefox JS code we define a shortcut for Components.utils; see http://dxr.mozilla.org/mozilla-central/source/toolkit/mozapps/extensions/AddonManager.jsm?from=AddonManager.jsm#10

so throughout this patch you can use Cu.now() instead of Components.utils.now()

::: toolkit/mozapps/extensions/internal/XPIProvider.jsm
@@ +2215,5 @@
>     */
>    getAddonStates: function XPI_getAddonStates(aLocation) {
>      let addonStates = {};
>      for (let file of aLocation.addonLocations) {
> +      let scanStarted = Components.utils.now();

Oops, you must have worked from a copy of mozilla-central that wasn't updated with the changes from bug 1049142 yet. You'll need to undo the changes in XPIProvider.jsm, merge in the latest mozilla-central changes, and then modify the new places where Date.now() is used. If you need help with hg to get through that, it's best to ask on IRC in irc.mozilla.org:#introduction

Comment 7

[Manu Jain]

Attachment: bug_1063561_v2.patch

All changes made.

Comment 8

[:Irving Reid (No longer working on Firefox)]

Comment on attachment 8491697 [details] [diff] [review]
bug_1063561_v2.patch

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

Thanks for the patch!

Comment 9

[:Irving Reid (No longer working on Firefox)]

https://hg.mozilla.org/integration/fx-team/rev/92d6f9e6cbc3

Comment 10

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

https://hg.mozilla.org/mozilla-central/rev/92d6f9e6cbc3