1074935 - Push psuedo-stack frames for js rope string flattening

Status: RESOLVED FIXED

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

Description

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

I got a huge speed up today for replacing a simple regexp with a charAt. A potential theory for why it was such a boon is that the regexp was flattening the js string. It could also be that charAt was inlined while the regexp wasn't, but that's besides the point.

The point is that flattening can be expensive for large ropes, and we have no insight from devtools into when it does or doesn't happen. It seems like the timeline is the right place to add this.

Comment 1

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

I think it would make sense to gate based on string length, and only add markers for large strings.

Comment 2

[Patrick Brosset <:pbro>]

Another idea is: when you click on any marker in the timeline, it'd be nice to have more information about it, like the time spent, the associated callstack, etc. If we know some specific actions (like string flattening) happened during a script execution marker in the timeline, we could mention it next to it, in the sidebar or something.

Comment 3

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

Moving into the Profiler component. Filter on GUTHRIE'S WAVY CAKES.

Comment 4

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

After talking with Shu and/or Kannan a little while back, I believe that we decide that pushing a psuedostack frame made more sense.

Comment 6

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

Anyone know where in platform code this occurs?

Comment 7

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

(In reply to Jordan Santell [:jsantell] [@jsantell] from comment #6)
> Anyone know where in platform code this occurs?

https://dxr.mozilla.org/mozilla-central/source/js/src/vm/String.h#1252

Comment 8

[Jan de Mooij [:jandem]]

(In reply to Nick Fitzgerald [:fitzgen][:nf] from comment #7)
> (In reply to Jordan Santell [:jsantell] [@jsantell] from comment #6)
> > Anyone know where in platform code this occurs?
> 
> https://dxr.mozilla.org/mozilla-central/source/js/src/vm/String.h#1252

(I'd use JSRope::flatten; ensureLinear also calls JSRope::flatten and is a lot more common than ensureFlat.)

Comment 9

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

(In reply to Jan de Mooij [:jandem] from comment #8)
> (In reply to Nick Fitzgerald [:fitzgen][:nf] from comment #7)
> > (In reply to Jordan Santell [:jsantell] [@jsantell] from comment #6)
> > > Anyone know where in platform code this occurs?
> > 
> > https://dxr.mozilla.org/mozilla-central/source/js/src/vm/String.h#1252
> 
> (I'd use JSRope::flatten; ensureLinear also calls JSRope::flatten and is a
> lot more common than ensureFlat.)

Ah, thanks!

Comment 10

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

Attachment: Add SPS psuedo frames for JSRope flattening

Comment 11

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/b4621131ea01

Comment 12

[Wes Kocher (:KWierso) (Not reading bugmail; email directly if needed)]

Backed out for Werror bustage along with bug 1204169:

https://hg.mozilla.org/integration/mozilla-inbound/rev/60adc85f21cb
https://treeherder.mozilla.org/logviewer.html#?job_id=14289237&repo=mozilla-inbound

Comment 13

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

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

Comment 14

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/cce8e013f935

Comment 15

[Phil Ringnalda (:philor)]

https://hg.mozilla.org/mozilla-central/rev/cce8e013f935