971420 - Error: TelemetryStopwatch when open about:healthreport

Status: RESOLVED WORKSFORME

(Firefox Health Report Graveyard :: Client: Desktop, defect, P3)

Description

[Alice0775 White]

Timestamp: 2014/02/12 11:25:50
Error: TelemetryStopwatch: key "HEALTHREPORT_COLLECT_CONSTANT_DATA_MS" was already initialized
Source File: resource://gre/modules/TelemetryStopwatch.jsm
Line: 53
Timestamp: 2014/02/12 11:25:50
Error: TelemetryStopwatch: key "HEALTHREPORT_POST_COLLECT_CHECKPOINT_MS" was already initialized
Source File: resource://gre/modules/TelemetryStopwatch.jsm
Line: 53
Timestamp: 2014/02/12 11:25:51
Error: TelemetryStopwatch: key "HEALTHREPORT_GENERATE_JSON_PAYLOAD_MS" was already initialized
Source File: resource://gre/modules/TelemetryStopwatch.jsm
Line: 53

And number of addons and plugins are reproted 0.

Comment 1

[Alice0775 White]

Regression window(m-i)
Good:
http://hg.mozilla.org/integration/mozilla-inbound/rev/a41fdfce8810
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140130113333
Bad:
http://hg.mozilla.org/integration/mozilla-inbound/rev/c78f9d502f0f
Mozilla/5.0 (Windows NT 6.1; WOW64; rv:29.0) Gecko/20100101 Firefox/29.0 ID:20140130113949
Pushlog:
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=a41fdfce8810&tochange=c78f9d502f0f

Regressed by:
c78f9d502f0f	Georg Fritzsche — Bug 928575 - Overhaul FHR data collection for extensions and plugins for desktop. r=gps

Comment 2

[Alice0775 White]

About number of addons and plugins are reproted 0. 
It is Bug 966871 .

So, this bug only about the error indication.

Comment 3

[Benjamin Smedberg]

This is because the telemetry-stopwatch only expects one measurement per session and we're loading the FHR data multiple times per session. This needs to be a histogram of some sort if we're going to have multiple measurements per session.

Not a high priority: gps/gfritzsche; either of you want to be a mentor for this bug?

Comment 4

[Gregory Szorc [:gps]]

I thought you could take multiple measurements per session and each one was recorded discretely! The docs for the "aObj" argument (http://dxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/TelemetryStopwatch.jsm?from=TelemetryStopwatch.jsm#20) seem to indicate this.

I think the problem is we aren't locking around collectMeasurements().

Comment 5

[Benjamin Smedberg]

I read http://dxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/TelemetryStopwatch.jsm?from=TelemetryStopwatch.jsm#34 to mean the exact opposite thing. Maybe vladan can clarify.

Comment 6

[Vladan Djeric (:vladan)]

This isn't a Telemetry restriction, it's just the way TelemetryStopwatch.jsm works.

You can take multiple measurements with TelemetryStopwatch.jsm in the same session, but each call to TelemetryStopwatch.start(<id>) needs to be followed by a call to TelemetryStopwatch.cancel(<id>) or TelemetryStopwatch.finish(<id>).

It doesn't really make sense to have two consecutive .start() calls for the same histogram ID because it wouldn't be clear which starting times are associated with which end times.

Comment 7

[Georg Fritzsche [:gfritzsche]]

So apparently we have overlapping tasks (and hence TelemetryStopWatches) running from collectMeasurements() and getJSONPayload()?
Is this actually expected to happen in some scenario and we are fine with locking or some other ordering mechanism?

Comment 8

[Benjamin Smedberg]

Is there an "aObj" context param to start() and stop() which we can use?

Comment 9

[Vladan Djeric (:vladan)]

(In reply to Benjamin Smedberg  [:bsmedberg] from comment #8)
> Is there an "aObj" context param to start() and stop() which we can use?

There is: http://dxr.mozilla.org/mozilla-central/source/toolkit/components/telemetry/TelemetryStopwatch.jsm#29

Comment 10

[Benjamin Smedberg]

Sorry, that question was for gfritzsche whether in our use case there was a good context object we could use... I should learn to use NEEDINFO!

Comment 11

[Gregory Szorc [:gps]]

We should ensure collectAndObtainJSONPayload() can't be called concurrently. Stash the promise from the task and have concurrent calls return the in-progress promise.

Comment 12

[Georg Fritzsche [:gfritzsche]]

That definitely sounds better, i should be fine mentoring it.

Alice0775, are there specific steps needed to reproduce this?

Comment 13

[Anup Kumar [:reznord]]

Hi, I am interested in working on this bug. So please can u assign this bug to me?

Thanks in Advance,

Regards,
A. Anup

Comment 14

[Georg Fritzsche [:gfritzsche]]

I think this bug is not the best one for you right now, let's check out bug 847072 later.

Comment 15

[Georg Fritzsche [:gfritzsche]]

FHR is going away per bug 1209088, closing.