For bug 1121013, I'd like to add the telemetry environment data as an annotation on crash reports. This will not only help to unify analysis across telemetry and crash-stats, but it will also be a way to submit a useful crash ping based on the same data.

Comment on attachment 8581664 [details] [diff] [review] 1140132-annotate-environment Review of attachment 8581664 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/telemetry/TelemetryStartup.js @@ +21,5 @@ > > TelemetryStartup.prototype.classID = Components.ID("{117b219f-92fe-4bd2-a21b-95a342a9d474}"); > TelemetryStartup.prototype.QueryInterface = XPCOMUtils.generateQI([Components.interfaces.nsIObserver]) > TelemetryStartup.prototype.observe = function(aSubject, aTopic, aData) { > if (aTopic == "profile-after-change" || aTopic == "app-startup") { "profile-after-change" is registered for chrome processes, "app-startup" for content processes. We probably only want to do the annotations in the chrome process?

Comment on attachment 8581664 [details] [diff] [review] 1140132-annotate-environment Review of attachment 8581664 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/components/telemetry/TelemetryStartup.js @@ +35,5 @@ > + try { > + let cr = Cc["@mozilla.org/toolkit/crash-reporter;1"] > + .getService(Ci.nsICrashReporter); > + let env = JSON.stringify(TelemetryEnvironment.currentEnvironment); > + cr.annotateCrashReport("Environment", env); I think TelemetryEnvironment would be more descriptive here.

This ran into xperf main-thread-I/O issues. 09:00:57 INFO - TEST-UNEXPECTED-FAIL : xperf: File '{profile}\secmod.db' was accessed and we were not expecting it. DiskReadCount: 6, DiskWriteCount: 0, DiskReadBytes: 16904, DiskWriteBytes: 0 09:00:57 INFO - TEST-UNEXPECTED-FAIL : xperf: File '{profile}\cert8.db' was accessed and we were not expecting it. DiskReadCount: 4, DiskWriteCount: 0, DiskReadBytes: 33288, DiskWriteBytes: 0 09:00:57 INFO - TEST-UNEXPECTED-FAIL : xperf: File '{profile}\key3.db' was accessed and we were not expecting it. DiskReadCount: 4, DiskWriteCount: 0, DiskReadBytes: 8712, DiskWriteBytes: 0 With this c++ stack: > xul.dll!EnsureNSSInitialized(EnsureNSSOperator op) Line 107 C++ xul.dll!`anonymous namespace'::nsCryptoHashConstructor(nsISupports * aOuter, const nsID & aIID, void * * aResult) Line 199 C++ xul.dll!mozilla::GenericFactory::CreateInstance(nsISupports * aOuter, const nsID & aIID, void * * aResult) Line 17 C++ xul.dll!nsComponentManagerImpl::CreateInstance(const nsID & aClass, nsISupports * aDelegate, const nsID & aIID, void * * aResult) Line 1146 C++ xul.dll!nsJSCID::CreateInstance(JS::Handle<JS::Value> iidval, JSContext * cx, unsigned char optionalArgc, JS::MutableHandle<JS::Value> retval) Line 651 C++ xul.dll!NS_InvokeByIndex(nsISupports * that, unsigned int methodIndex, unsigned int paramCount, nsXPTCVariant * params) Line 71 C++ And this JS stack: 0 getIDHashForString(aStr = "Adobe AcrobatAdobe PDF Plug-In For Firefox and Nets cape 11.0.10") ["resource://gre/modules/addons/PluginProvider.jsm":33] 1 PL_getPluginList() ["resource://gre/modules/addons/PluginProvider.jsm":198] this = [object Object] 2 PL_buildPluginList() ["resource://gre/modules/addons/PluginProvider.jsm":219] this = [object Object] 3 PL_getAddonsByTypes(aTypes = plugin, aCallback = [function]) ["resource://gre/ modules/addons/PluginProvider.jsm":144] this = [object Object] 4 callProviderAsync(aProvider = [object Object], aMethod = "getAddonsByTypes", a Args = plugin,function getAddonsByTypes_concatAddons(aProviderAddons) { "use strict"; if (aProviderAddons) { addons = addons.concat(aProviderAddons); } aCaller.callNext(); }, [function]) ["resource://gre/modules/AddonManager.jsm":235] 5 getAddonsByTypes_nextObject(aCaller = [object Object], aProvider = [object Obj ect]) ["resource://gre/modules/AddonManager.jsm":2226] this = [object Object] 6 AOC_callNext() ["resource://gre/modules/AddonManager.jsm":311] this = [object Object] 7 getAddonsByTypes_concatAddons(aProviderAddons = [object Object],[object Object ]) ["resource://gre/modules/AddonManager.jsm":2231] 8 GMPProvider.getAddonsByTypes(aTypes = plugin, aCallback = [function]) ["resour ce://gre/modules/addons/GMPProvider.jsm":560] this = [object Object] 9 callProviderAsync(aProvider = [object Object], aMethod = "getAddonsByTypes", a Args = plugin,function getAddonsByTypes_concatAddons(aProviderAddons) { "use strict"; if (aProviderAddons) { addons = addons.concat(aProviderAddons); } aCaller.callNext(); }, [function]) ["resource://gre/modules/AddonManager.jsm":235] 10 getAddonsByTypes_nextObject(aCaller = [object Object], aProvider = [object Ob ject]) ["resource://gre/modules/AddonManager.jsm":2226] this = [object Object] 11 AOC_callNext() ["resource://gre/modules/AddonManager.jsm":311] this = [object Object] I'm surprised that the crypto-hash code needs to do the full initialization of NSS, but this will clearly happen right *near* startup anyway, if the user is visiting SSL sites. Are we willing to accept this main-thread I/O hit for these files anyway, on the theory that we're going to end up doing this main-thread I/O anyway? Avi, are you the right person to make this decision? If we do accept this, what's the right procedure to get these files added to the xperf whitelist?

Backed out for xperf regressions. https://hg.mozilla.org/integration/fx-team/rev/dfcd9278ed70 https://treeherder.mozilla.org/logviewer.html#?job_id=2841963&repo=fx-team

(In reply to Benjamin Smedberg [:bsmedberg] from comment #5) > ... > I'm surprised that the crypto-hash code needs to do the full initialization > of NSS, but this will clearly happen right *near* startup anyway, if the > user is visiting SSL sites. Are we willing to accept this main-thread I/O > hit for these files anyway, on the theory that we're going to end up doing > this main-thread I/O anyway? Avi, are you the right person to make this > decision? Typically Vladan and I _think Aaron help with the main thread IO decisions. I'm pretty sure there wasn't and isn't a hard threshold as for what should be considered acceptable. It's a judgment call which tries to assess the overall usefulness of one approach vs another. That being said, from what I understand so far this isn't a new thing, so it's not a "Regression", but rather a situation which needs to be assessed if it can be improved or not. Aaron, any insights on this?

One of the common things that we see is that something that previously caused main thread I/O outside of startup (where in the case of xperf, this means past the sessionstore-windows-restored cutoff point) is moved earlier such that it now falls under the scope of xperf. When they can be easily avoided, we like to do so, but sometimes we just have to take the hit. NSS seems to me like an instance of the latter. I'd be ok with whitelisting these.

Comment on attachment 8598171 [details] [diff] [review] 1140132-nss Review of attachment 8598171 [details] [diff] [review]: ----------------------------------------------------------------- Once this lands in Talos you'll also need to update the talos revision in m-c. See bug 966347.

https://hg.mozilla.org/integration/fx-team/rev/d0f4e0591fa2 https://hg.mozilla.org/integration/fx-team/rev/40520b2ca6b8

Backout: https://hg.mozilla.org/integration/fx-team/rev/dd45b61c143b https://hg.mozilla.org/integration/fx-team/rev/9c95109b241b

backed this out in case this cause also https://treeherder.mozilla.org/logviewer.html#?job_id=2868187&repo=fx-team