Once bug 1030389 lands, it needs to be hooked up to the TableTicker so we can actually expose the optimization info. Interface --------- I plan to add an additional property 'ionOpts' (in addition to 'location', 'line', and 'category') to each frame. This property will be initially something like: ionOpts :: [ types, attempts ] types :: type | type, types type :: { site: str, mirType: int } attempts :: attempt | attempt, attempts attempt :: { strategy: str, outcome: int } Currently I plan to add at least constructor:lineno info to the 'type' production above, for exposing TypeObjects. Implementation -------------- Both the types and attempts lists defined above are already stored in a native code-addressed side table in JS land. That is, there's a side table mapping some address to a vector. To save precious circular buffer space, I plan to add a new ProfileEntry tag 'j' that stores an index into this side table. Symbolication (i.e., reading out the types and attempts) is then done during JSON streaming time. This seems pretty threadsafe to me, this side table isn't a fancy data structure and isn't self-modifying during reads.

So, as I tried to hook up this stuff to the profiler, I realized I didn't design the internals with public JSAPI in mind. This refactors some stuff, makes the enums public, and reworks the public API.

Comment on attachment 8557046 [details] [diff] [review] Rework optimization tracking JSAPI to be more usable from the profiler. Review of attachment 8557046 [details] [diff] [review]: ----------------------------------------------------------------- Cancelling review. I'm still iterating.

Comment on attachment 8558293 [details] [diff] [review] Attach optimization info to frames in profile. Review of attachment 8558293 [details] [diff] [review]: ----------------------------------------------------------------- ::: tools/profiler/ProfileEntry.cpp @@ +95,5 @@ > // and union variant fields, so the following was derived > // by looking through all the use points of TableTicker.cpp. > // mTagMarker (ProfilerMarker*) m > // mTagData (const char*) c,s > + // mTagPtr (void*) d,l,L,E,B (immediate backtrace), S(start-of-stack) Please add the name/payload description in parentheses for what the enum type should be. I'm not sure what 'E' stands for. ::: tools/profiler/TableTicker.cpp @@ +586,5 @@ > + // Symbolication of optimization information is delayed until streaming > + // time. To re-lookup the entry in the JitcodeGlobalTable, we need to > + // store both the the return address ('E') and the tracked optimization > + // index ('o') in the circular buffer. > + if (jsFrames[jsIndex].hasTrackedOptimizations) { Nice. This will have a trivial runtime overhead. I'm assuming the js code also adds minimal runtime overhead? If not consider making it an optional feature. Otherwise this is great!

Comment on attachment 8558291 [details] [diff] [review] Rework optimization tracking JSAPI to be more usable from the profiler. Review of attachment 8558291 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/public/TrackedOptimizationInfo.h @@ +278,5 @@ > +JS_PUBLIC_API(void) > +ForEachTrackedOptimizationAttempt(JSRuntime *rt, void *addr, uint8_t index, > + ForEachTrackedOptimizationAttemptOp &op); > + > +struct ForEachTrackedOptimizationTypeInfoOp For this type, could you document what the expected protocol is for sequences of calls to "operator()" vs "readType" that the interface can expect? ::: js/src/jit/OptimizationTracking.h @@ +426,5 @@ > + enum { > + HasNothing, > + HasAllocationSite, > + HasConstructor > + } hasAddendum; Ew. Give this a named type Addendum or something.

Renamed 'E' to 'J' for JIT code address for clarity. This will be landed disabled at the JS level. That is, I will land this but no JS frames will have an optimization info index.

Comment on attachment 8558774 [details] [diff] [review] Attach optimization info to frames in profile. Review of attachment 8558774 [details] [diff] [review]: ----------------------------------------------------------------- Err, hold on, I screwed up rebasing.

Rebased across bhackett's ObjectGroup rename. Carrying r=djvj.

This is burning inbound. https://treeherder.mozilla.org/logviewer.html#?job_id=6239375&repo=mozilla-inbound

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/db6d27b3a8f0 for static analysis bustage, https://treeherder.mozilla.org/logviewer.html#?job_id=6239348&repo=mozilla-inbound

Sorry had to back this out again in https://treeherder.mozilla.org/#/jobs?repo=mozilla-inbound&revision=6ae06b9d651b since one of this changes caused xpcshell test failures like https://treeherder.mozilla.org/logviewer.html#?job_id=6253683&repo=mozilla-inbound

Third time's the charm I hope... Try with a bunch of xpcshell retriggers: https://treeherder.mozilla.org/#/jobs?repo=try&revision=386fdf91c1a3 remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/39422c6d5efc remote: https://hg.mozilla.org/integration/mozilla-inbound/rev/dd1cdbf4be40

https://hg.mozilla.org/mozilla-central/rev/39422c6d5efc https://hg.mozilla.org/mozilla-central/rev/dd1cdbf4be40

https://hg.mozilla.org/projects/cypress/rev/66c90dec344b https://hg.mozilla.org/projects/cypress/rev/f1d372961125 https://hg.mozilla.org/projects/cypress/rev/cde091bad9e8 https://hg.mozilla.org/projects/cypress/rev/fbf1b0195d80 https://hg.mozilla.org/projects/cypress/rev/b2bf96614c44 https://hg.mozilla.org/projects/cypress/rev/df4024b8bb2b