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

RESOLVED FIXED

Status

()

Core
JavaScript Engine
RESOLVED FIXED
5 years ago
5 years ago

People

(Reporter: nbp, Assigned: nbp)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

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.
Created attachment 640282 [details] [diff] [review]
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.
Attachment #640282 - Flags: review?(jdemooij)
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());
Attachment #640282 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/projects/ionmonkey/rev/24c81eaaf528
Status: ASSIGNED → RESOLVED
Last Resolved: 5 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.