Closed Bug 1164344 Opened 9 years ago Closed 9 years ago

JS Views should take GC into account

Categories

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

41 Branch
defect
Not set
normal

Tracking

(firefox41 affected)

RESOLVED WORKSFORME
Tracking Status
firefox41 --- affected

People

(Reporter: jsantell, Unassigned)

References

Details

Attachments

(3 files)

Attached image 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).
Depends on: 1152829
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.
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
Flags: needinfo?(nfitzgerald)
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.
Flags: needinfo?(nfitzgerald)
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.
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.
Depends on: 1204169
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.
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?>
Flags: needinfo?(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?
Flags: needinfo?(jsantell)
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → WORKSFORME
(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #10)
> the new GC pseudoframes will be categorized as "GC", not "Gecko", right?

Yup!
Product: Firefox → DevTools
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: