This timer is the top#8 unlabeled runnable in the latest analysis: https://gist.github.com/bevis-tseng/d8a918cb53112a82357c872ce15ee790 We need to figure out a better way to identify the call sites. arguments.callee.caller or Components.stack.caller might be options to name the callers.

Name the users of setTimeout() using Components.stack.caller.(name|filename).

Comment on attachment 8899785 [details] [diff] [review] (v1) Patch: Name the users of setTimeout() in Timer.jsm. Review of attachment 8899785 [details] [diff] [review]: ----------------------------------------------------------------- ::: toolkit/modules/Timer.jsm @@ +61,4 @@ > } > > this.setTimeout = function setTimeout(aCallback, aMilliseconds, ...aArgs) { > + return _setTimeoutOrIsInterval(Components.stack.caller, This is pretty expensive to call. What if instead we do |Cu.generateXPCWrappedJS(aCallback).QueryInterface(Ci.nsINamed).name|? I haven't tried this, but I think it should work. It will return the name as a string.

(In reply to Bill McCloskey (:billm) from comment #2) > This is pretty expensive to call. What if instead we do > |Cu.generateXPCWrappedJS(aCallback).QueryInterface(Ci.nsINamed).name|? I > haven't tried this, but I think it should work. It will return the name as a > string. Unfortunately, Cu.generateXPCWrappedJS(aCallback) returns a WrappedJSHolder instead which holds an nsXPCWrappedJS accessible in only C++. We'll hit NS_NOINTERFACE because |WrappedJSHolder::QueryInterface()| is called (only nsISupports is identified) instead of |nsXPCWrappedJSClass::DelegatedQueryInterface()|. This seems intended according to the explanation of WrappedJSHolder: http://searchfox.org/mozilla-central/rev/48ea452803907f2575d81021e8678634e8067fc2/js/xpconnect/src/XPCComponents.cpp#3158-3164 BTW, arguments.callee.caller is also forbidden in strict mode functions. Is there any better suggestion?

Maybe I should at least make WrappedJSHolder inheriting nsINamed instead of nsISupports and forward the QueryInterface of nsINamed to nsXPCWrappedJS since all nsXPCWrappedJSClass implements nsINamed.

1. Make nsINamed queriable on WrappedJSHolder. 2. Identify callers via |Cu.generateXPCWrappedJS(aCallback).QueryInterface(Ci.nsINamed).name|. Treeherder result looks fine, btw: https://treeherder.mozilla.org/#/jobs?repo=try&revision=106d8350854056cde1ac0c024d2e930da490e2ae

Comment on attachment 8900643 [details] [diff] [review] (v2) Patch: Name the users of setTimeout() in Timer.jsm. Review of attachment 8900643 [details] [diff] [review]: ----------------------------------------------------------------- Sorry for the delay. I think this is fine, but I'm not sure if these are the right QI macros to use. I'm forwarding to mccr8 who is the expert.

Comment on attachment 8900643 [details] [diff] [review] (v2) Patch: Name the users of setTimeout() in Timer.jsm. Review of attachment 8900643 [details] [diff] [review]: ----------------------------------------------------------------- I don't know too much about QI, but I think it violates COM rules to return a pointer to a different object (ignore that we do that in the CC...). It seems a little weird that WrappedJSHolder is exposed to JS at all.

Olli, do you have any ideas how this could be fixed?

(In reply to Andrew McCreight [:mccr8] from comment #7) > Comment on attachment 8900643 [details] [diff] [review] > (v2) Patch: Name the users of setTimeout() in Timer.jsm. > > Review of attachment 8900643 [details] [diff] [review]: > ----------------------------------------------------------------- > > I don't know too much about QI, but I think it violates COM rules to return > a pointer to a different object (ignore that we do that in the CC...). The Microsoft documentation seems to allow it: https://msdn.microsoft.com/en-us/library/windows/desktop/ms682521(v=vs.85).aspx > It seems a little weird that WrappedJSHolder is exposed to JS at all. I think that's its only purpose. We don't use it in C++.

TearOffs for example return different objects from QI. Looks like we don't have many such anymore. (Though TearOff should QI back to the original object.) This is a bit weird, but xpconnect has all sorts of unusual QI implementations.

Comment on attachment 8900643 [details] [diff] [review] (v2) Patch: Name the users of setTimeout() in Timer.jsm. Review of attachment 8900643 [details] [diff] [review]: ----------------------------------------------------------------- Alright. The macros look okay to me. I mean, maybe this isn't the right way to use them, but at least the underlying code looks reasonable.

(In reply to Bill McCloskey (:billm) from comment #9) > > It seems a little weird that WrappedJSHolder is exposed to JS at all. > > I think that's its only purpose. We don't use it in C++. There is also some explanation of this WrappedJSHolder class in the source file: /* * Below is a bunch of awkward junk to allow JS test code to trigger the * creation of an XPCWrappedJS, such that it ends up in the map. We need to * hand the caller some sort of reference to hold onto (to prevent the * refcount from dropping to zero as soon as the function returns), but trying * to return a bonafide XPCWrappedJS to script causes all sorts of trouble. So * we create a benign holder class instead, which acts as an opaque reference * that script can use to keep the XPCWrappedJS alive and in the map. */ http://searchfox.org/mozilla-central/source/js/xpconnect/src/XPCComponents.cpp#3165-3173

Pushed by btseng@mozilla.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/0dca24b242cf Name the users of setTimeout() in Timer.jsm. r=billm,mccr8

After running the telemetry analysis, most of the unlabeled use of setTimeout in Timer.jsm are in the devtools: 1. http://searchfox.org/mozilla-central/rev/999385a5e8c2d360cc37286882508075fc2078bd/devtools/server/actors/reflow.js#322-323 2. http://searchfox.org/mozilla-central/rev/999385a5e8c2d360cc37286882508075fc2078bd/devtools/server/performance/timeline.js#174-176 It seems that we should either lower or filter out the labeling of this two call sites because devtools are not frequently used by normal users.