772087 - IonMonkey: Immediate Invalidation from idempotent cache v8-raytrace.js:710

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicolas B. Pierron [:nbp]]

Investigate what is the missing condition which is supposed to prevent this invalidation.  JM is supposed to have run enough time to prevent any immediate invalidation in Ion.

The type inference does not report any new type.  The same kind of invalidation happens also for  raytrace.js:427 except that there is an invalidation caused by Bug 766805 between the compilation of the Ion cache invalidation.

This bug can be detected with IONFLAGS=caches,logs,mir,aborts,bailouts,osi and TI spew can be enabled to ensure that no Type Inference constraint caused anything else in the mean time.

Comment 1

[Nicolas B. Pierron [:nbp]]

Attachment: Avoid IonMonkey recompilation caused by wrong idempotent flags.

- Factor the the idempotent proto chain predicate to JSObject such as it
  can be shared between JM and IM.
    
- Filter the resolve function of JSFunction as idempotent.

Comment 2

[Jan de Mooij [:jandem]]

Comment on attachment 640282 [details] [diff] [review]
Avoid IonMonkey recompilation caused by wrong idempotent flags.

Review of attachment 640282 [details] [diff] [review]:
-----------------------------------------------------------------

Looks good, r=me with comments addressed.

::: js/src/jsobj.h
@@ +380,4 @@
>                                      size_t *slotsSize, size_t *elementsSize,
>                                      size_t *miscSize) const;
>  
> +    inline bool isIdempotentProtoChain() const;

Nit: maybe hasIdempotentProtoChain? I added the original name but "has" seems better now...

::: js/src/methodjit/PolyIC.cpp
@@ +698,5 @@
>          if (!aobj->isNative())
>              return ic.disable(f, "non-native");
>  
> +        if (!aobj->isIdempotentProtoChain())
> +            return Lookup_Uncacheable;

This condition is too strict for JM ICs (which are never idempotent). Can you instead add the following to GetPropCompiler::update near the hadGC() check:

if (..not idempotent..)
    MarkNotIdempotent(f.script(), f.pc());

Comment 3

[Nicolas B. Pierron [:nbp]]

https://hg.mozilla.org/projects/ionmonkey/rev/24c81eaaf528