Last Comment Bug 772087 - IonMonkey: Immediate Invalidation from idempotent cache v8-raytrace.js:710
: IonMonkey: Immediate Invalidation from idempotent cache v8-raytrace.js:710
Status: RESOLVED FIXED
:
Product: Core
Classification: Components
Component: JavaScript Engine (show other bugs)
: unspecified
: x86_64 Linux
: -- normal (vote)
: ---
Assigned To: Nicolas B. Pierron [:nbp]
:
Mentors:
Depends on:
Blocks: 768745
  Show dependency treegraph
 
Reported: 2012-07-09 08:51 PDT by Nicolas B. Pierron [:nbp]
Modified: 2012-07-10 02:37 PDT (History)
2 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---


Attachments
Avoid IonMonkey recompilation caused by wrong idempotent flags. (5.73 KB, patch)
2012-07-09 10:53 PDT, Nicolas B. Pierron [:nbp]
jdemooij: review+
Details | Diff | Review

Description Nicolas B. Pierron [:nbp] 2012-07-09 08:51:21 PDT
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] 2012-07-09 10:53:57 PDT
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.
Comment 2 Jan de Mooij [:jandem] 2012-07-09 11:34:52 PDT
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] 2012-07-10 02:37:03 PDT
https://hg.mozilla.org/projects/ionmonkey/rev/24c81eaaf528

Note You need to log in before you can comment on or make changes to this bug.