814823 - IonMonkey: Clean-up Ion Cache mechanism.

Status: RESOLVED FIXED

(Core :: JavaScript Engine, defect)

Description

[Nicolas B. Pierron [:nbp]]

Currently, IonCache is a mess it has to contain the all data in an union, which makes it unreadable and makes our cache system monolithic.  The only restriction which currently restrict us from switching to an inheritance system is the IonScript::purgeCaches function.

To do that we need to make the cache list an indirection table and to add a data cache.

Comment 1

[Nicolas B. Pierron [:nbp]]

Attachment: Draft v2

Big changes:
- Use virtual for updateBaseAddress & reset
- Move data to each classes
- Remove the init function.
- Add option to later use a cache of data instead of instructions.
- Factor link & attachStub, attachStub do the increment of the stub count.

Comment 2

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 685052 [details] [diff] [review]
Draft v2

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

::: js/src/ion/IonCaches.cpp
@@ +112,5 @@
> +    if (exitOffset) {
> +        CodeLocationJump exitJump(code, *exitOffset);
> +        PatchJump(exitJump, cacheLabel());
> +        updateLastJump(exitJump);
> +    }

Nice, I really like factoring this and linkCode out.

@@ +803,5 @@
>  }
>  
>  bool
> +IonCacheGetProperty::fallback(JSContext *cx, size_t cacheIndex,
> +                              HandleObject obj, MutableHandleValue vp)

Calling these "fallbacks" feels a little strange to me. It's not a fallback, but the code which performs the caching, and whether it can cache or not it does the same lookup. We don't really have a clear fallback mechanism.

@@ +1650,5 @@
>          return false;
>  
>      rejoinOffset.fixup(&masm);
>      exitOffset.fixup(&masm);
> +    attachStub(code, rejoinOffset, &exitOffset);

Is it not possible to make attachStub perform the fixup?

::: js/src/ion/IonCaches.h
@@ +106,5 @@
> +        return IonCache::Cache_##ickind;                                \
> +    }
> +
> +
> +class IonCodeCache : public IonCache

What's the difference between IonCodeCache and IonCache? It seems like nothing inherits from IonCache but not IonCodeCache, so can we just fold IonCodeCache into IonCache?

::: js/src/ion/shared/CodeGenerator-shared.h
@@ +172,5 @@
>  
> +    size_t allocateData(size_t size) {
> +        JS_ASSERT(size % sizeof(void *) == 0);
> +        size_t dataOffset = runtimeData_.length();
> +        masm.reportMemory(runtimeData_.appendN(0, size));

Oh dear, "reportMemory" is a confusing name. I realize it's not introduced in this patch, but could we change it to "propagateOOMCheck" or something?

Comment 3

[Nicolas B. Pierron [:nbp]]

(In reply to David Anderson [:dvander] from comment #2)
> Comment on attachment 685052 [details] [diff] [review]
> Draft v2
> 
> Review of attachment 685052 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> @@ +803,5 @@
> >  }
> >  
> >  bool
> > +IonCacheGetProperty::fallback(JSContext *cx, size_t cacheIndex,
> > +                              HandleObject obj, MutableHandleValue vp)
> 
> Calling these "fallbacks" feels a little strange to me. It's not a fallback,
> but the code which performs the caching, and whether it can cache or not it
> does the same lookup. We don't really have a clear fallback mechanism.

Otherwise I was thinking of naming them “genCache”, but I am sure we can find a better name for them.

> @@ +1650,5 @@
> >          return false;
> >  
> >      rejoinOffset.fixup(&masm);
> >      exitOffset.fixup(&masm);
> > +    attachStub(code, rejoinOffset, &exitOffset);
> 
> Is it not possible to make attachStub perform the fixup?

I think so, I miss it because I started making a big function doing what linkCode and attachStub are doing, but many cases add some extra fixup.  So I gave up on this way, but I can surely lift rejoinOffset and exitOffset fixup in attachStub.

> ::: js/src/ion/IonCaches.h
> @@ +106,5 @@
> > +        return IonCache::Cache_##ickind;                                \
> > +    }
> > +
> > +
> > +class IonCodeCache : public IonCache
> 
> What's the difference between IonCodeCache and IonCache? It seems like
> nothing inherits from IonCache but not IonCodeCache, so can we just fold
> IonCodeCache into IonCache?

