1177939 - Should show aggregate bailout stats in optimizations tab

Status: RESOLVED INVALID

(DevTools :: Performance Tools (Profiler/Timeline), defect, P3)

Description

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Should show the bailouts, and aggregate them by reason for bailout.

Kannan suspects that we are seeing bailouts due to shrinking GCs, but we can't really tell why and to tell how many we had to open a JS shell and pass custom flags and it was too hard.

Additionally, we should overlay GCs on top of the optimizations view to help correlate shrinking GCs with ion bailouts.

Comment 1

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(In reply to Nick Fitzgerald [:fitzgen][:nf] from comment #0)
> Should show the bailouts, and aggregate them by reason for bailout.

Note that we have this info already in the SPS data.

Comment 2

[Jordan Santell [:jsantell] [@jsantell]]

Can someone familiar with bailout data please explain where and what it is

Comment 3

[Jordan Santell [:jsantell] [@jsantell]]

Adding bug 1150292 as a dependency, unless we have all the data we need already, but need guidance from team ion

Comment 4

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #2)
> Can someone familiar with bailout data please explain where and what it is

I know they are in the GP. ni benwa for more.

Comment 5

[Kannan Vijayan [:djvj]]

A quick trace of how these events are materialized.

In tools/profiler/PseudoStack.h, we have:
https://dxr.mozilla.org/mozilla-central/source/tools/profiler/PseudoStack.h#352
  void enableJSSampling() {
    if (mRuntime) {
      js::EnableRuntimeProfilingStack(mRuntime, true);
      js::RegisterRuntimeProfilingEventMarker(mRuntime, &ProfilerJSEventMarker);
      mStartJSSampling = false;
    } else {
      mStartJSSampling = true;
    }
  }

The "RegisterRuntimeProfilingEventMarker" installs a callback that gets invoked with an event string when events are raised from within the JS engine.

The callback in this case calls mozilla_sampler_add_marker in tools/profiler/platform.cpp, which should save a marker stack trace for this and send it to the profiler for handling.

For an example of raising this marker from within the engine, see:  js/src/jit/Ion.cpp, in "jit::Invalidate", we have:

        JS_snprintf(buf, len, "Invalidate %s:%" PRIuSIZE, filename, script->lineno());
        cx->runtime()->spsProfiler.markEvent(buf);
        js_free(buf);

the spsProfiler.markEvent() calls the callback installed by RegisterRuntimeProfilingEventMarker.

The events recorded by this call should be present in the profiler json data that is sent to the frontend.

Comment 6

[Shu-yu Guo [:shu]]

To elaborate on Kannan's comment, the bailout markers are part of the markers in the profile JSON. Those markers aren't used at all by the performance tool right now, which uses the docshell markers instead. Supporting this will first require processing of profile markers.

Comment 7

[Benoit Girard (:BenWa)]

Yes, the above comments describe the current state very well.

I imagine if we're interested in some of them we would want to add something like the profiler_label category and/or just a way to indicate if the marker belongs for the platform/devtools/both profiling.

Or you guys can just manually white-list some in your frontend.

Comment 8

[Jordan Santell [:jsantell] [@jsantell]]

I think that's a good plan for now -- we can probably just parse out profiler markers that we want in devtools for now pretty easily without having to change the profiler_label categories.

Comment 9

[Kannan Vijayan [:djvj]]

Attachment: emit-debugger-deopt-events.patch

Was talking to :fitzgen, and he noted that the ion-optimization events currently used by the devtools performance frontend uses information forwarded through a debugger API.

Looked through that, and wrote a patch for deoptimization info to be forwarded the same way.  :fitzgen suggested that this might be easier for devtools to deal with, so I wrote it.

The upside with the SPS-profiler based mechanism that already exists is that you get a full stack trace with the event.  The debugger-based mechanism doesn't provide that.

Still, putting the patch up in case it turns out to be useful.

Comment 10

[Jordan Santell [:jsantell] [@jsantell]]

I'll check it out; I started a patch that handles profiler markers, expanding is straight forward. Difficulty is, most just have a string of information, so would require a handful of parsing and ensuring that anything with "bailout" in it can be considered a bailout marker (and if that assumption is valid)

Comment 11

[Benoit Girard (:BenWa)]

You should create a custom marker payload type which you can implement a custom JSON format. This will avoid parsing:

http://mxr.mozilla.org/mozilla-central/source/tools/profiler/ProfilerMarkers.cpp

Comment 12

[Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8]]

Making bug 1150292 for showing bailouts on top of the tier graph. Making this for aggregating stats about bailouts, eg 45% (9 count) of bailouts were do to compacting GC throwing away jit code.

Comment 13

[Victor Porof [:vporof][:vp]]

Triaging. Filter on ADRENOCORTICOTROPIC (yes).

Comment 14

[Greg Tatum [:gregtatum]]

This bug is no longer valid as this code was removed in Bug 1614622