1242270 - Add SPS pseudo frames for the Array.prototype methods

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

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

This commit adds SPS pseudo frames for the Array.prototype methods that are not
self-hosted. This provides better insight into where time is spent when
profiling. Self-hosted methods do not need special treatment, as they will be
seen by the usual JS stack sampling.

Comment 1

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

Attachment: Add SPS pseudo frames for the Array.prototype methods

Comment 2

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

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=7bec5558421f

Comment 3

[Jan de Mooij [:jandem]]

Should we come up with a more generic solution that works for all natives, inside and outside SpiderMonkey?

Comment 4

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

(In reply to Jan de Mooij [:jandem] from comment #3)
> Should we come up with a more generic solution that works for all natives,
> inside and outside SpiderMonkey?

That would be a *really* great thing to have, but I don't think we need to block this patch until we have a more generic solution in place.

I was thinking that we should modify the webidl bindings generator to emit these pseudo frames, but I haven't looked into it yet. Is there a better place to hook in? Somewhere that would catch all SM internal native methods as well?

Comment 5

[Shu-yu Guo [:shu]]

(In reply to Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8] from comment #4)
> (In reply to Jan de Mooij [:jandem] from comment #3)
> > Should we come up with a more generic solution that works for all natives,
> > inside and outside SpiderMonkey?
> 
> That would be a *really* great thing to have, but I don't think we need to
> block this patch until we have a more generic solution in place.
> 
> I was thinking that we should modify the webidl bindings generator to emit
> these pseudo frames, but I haven't looked into it yet. Is there a better
> place to hook in? Somewhere that would catch all SM internal native methods
> as well?

For script-used natives that get native JSFunction wrappers, we could wrap the native again with something that automatically pushes a pseudoframe. E.g., JS_PROFILABLE_FUN or whatevs. Should be pretty easy, actually. Wanna take a stab, Nick?

Comment 6

[Jan de Mooij [:jandem]]

(In reply to Nick Fitzgerald [:fitzgen] [⏰PST; UTC-8] from comment #4)
> That would be a *really* great thing to have, but I don't think we need to
> block this patch until we have a more generic solution in place.

Sure.

> I was thinking that we should modify the webidl bindings generator to emit
> these pseudo frames, but I haven't looked into it yet. Is there a better
> place to hook in? Somewhere that would catch all SM internal native methods
> as well?

Hm. Inside SM we're self-hosting more and more natives, so that's great for the profiler.

If we can wrap the natives, as shu suggested, please keep an eye on libxul binary size; in the browser we generate a ton of natives...

Comment 7

[Shu-yu Guo [:shu]]

Wrapping in an easy-to-use fashion is harder than expected, since string literals can't be used as template parameters.

Comment 8

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/831f45255022

Comment 9

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/831f45255022