The idea was to provide a cache which does not necessarily provide code, but just data.  So this are the premise of such a system.  One example might be a “.prototype” cache for functions, as this property is owned by the function object, we only need to check the shape of the function, and we can iterate faster on the data of the cache without any out-of-line cache.

At this stage this looks more like a proof of concept.

> ::: js/src/ion/shared/CodeGenerator-shared.h
> @@ +172,5 @@
> >  
> > +    size_t allocateData(size_t size) {
> > +        JS_ASSERT(size % sizeof(void *) == 0);
> > +        size_t dataOffset = runtimeData_.length();
> > +        masm.reportMemory(runtimeData_.appendN(0, size));
> 
> Oh dear, "reportMemory" is a confusing name. I realize it's not introduced
> in this patch, but could we change it to "propagateOOMCheck" or something?

Indeed.

What would you think about replacing the IonCache prefix by IC, for IonCacheGetProperty and others, to look more like the MIR/LIR naming ?

Comment 4

[Nicolas B. Pierron [:nbp]]

I went on the CodeGen clean-up of IonCache, here is the latest result of an inlineCache in the CodeGen:

struct GetPropertyIC::UpdateData
{
    typedef ICArgSeq<ICField<CACHE_FIELD(GetPropertyIC, Register, object_)>
                     > Args;
    typedef ICStoreValueTo<CACHE_FIELD(GetPropertyIC, TypedOrValueRegister, output_)> Output;
    typedef bool (*Fn)(JSContext *, size_t, HandleObject, MutableHandleValue);
};
const VMFunction GetPropertyIC::UpdateInfo =
    FunctionInfo<GetPropertyIC::UpdateData::Fn>(GetPropertyIC::update);

bool CodeGenerator::visitGetPropertyCacheV(LGetPropertyCacheV *ins) {
    RegisterSet liveRegs = ins->safepoint()->liveRegs();
    Register objReg = ToRegister(ins->getOperand(0));
    PropertyName *name = ins->mir()->name();
    bool allowGetters = ins->mir()->allowGetters();
    TypedOrValueRegister output = TypedOrValueRegister(GetValueOutput(ins));

    GetPropertyIC cache(liveRegs, objReg, name, output, allowGetters);

    return inlineCache(ins, cache);
}

The structure is here to instrument the out-of-line path generated by the inlineCache function.
The patch should be usable in a few hours.

Comment 5

[Nicolas B. Pierron [:nbp]]

Attachment: Draft v3

In addition to the last modification:
 - Factor again the backend to have one function named linkAndAttachStub(…)
 - Replace the Yet-Another-Dispatch mess which had to be manually patched for adding a new IonCache call.  It is replaced by a oolCallVM like, except that data are extracted from the cache instead of creating a new data structure on the fly to account for each set of arguments.
 - Rename fallback -> update.  Attach the UpdateInfo to the class, as well as the UpdateData use to generate the out-of-line path out of the cache.

Comment 6

[Nicolas B. Pierron [:nbp]]

Attachment: Rewrite IonCache interface.

This patch rewrite the API of IonCache such as it is make a proper usage of the inheritance and avoid some weird junction point in visitCache which makes them difficult to understand.

This patch add another storage location and an indirection to refer to IonCache functions, thus caches can have different size and can still be fixup/reset.

Also, it moves the update function of each cache to a static function on the cache owner, such as the name of the cache naturally appear in the VM function which is using it.

The call mechanism for "inlineCache" is now doing some template-magic where the MyIC::UpdateData is used as a traits for describing how the inputs of the cache stubs (register allocations) are mapped to the arguments of the update function.  This is used to improve code locality and reduce the burden related to the current cache mechanism.

In addition, the OutOfLineCache structure (renamed to OutOfLineUpdateCache<Cache> due to the migration) is now allocated after the cache it-self, which means that we can avoid storing temporary inlineJump & inlineLabel and store them directly to the cache.  Still, it has to hold the cacheIndex such as the cache can be updated during the out-of-line path.

Concerning the inline cache writing, not much has been done except factoring the bottom part of attach functions into linkCode, attachStub and linkAndAttachStub functions. The attachStub function is now responsible for incrementing the number of stubs when it updates the lastJump, such as we cannot accidentally increase the number of stub without adding any.

I think there is still motivations to clean-up the content of the IC, by adding an ICCodeGenerator to handle what is done by the GetNativePropertyStub structure … but we should better wait for more use case and more insane things to show up as understandable.

Comment 7

[Shu-yu Guo [:shu]]

Comment on attachment 685933 [details] [diff] [review]
Rewrite IonCache interface.

Didn't do too-deep a reading of the patch, but I like the new abstractions so far. The less boilerplate and manual dispatch the better.

Drive-by nitpick:

>      if (!cache.idempotent()) {
> +        JS_ASSERT(!cache.idempotent());

Do we have so little faith in our C++ compiler?

Comment 8

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 685933 [details] [diff] [review]
Rewrite IonCache interface.

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

This patch introduces a lot of C++ complexity that wasn't needed before. The original patch, which separated out the IonCache structure to not have unions, and factored out the gritty boilerplate details of linking, was definitely highly desirable. That alone would go a long way in making the ICs easier to work with.

Maybe the thing to do is to get that polished and landed first, and then look at what can be done next to make the design better.

Comment 9

[Nicolas B. Pierron [:nbp]]

Attachment: Rewrite IonCache interface & dispatch.

I ask for feedback to know if this orientation of the clean-up patch which does a double visitor dispatch instead of relying on the LIR opcodes, and use well named function to produce the out-of-line path making a call to the update function ICs.

This patch is not rebased and contains an outdated comment describing how to encode an IC in the codegen.  If the design is good enough, I will submit it for a review later.

Comment 10

[Nicolas B. Pierron [:nbp]]

Attachment: Rewrite IonCache interface & dispatch.

CodeGen changes:
- Add a runtimeData entry to the IonCode, to store IonCaches of various size.
- Add a vector corresponding to the runtime data, with an allocateCache function to register IonCache.
- Re-order IonCode fields in the order of usage frequency (for sanity).
- Add visit function for the out-of-line path of each IC based on a dual dispatch instead of a switch case on the LOpcode.
- Move register definitions of inline caches input/output registers to the visit function of the LIR instruction instead of a switch case on the LOpcode.
- Add an inlinceCache function to factor the declaration of IC in the visit functions of LIR instructions.

IonCache changes:
- Use inheritance instead of a catch-all union.
- Remove place-holders for TypedOrValueRegister and ConstantOrRegister containers.
- Hide stub counter under accessors.
- Factor Link & Attach phase of stub generation.
- Move stub counter increments to the link phase.
- Use macros similar to MIR & LIR declarations, instead of hand-written boilerplate.
- Use static MyCodeIC::update function name.

Sorry for the size of this patch.
https://tbpl.mozilla.org/?tree=Try&rev=b11c663e0a50

Comment 11

[Nicolas B. Pierron [:nbp]]

Comment on attachment 702698 [details] [diff] [review]
Rewrite IonCache interface & dispatch.

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

::: js/src/ion/IonCaches.cpp
@@ +930,5 @@
>  }
>  
>  bool
> +GetPropertyIC::update(JSContext *cx, size_t cacheIndex,
> +                      HandleObject obj, MutableHandleValue vp)

This comment is removed, because it is incorrect.
I replaced the following line in GetPropertyIC::update, to make sure that we don't produce any stub when the cache is full.

-    if (!isCacheable && !cache.idempotent() && cx->names().length == name) {
+    if (!isCacheable && canAttachStub() && !cache.idempotent() && cx->names().length == name) {

This should fix the problem seen with JQuery (mochitest-3 on tbpl) which attempt to attach a DenseArrayLength stub after overflowing the IC.

@@ +1581,5 @@
>      RepatchLabel exit_;
>      CodeOffsetJump exitOffset = masm.jumpWithPatch(&exit_);
>      masm.bind(&exit_);
>  
> +    return linkAndAttachStub(cx, masm, ion, "dense array", rejoinOffset, &exitOffset);

I added "setHasDenseArrayStub();" back above this line.

Comment 12

[Nicolas B. Pierron [:nbp]]

Attachment: Rewrite IonCache interface & dispatch.

Rebased version.
Same delta as the previous patch, Plus:
 - Convert CallsiteClone IC.
 - Follow removal of prebarrier location.
 - Add test case exercising the max limit of get property for array.length.

Comment 13

[David Anderson [:dvander] - inactive, e-mail if emergency]

Comment on attachment 703708 [details] [diff] [review]
Rewrite IonCache interface & dispatch.

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

Thanks for doing this, it definitely will help the insane amount of boilerplate associated with ICs, as well as improve IC clarity. Patch looks good overall, I have some specific comments around stuff I found confusing.

::: js/src/ion/CodeGenerator.cpp
@@ +80,5 @@
> +
> +// This function is declared here because it needs to instantiate an
> +// OutOfLineUpdateCache, but we want to keep it visible inside the
> +// CodeGeneratorShared such as we can specialize inline caches in function of
> +// the architecture.

What does "in function of the architecture" mean here?

@@ +82,5 @@
> +// OutOfLineUpdateCache, but we want to keep it visible inside the
> +// CodeGeneratorShared such as we can specialize inline caches in function of
> +// the architecture.
> +bool
> +CodeGeneratorShared::inlineCache(LInstruction *lir, size_t cacheIndex)

Prefer renaming this to "defineCache" or "addCache" or something. (would also fit better with allocateCache).

@@ +105,5 @@
> +    return true;
> +}
> +
> +IonCache *
> +CodeGeneratorShared::updateCachePrefix(size_t cacheIndex)

This function seems out of place. I'd prefer to rename it to "visitOutOfLineCache" (or something), and then have it do cache->accept(this), which will make OutOfLineUpdateCache::accept cleaner.

@@ +3702,5 @@
>      ionScript->setDeoptTable(deoptTable_);
> +
> +    // Ordered by expected frequency of usage and by sequences of reads.
> +
> +    // for generating caches during the execution.

This order will be indistinguishable from just alphabetizing, so I'd delete the comment.

@@ +3715,4 @@
>      if (safepoints_.size())
>          ionScript->copySafepoints(&safepoints_);
>  
> +    // for rare cases, such as f.arguments or bailouts.

I don't understand this comment, I'd just remove it.

@@ +4128,3 @@
>  
> +bool
> +CodeGenerator::visitSetPropertyCacheT(LSetPropertyCacheT *ins) {

nit: brace on newline

::: js/src/ion/Ion.cpp
@@ +522,2 @@
>  
> +    // for generating caches during the execution.

Drop this comment

@@ +537,5 @@
> +    script->safepointsStart_ = offsetCursor;
> +    script->safepointsSize_ = safepointsSize;
> +    offsetCursor += paddedSafepointSize;
> +
> +    // for rare cases, such as f.arguments or bailouts.

same

::: js/src/ion/IonCaches.cpp
@@ +155,5 @@
> +    // MarkIonExitFrame).
> +    if (stubLabel) {
> +        stubLabel->fixup(&masm);
> +        Assembler::patchDataWithValueCheck(CodeLocationLabel(code, *stubLabel),
> +                                           ImmWord(uintptr_t(code)), CODE_MARK);

I don't understand what CODE_MARK is, or why it needs to be a member of IonCache. If it's just a poison value, it can be static local to this file, and named something like INVALID_DEBUG_WORD.

::: js/src/ion/IonCaches.h
@@ +81,5 @@
> +// with its update function. A cache derive from the IonCache class and use
> +// CACHE_HEADER to pre-declare a few members such as UpdateInfo (VMFunction) and
> +// the accept function. The name of the derived cache must be registered in the
> +// IONCACHE_KIND_LIST used to provide convertion operations and to provide
> +// default accessors.

"An IonCache is the base structure of an inline cache, which generates code stubs dynamically and attaches them to an IonScript."

@@ +173,2 @@
>  
> +    CodeLocationLabel cacheLabel() const { return cacheLabel_; }

nit: {
    return cacheLabel_;
}

@@ +185,5 @@
>    public:
>  
> +    IonCache()
> +      : pure_(false),
> +        idempotent_(false),

Is idempotent only a property of GetProperty?

@@ +203,5 @@
> +
> +    // Bind inline boundaries of the cache. This include the patchable jump
> +    // location and the location where the successful exit rejoin the inline
> +    // path.
> +    void bindInline(CodeOffsetJump initialJump, CodeOffsetLabel rejoinLabel) {

"Bind" implies that something is or will be patched as a result of this call. It looks like this is the initial state for the cache, so maybe:

// Set the initial jump state of the cache. The initialJump is the inline
// jump that will point to out-of-line code (such as the slow path, or stubs),
// and the rejoinLabel is the position that all out-of-line paths will rejoin
// to.
void setInlineJump(...

@@ +213,5 @@
> +
> +    // Bind out-of-line boundaries of the cache. This include the location of
> +    // the update function call.  This location will be set to the exitJump of
> +    // the last generated stub.
> +    void bindOutOfLine(CodeOffsetLabel cacheLabel) {

Same here (cacheLabel_ strikes me as a bad name, but that's pre-existing):

// Set the position of the default, fallback code for the inline cache.
void setFallbackLabel(...

@@ +246,5 @@
> +
> +    // Return value of linkCode (see linkCode). This special value is used to
> +    // identify the fact that the cache might have been flushed or that the
> +    // IonScript might be invalidated since we entered the update function.
> +    static IonCode * const CACHE_FLUSHED;

This mechanism is kind of gross - if we need to distinguish between different failure modes, we should use an outparam:

bool link(JSContext *cx, IonScript *ion, MacroAssembler &masm, IonCode **codep);

(Or return an enum if more failure modes are needed)

@@ +343,5 @@
> +    PropertyName *name() const { return name_; }
> +    TypedOrValueRegister output() const { return output_; }
> +    bool allowGetters() const { return allowGetters_; }
> +    bool hasArrayLengthStub() const { return hasArrayLengthStub_; }
> +    bool hasTypedArrayLengthStub() const { return hasTypedArrayLengthStub_; }

These (and elsewhere) should put the "return" and } on separate lines.

@@ +364,5 @@
> +    // value of these registers must be preserved by the cache.
> +    RegisterSet liveRegs_;
> +
> +    Register object_;
> +    PropertyName *name_; // rooting issues ?!

No rooting issue since we purge ICs on GC.

::: js/src/ion/IonMacroAssembler.h
@@ +108,5 @@
>      size_t instructionsSize() const {
>          return size();
>      }
>  
> +    void collectOOM(bool success) {

s/collectOOM/propagateOOM/

::: js/src/ion/shared/CodeGenerator-shared.h
@@ +158,5 @@
>  
> +  private:
> +    // Ensure the cache is an IonCache while expecting the size of the derived
> +    // class.
> +    size_t allocateCacheSize(const IonCache &, size_t size) {

s/allocateCacheSize/allocateCache/

@@ +159,5 @@
> +  private:
> +    // Ensure the cache is an IonCache while expecting the size of the derived
> +    // class.
> +    size_t allocateCacheSize(const IonCache &, size_t size) {
> +        using namespace mozilla;

Why is that |using namespace| needed?

Comment 14

[Nicolas B. Pierron [:nbp]]

(In reply to David Anderson [:dvander] from comment #13)
> Comment on attachment 703708 [details] [diff] [review]
> Rewrite IonCache interface & dispatch.
> 
> Review of attachment 703708 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Thanks for doing this, it definitely will help the insane amount of
> boilerplate associated with ICs, as well as improve IC clarity. Patch looks
> good overall, I have some specific comments around stuff I found confusing.
> 
> ::: js/src/ion/CodeGenerator.cpp
> @@ +80,5 @@
> > +
> > +// This function is declared here because it needs to instantiate an
> > +// OutOfLineUpdateCache, but we want to keep it visible inside the
> > +// CodeGeneratorShared such as we can specialize inline caches in function of
> > +// the architecture.
> 
> What does "in function of the architecture" mean here?

I mean that we might want to be able to add cache which are registered in CodeGenerator-x86-shared.cpp, so we need addCache declaration to be in the CodeGeneratorShared::

> @@ +105,5 @@
> > +    return true;
> > +}
> > +
> > +IonCache *
> > +CodeGeneratorShared::updateCachePrefix(size_t cacheIndex)
> 
> This function seems out of place. I'd prefer to rename it to
> "visitOutOfLineCache" (or something), and then have it do
> cache->accept(this), which will make OutOfLineUpdateCache::accept cleaner.

Indeed, this is cleaner, also to keep the "bool visitType(Type *ins)" scheme I moved the cache->accept into the the visitOutOfLineCache function.

> ::: js/src/ion/IonCaches.cpp
> @@ +155,5 @@
> > +    // MarkIonExitFrame).
> > +    if (stubLabel) {
> > +        stubLabel->fixup(&masm);
> > +        Assembler::patchDataWithValueCheck(CodeLocationLabel(code, *stubLabel),
> > +                                           ImmWord(uintptr_t(code)), CODE_MARK);
> 
> I don't understand what CODE_MARK is, or why it needs to be a member of
> IonCache. If it's just a poison value, it can be static local to this file,
> and named something like INVALID_DEBUG_WORD.

I replaced it by STUB_ADDR, as this is supposed to be the place in which the STUB_ADDR will be put in when linking the stub.

> @@ +185,5 @@
> >    public:
> >  
> > +    IonCache()
> > +      : pure_(false),
> > +        idempotent_(false),
> 
> Is idempotent only a property of GetProperty?

Indeed, I don't see any other use cases.
I think you would agree to do that as part of another clean-up patch ;)

> @@ +246,5 @@
> > +
> > +    // Return value of linkCode (see linkCode). This special value is used to
> > +    // identify the fact that the cache might have been flushed or that the
> > +    // IonScript might be invalidated since we entered the update function.
> > +    static IonCode * const CACHE_FLUSHED;
> 
> This mechanism is kind of gross - if we need to distinguish between
> different failure modes, we should use an outparam:
> 
> bool link(JSContext *cx, IonScript *ion, MacroAssembler &masm, IonCode
> **codep);
> 
> (Or return an enum if more failure modes are needed)

Removed the special CACHE_FLUSHED variable, and added an enum with the 3 states such as we understand clearly what is going on.

> @@ +343,5 @@
> > +    PropertyName *name() const { return name_; }
> > +    TypedOrValueRegister output() const { return output_; }
> > +    bool allowGetters() const { return allowGetters_; }
> > +    bool hasArrayLengthStub() const { return hasArrayLengthStub_; }
> > +    bool hasTypedArrayLengthStub() const { return hasTypedArrayLengthStub_; }
> 
> These (and elsewhere) should put the "return" and } on separate lines.

To be more precise in case somebody else wants to do the same thing on other files:

    sed -i 's/^\([ ]*\)\(.* {\) \(return [^;]*;\) }$/\1\2\n\1    \3\n\1}/'

> @@ +159,5 @@
> > +  private:
> > +    // Ensure the cache is an IonCache while expecting the size of the derived
> > +    // class.
> > +    size_t allocateCacheSize(const IonCache &, size_t size) {
> > +        using namespace mozilla;
> 
> Why is that |using namespace| needed?

It is not, I got rid of it.

Comment 15

[Nicolas B. Pierron [:nbp]]

Try: https://tbpl.mozilla.org/?tree=Try&rev=82bf14da97da

https://hg.mozilla.org/integration/mozilla-inbound/rev/62fa6ee0a279

Comment 16

[Phil Ringnalda (:philor)]

Backed out in https://hg.mozilla.org/integration/mozilla-inbound/rev/9664c3c609a1 because Android mochitests didn't care much for it, feeling the need to crash.

Comment 17

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/62fa6ee0a279

Comment 18

[Jim Jeffery not reading bug-mail 1/2/11]

(In reply to Ryan VanderMeulen [:RyanVM] from comment #17)
> https://hg.mozilla.org/mozilla-central/rev/62fa6ee0a279

Am I missing something here?  comment #16 seems to indicate it was backed out..yet marked Fixed after the merge ?  I fail to see the re-landing...

Comment 19

[Ryan VanderMeulen [:RyanVM]]

You're right. Our merge tool didn't properly pick up the backout.

Comment 20

[Nicolas B. Pierron [:nbp]]

Apparently the try server does not want to check any Android mochitest … so let's hope it will work after adding back the AutoFlushCache to GetElementIC.

https://tbpl.mozilla.org/?tree=Try&rev=d200301c3087
https://tbpl.mozilla.org/?tree=Try&rev=a331e0e1413e

https://hg.mozilla.org/integration/mozilla-inbound/rev/f84b3c01089e

Comment 21

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f84b3c01089e