If a property lookup misses (producing undefined), we disable the IC and stubcall every time. I'm looking at a JS physics engine (closed atm, unfortunately) that is doing a ton of these, taking up to 20% of our time. v8 seems to have an IC for this which makes them about 16x faster on a simple testcase. Any chance we could get one too? It seems like it would be pretty simple.

Yeah, this should be very easy in either JIT. Can you attach the/a testcase you used to benchmark?

I was using: var o = {x:1, y:2}; var s = 0; for (var i = 0; i < 1000000; ++i) { if (o.z) s++; } for the simple test. This test doesn't show it, but the IC would need to be polymorphic to be effective for the purposes of the physics engine.

Digging in a little more, it seems that the property sometimes exists (a small but non-trivial percent of the time). Thus, to really help, it seems like this would need to be integrated into the general property IC.

Here's a basic WIP that works for my little test.

This was eerily easy, so I'm worried I'm missing something. I didn't touch TI; it seems like the type barriers would cover it; is that right?

Comment on attachment 646653 [details] [diff] [review] patch Review of attachment 646653 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/methodjit/PolyIC.cpp @@ +599,5 @@ > + GetPropLookup_Error = Lookup_Error, > + GetPropLookup_Uncacheable = Lookup_Uncacheable, > + GetPropLookup_Cacheable = Lookup_Cacheable, > + GetPropLookup_NoProperty > +}; This enum seems to make things overly complicated. Can you add Lookup_NoProperty to LookupStatus instead? That requires going over the other places where the statuses are tested, but it looks like they are mostly correct already, testing for != Lookup_Cacheable rather than == Lookup_Uncacheable. @@ +1278,5 @@ > > + /* > + * A non-null 'shape' tells us where to find the property value in the > + * holder object. A null shape means that the above checks guard the > + * absence of the property, so the get-prop returns 'undefined'. A comment on why it's ok to produce an undefined value without checking the type information would be good. Since the stub call generating the cache will produce undefined, and the results of all getprop stub calls are monitored regardless, this will work.

(In reply to Brian Hackett (:bhackett) from comment #6) > This enum seems to make things overly complicated. Can you add > Lookup_NoProperty to LookupStatus instead? Adding a new case to an enum used in a wider context seems scary, but 'sure'.

Tests found two important cases where we need to disallow the missing-prop ic: - __noSuchMethod__ - obj->getClass()->getProperty != JS_PropertyStub non-standard SM-isms ftl