Closed Bug 572310 Opened 14 years ago Closed 14 years ago

JM: Port PICStubCompiler to fatval

Categories

(Core :: JavaScript Engine, defect)

Product:
Core
Component:
JavaScript Engine
Platform:
x86
macOS
Type:
defect
Priority:
Not set
Severity:
normal

Tracking

()

Status:
RESOLVED FIXED

People

(Reporter: dvander, Assigned: dvander)

Details

Last *really big* remaining win to port from the old compiler. This will fast-path GETPROP, SETPROP, and CALLPROP with polymorphic inline caches.
Part 2: GETPROP, special case for length atom with strings and arrays.

http://hg.mozilla.org/users/danderson_mozilla.com/moo/rev/aeec639233c3

The string portion alone was an overall slow down, particularly on date-format-tofte. Most string.length accesses occur on object strings, rather than primitives. There is about a 6ms to be gained back, and it shouldn't be too hard to add another special stub case.
David, can you check that JM is not making String object wrappers when it should not -- when a primitive string is the left operand of . or [ in particular? Most string.length accesses are on primitives in real code, and (I thought) even in SS.

/be
(In reply to comment #3)
Real code, I suspect, does not call "new String" everywhere like tofte :(

I think that's the only reason.
Part 3: SETPROP http://hg.mozilla.org/users/danderson_mozilla.com/moo/rev/48f71ddbbbac

Unfortunately, this is a wash - at worst a tiny regression. It slows down earley-boyer and raytrace significantly, and speeds up richards. I've disabled it for now.

From looking at the PIC spew, I believe the reason is that we can't cache adding properties, and the slow path really hurts us. Having faster slow paths will help; more intelligent ICs would too. 

Next up is CALLPROP which should prove better.
Okay, I was able to hide this loss by patching the slow path to use the property cache instead. raytrace still showed about a small (~10%) loss, but overall there was an ~8% win.
(In reply to comment #5)
> Part 3: SETPROP
> http://hg.mozilla.org/users/danderson_mozilla.com/moo/rev/48f71ddbbbac
> 
> Unfortunately, this is a wash - at worst a tiny regression. It slows down
> earley-boyer and raytrace significantly, and speeds up richards. I've disabled
> it for now.
> 
> From looking at the PIC spew, I believe the reason is that we can't cache
> adding properties, and the slow path really hurts us

Confusion: why can't you cache adding properties? The property cache does. Or is that what you are using in the slow path(s)?

/be
s/can't/don't, I'm (mostly) doing a straight port of the original PIC code for now, and it doesn't handle property additions.

My mistake was when disabling the PIC for this case, I used obj->setProperty, instead of the function that uses the property cache.
A few minor complications. The first:

  function f(x) {
    return x.push;
  }
  f([]);
  f([]);

The old PIC code guarded on obj->map->shape, which can be SHAPELESS. The new code guards on obj->shape(), which asserts not SHAPELESS, so this test asserts. We should instead guard on whether or not it's a dense array, and then sneak in its prototype instead.

The second, CALLPROP seems to fall into three cases for our compiler:
1) We know the type of LHS, and it's an object. In this case, it becomes GETPROP.
2) We know the type of the LHS, and it's a string, and we're compile n' go. We can guard on String.prototype and bake in the function.
3) We don't know the type. The function to patch becomes significantly more convoluted the LHS is not guaranteed to be the end |vp[1]|. We have to save remat info for the LHS so each stub can write it back.

Third, CALLPROP has this __noSuchMethod__ callback. I'm not sure how to handle it. Probably the sanest thing is to check if this callback exists on the LHS, and if so, guard that the rval is !undefined at the bottom of the stub. This should work because of the shape guard.

So, estimating around 3 days to finish CALLPROP.
(In reply to comment #7)
> (In reply to comment #5)
> > From looking at the PIC spew, I believe the reason is that we can't cache
> > adding properties, and the slow path really hurts us
> 
> Confusion: why can't you cache adding properties? The property cache does. Or
> is that what you are using in the slow path(s)?

Bug 561506 has a lot of discussion on this. The short story is that adding properties with a cached lookup is still complex enough to usually require a slow call, and the available win is not that big. We'll want to do it someday but it is not a high priority. Also, I expect it to become easier with the changes to objects and scopes.
Nits, hope it's ok to leave them here:

+        // If the scope is branded, or has a method barrier. It's now necessary

"... barrier, it's now necessary"

+        // to guard that we're not overwriting a function-valued property.
+        Jump rebrand;
+        JSScope *scope = obj->scope();
+        if (scope->branded() || scope->hasMethodBarrier()) {

Test scope->brandedOrHasMethodBarrier() instead.

Same goes for followup patch.


/be
Thanks, fixed those nits, as well as:

Use PIC for instanceof prototype fetch:
http://hg.mozilla.org/users/danderson_mozilla.com/moo/rev/9f79a1415367

Generate stub for dense array props, then disable PIC:
http://hg.mozilla.org/users/danderson_mozilla.com/moo/rev/6e62086a671e

Use GETPROP for CALLPROP when object is known to be an object:
http://hg.mozilla.org/users/danderson_mozilla.com/moo/rev/f266c97511da
And for strings.

http://hg.mozilla.org/users/danderson_mozilla.com/moo/rev/a2f18befed2e

This is basically done, I've left some FIXMEs but this has been ported and more, in no small part thanks to the simplicity & readability of the old PICStubCompiler. Yay!

Major future work seems to be:
 - Making better use of branding. In fact right now we won't brand unless we take a slow path and use the property cache.
 - Adding more cases to SETPROP, which still shows up in hitcount profiles.
Assignee: cdleary → dvander
Status: NEW → RESOLVED
Closed: 14 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.