501472 - The dromaeo charAt benchmark doesn't trace

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect, P2)

Description

[Boris Zbarsky [:bzbarsky]]

Attachment: test subframe

I'm picking on this one, because tracing it gives us a 20x speedup, as far as I can tell...  At least in shell.  Test files coming up; to test just load the "load this one" test file and see what number is reported.  It should be closer to 2000 than to 100, on a 2.5Ghz macbook pro.  Alternately, it should be in the same ballpark as SFX.

Comment 1

[Boris Zbarsky [:bzbarsky]]

Attachment: test js

Comment 2

[Boris Zbarsky [:bzbarsky]]

Attachment: Testcase. Load this one.

Comment 3

[Boris Zbarsky [:bzbarsky]]

So this doesn't trace for various reasons that all seem to have to do with upvar.  The assignment to ret is one of them.  So's reading str, for some reason.

Sorta-minimalish testcase that tries to exercise all of these issues coming up.  Making it trace is a prereq for making the original test trace, but may not be sufficient.

Please add any relevant bugs as dependencies?

Comment 4

[Boris Zbarsky [:bzbarsky]]

Attachment: js shell testcase

Comment 5

[David Mandelin [:dmandelin]]

In the shell test case, the first abort is a JSOP_BINDNAME on line 11 for |ret|. I was planning on doing JSOP_NAME before JSOP_BINDNAME/SETNAME because I figured it would be easier. Make sense?

Comment 6

[Boris Zbarsky [:bzbarsky]]

Sure; this is a tracking bug, basically.  Just add whatever bugs we think we need to fix to trace this to the "depends on" list.  ;)

Comment 7

[Boris Zbarsky [:bzbarsky]]

With the checkin for bug 495329 the browser and shell testcase here trace (only the inner loop for the former, but still pretty good).  The original dromaeo test, however, doesn't seem to.

I'll to minimize again.

Comment 8

[Boris Zbarsky [:bzbarsky]]

What I think is now the relevant abort:

abort: 6653: fp->scopeChain is not global or active call object
Abort recording of tree file:///Users/bzbarsky/mozilla/dromaeo/web/webrunner.js:100@83 at file:///Users/bzbarsky/mozilla/dromaeo/web/webrunner.js:101@84: callname.

Comment 9

[David Mandelin [:dmandelin]]

I bet it's trying to call a returned closure.

Comment 10

[Boris Zbarsky [:bzbarsky]]

It's a bit more complicated.  I've managed to reproduce it in browser; trying to figure out a way to reproduce in shell now.

What it's doing is pushing a closure into an array and then off a setTimeout shifting it out of the array and calling it.  For some reason the setTimeout is being key so far; I haven't figured out a way to not need it yet.  Still looking, though.

Comment 11

[Boris Zbarsky [:bzbarsky]]

Attachment: js shell testcase that maybe captures that

I agree that this should be equivalent to calling a returned closure.  It might just be that in the original testcase I was always doing the shift in the function that has |var str| in it, and that that was the Call object on the stack and everything was hunky-dory.

In any case, this testcase is certainly closer to what dromaeo is actually doing.

Comment 12

[Boris Zbarsky [:bzbarsky]]

Note that it doesn't quite capture it, since in the original we seem to abort on the call to fn(); I think there's on or two levels more of functions nesting there.  But yeah, let's see how things look after bug 505591.

Comment 13

[Boris Zbarsky [:bzbarsky]]

Ok, with the current patch in bug 505591 (the one that cheats, etc), I now get:

Abort recording of tree file:///Users/bzbarsky/mozilla/dromaeo/web/webrunner.js:100@83 at file:///Users/bzbarsky/mozilla/dromaeo/web/webrunner.js:101@87: call.

This is on the line that calls the test function...

Comment 14

[Boris Zbarsky [:bzbarsky]]

Ah, actually the full abort is:

abort: 10574: JSOP_CALL or JSOP_NEW crosses global scopes
Abort recording of tree file:///Users/bzbarsky/mozilla/dromaeo/web/webrunner.js:100@83 at file:///Users/bzbarsky/mozilla/dromaeo/web/webrunner.js:101@87: call.

So the scopeChain abort is in fact fixed.  But we're still aborting on that function call.  I assume we're tracing the innermost loop, or at least I hope that's where the observed 2x speedup comes from.  But we're still about 5x slower than I'd think we could be (the now-tracing testcase attached to this bug).

Comment 15

[Damon Sicore (:damons)]

Gal is going to take this, but he says "Even in a parallel universe, this isn't going to get fixed for 1.9.2."  Noming for 1.9.3.

Sayre, feel free to beat him down.

Comment 16

[Boris Zbarsky [:bzbarsky]]

The changes for bug 530255 and bug 532477 make this pretty fast in the dromaeo setup, I think.

Comment 17

[Boris Zbarsky [:bzbarsky]]

For what it's worth, I'm tempted to call this fixed.  The bugs mentioned in comment 16 take the charAt score on dromaeo from 200 to about 3000.  One nice thing is that this test is in fact using a JSString, not a String object, due to this prep step after some previous tests:

  str += str;

so we end up specializing the charAt call to js_String_getelem and things are Really Fast.  If I switch to actually forcing a String object here I end up at 800 runs/s or so.

In any case, I think this is fixed.