Bug 22480 – We should cache get by id chain accesses polymorphically.

• Status: RESOLVED
• Resolution: FIXED
• Component: JavaScriptCore

[Gavin Barraclough]

deltablue spends 7-10% of it's time doing these.

[Gavin Barraclough]

Created attachment 25475 [details]
The patch

        Polymorpic caching for get by id chain.  Similar to the polymorphic caching already implemented
        for self and proto accesses (implemented by allowing multiple trampolines to be JIT genertaed,
        and linked together) - the get by id chain caching is implemented as a genericization of the
        proto list caching, allowing cached access lists to contain a mix of proto and proto chain
        accesses (since in JS style inheritance hierarchies you may commonly see a mix of properties
        being overridden on the direct prototype, or higher up its prototype chain).

        In order to allow this patch to compile there is a fix to appease gcc 4.2 compiler issues
        (removing the jumps between fall-through cases in privateExecute).
        
        This patch also removes redundant immediate checking from the reptach code, and fixes a related
        memory leak (failure to deallocate trampolines). 

        ~2% progression on v8 tests (bulk on the win on deltablue)

[Geoffrey Garen]

Comment on attachment 25475 [details]
The patch

> +    } else if (vPC[0].u.opcode == interpreter->getOpcode(op_get_by_id_chain)) {
> +        prototypeStructureList = new PolymorphicAccessStructureList(vPC[6].u.operand, stubInfo->stubRoutine, vPC[4].u.structure, vPC[5].u.structureChain);
> +        stubInfo->stubRoutine = 0;
> +
> +        vPC[0] = interpreter->getOpcode(op_get_by_id_proto_list);
> +        vPC[4] = prototypeStructureList;
> +        vPC[5] = 2;
> +    } else {

Should ASSERT here which opcode is in vPC[0].

> -            vPC[0] = ARG_globalData->interpreter->getOpcode(op_get_by_id_proto_list);
> -            vPC[4] = prototypeStructureList;
> -            vPC[5] = 2;
> -        } else {
> -            prototypeStructureList = vPC[4].u.polymorphicStructures;
> -            listIndex = vPC[5].u.operand;
> +            StructureChain* chain = structure->cachedPrototypeChain();
> +            if (!chain)
> +                chain = cachePrototypeChain(callFrame, structure);
> +            ASSERT(chain);
>  
> -            vPC[5] = listIndex + 1;
> -        }
> +        StructureStubInfo* stubInfo = &codeBlock->getStubInfo(CTI_RETURN_ADDRESS);
> +        int listIndex;
> +        PolymorphicAccessStructureList* prototypeStructureList = getPolymorphicAccessStructureListSlot(ARG_globalData->interpreter, stubInfo, vPC, listIndex);

Incorrect indentation here? Is "chain" ever used? Why do we compute it?

I hope we can refactor some of this code soon to make the memory management automatic.

It would be nice to eventually add a feature for re-caching by starting over from scratch in the case where the prototype has changed.

r=me

[Gavin Barraclough]

Sending        JavaScriptCore/ChangeLog
Sending        JavaScriptCore/bytecode/Instruction.h
Sending        JavaScriptCore/interpreter/Interpreter.cpp
Sending        JavaScriptCore/interpreter/Interpreter.h
Sending        JavaScriptCore/jit/JIT.cpp
Sending        JavaScriptCore/jit/JIT.h
Transmitting file data ......
Committed revision 38763.