1164344 - JS Views should take GC into account

Status: RESOLVED WORKSFORME

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

Description

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

Attachment: gc markers with flames

Once we have sources sync'd, it could be interesting to cross reference our markers, in this case, GC markers, with our sample data from the Profiler.

In the attachment, we see most of the work being done in onNewScript, but looking at the timeline, almost all that time is spent doing GC, and it's not necessarily the fault of onNewScript, but it appears in the calltree/flamegraph as if 80% of the time is in onNewScript (and that's true, it IS, but we should maybe notate in a frame-inspection-zoom or something how much time was in GC).

Comment 1

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

In Chrome, there's a pseudo entry for "(garbage collector)" -- it doesn't seem to have any stack-awareness, just an offset of GC times from the underlying frames.

Comment 2

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

So while we do have frames in the call tree marked GC, we can have GC markers that do not contain any GC samples inside of it, if that makes sense. This could be because the sampler missed the GC frames, but that seems unlikely from experimenting a bit. It also could be that GC occuring in content code does not generate these pseudo frames -- and unsure if they should[0].

Nick, do all GC operations have corresponding profiler pseudo frames?

[0] GC occuring in content code, entire marker is a GC marker, but not listed in call tree. It's not obvious if we should even add a GC frame here, because we want to "blame" this frame. http://i.imgur.com/RNqZTVM.gif

Comment 3

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

I'm not sure where where/how profiler psuedo frames are annotated inside spidermonkey because it doesn't have access to the PROFILER_LABEL macro. Digging in a little more.

Comment 4

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

It looks like the only GCs that get psuedo frame entries are a few DOM triggered paths: https://dxr.mozilla.org/mozilla-central/search?q=js%3A%3AProfileEntry%3A%3ACategory%3A%3AGC&redirect=false&case=false&limit=87&offset=0

It seems to me that in most cases we won't have any psuedo entries for GCs.

Comment 5

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

Dependent on bug 1204169, possibly nothing to do here, but want to make sure its still obvious in the tree which function is causing GC.

Comment 6

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

Attachment: w/out platform data

Comment 7

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

Attachment: w/ platform data

Comment 8

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

As you can see, I have some initial work that makes psuedo frames for all GCs, but the category isn't getting passed through properly so it doesn't help much when not showing gecko platform data.

Comment 9

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

Jordan, did you want to implement some kind of fuzzy sanity test[0] or are we done here?

[0] Something like: start profile -> force gc -> wait for gc marker -> select gc marker range -> assert gc is top leaf node in inverted tree?>

Comment 10

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

I don't think a client side test is necessary unless we elect to show certain un-generalized platform data in leaf nodes (like we talked about showing String.prototype.replace, for example) -- otherwise, it's just retesting platform pseudoframes, which isn't necessary -- the new GC pseudoframes will be categorized as "GC", not "Gecko", right?

Comment 11

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

(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #10)
> the new GC pseudoframes will be categorized as "GC", not "Gecko", right?

Yup!