⚠ Archived content — this site is no longer maintained.   Current WebKit documentation is at docs.webkit.org.

Changeset 201456 in webkit


Ignore:
Timestamp:
May 27, 2016, 11:36:30 AM (10 years ago)
Author:
keith_miller@apple.com
Message:

get_by_id should support caching unset properties in the LLInt
​https://bugs.webkit.org/show_bug.cgi?id=158136

Reviewed by Benjamin Poulain.

Recently, we started supporting prototype load caching for get_by_id
in the LLInt. This patch extends that to caching unset properties.
While it is uncommon in general for a program to see a single structure
without a given property, the Array.prototype.concat function needs to
lookup the Symbol.isConcatSpreadable property. For any existing code
That property will never be set as it did not exist prior to ES6.

Similarly to the get_by_id_proto_load bytecode, this patch adds a new
bytecode, get_by_id_unset that checks the structureID of the base and
assigns undefined to the result.

There are no new tests here since we already have many tests that
incidentally cover this change.

  • bytecode/BytecodeList.json:
  • bytecode/BytecodeUseDef.h:

(JSC::computeUsesForBytecodeOffset):
(JSC::computeDefsForBytecodeOffset):

  • bytecode/CodeBlock.cpp:

(JSC::CodeBlock::printGetByIdOp):
(JSC::CodeBlock::dumpBytecode):
(JSC::CodeBlock::finalizeLLIntInlineCaches):

  • bytecode/GetByIdStatus.cpp:

(JSC::GetByIdStatus::computeFromLLInt):

  • dfg/DFGByteCodeParser.cpp:

(JSC::DFG::ByteCodeParser::parseBlock):

  • dfg/DFGCapabilities.cpp:

(JSC::DFG::capabilityLevel):

  • jit/JIT.cpp:

(JSC::JIT::privateCompileMainPass):
(JSC::JIT::privateCompileSlowCases):

  • llint/LLIntSlowPaths.cpp:

(JSC::LLInt::setupGetByIdPrototypeCache):
(JSC::LLInt::LLINT_SLOW_PATH_DECL):

  • llint/LLIntSlowPaths.h:
  • llint/LowLevelInterpreter32_64.asm:
  • llint/LowLevelInterpreter64.asm:
Location:
trunk/Source/JavaScriptCore
Files:
12 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/ChangeLog

    r201451 r201456  
     12016-05-27  Keith Miller  <keith_miller@apple.com>
     2
     3        get_by_id should support caching unset properties in the LLInt
     4        https://bugs.webkit.org/show_bug.cgi?id=158136
     5
     6        Reviewed by Benjamin Poulain.
     7
     8        Recently, we started supporting prototype load caching for get_by_id
     9        in the LLInt. This patch extends that to caching unset properties.
     10        While it is uncommon in general for a program to see a single structure
     11        without a given property, the Array.prototype.concat function needs to
     12        lookup the Symbol.isConcatSpreadable property. For any existing code
     13        That property will never be set as it did not exist prior to ES6.
     14
     15        Similarly to the get_by_id_proto_load bytecode, this patch adds a new
     16        bytecode, get_by_id_unset that checks the structureID of the base and
     17        assigns undefined to the result.
     18
     19        There are no new tests here since we already have many tests that
     20        incidentally cover this change.
     21
     22        * bytecode/BytecodeList.json:
     23        * bytecode/BytecodeUseDef.h:
     24        (JSC::computeUsesForBytecodeOffset):
     25        (JSC::computeDefsForBytecodeOffset):
     26        * bytecode/CodeBlock.cpp:
     27        (JSC::CodeBlock::printGetByIdOp):
     28        (JSC::CodeBlock::dumpBytecode):
     29        (JSC::CodeBlock::finalizeLLIntInlineCaches):
     30        * bytecode/GetByIdStatus.cpp:
     31        (JSC::GetByIdStatus::computeFromLLInt):
     32        * dfg/DFGByteCodeParser.cpp:
     33        (JSC::DFG::ByteCodeParser::parseBlock):
     34        * dfg/DFGCapabilities.cpp:
     35        (JSC::DFG::capabilityLevel):
     36        * jit/JIT.cpp:
     37        (JSC::JIT::privateCompileMainPass):
     38        (JSC::JIT::privateCompileSlowCases):
     39        * llint/LLIntSlowPaths.cpp:
     40        (JSC::LLInt::setupGetByIdPrototypeCache):
     41        (JSC::LLInt::LLINT_SLOW_PATH_DECL):
     42        * llint/LLIntSlowPaths.h:
     43        * llint/LowLevelInterpreter32_64.asm:
     44        * llint/LowLevelInterpreter64.asm:
     45
    1462016-05-26  Filip Pizlo  <fpizlo@apple.com>
    247

  • trunk/Source/JavaScriptCore/bytecode/BytecodeList.json

    r201363 r201456  
    6262            { "name" : "op_get_by_id", "length" : 9  },
    6363            { "name" : "op_get_by_id_proto_load", "length" : 9 },
     64            { "name" : "op_get_by_id_unset", "length" : 9 },
    6465            { "name" : "op_get_by_id_with_this", "length" : 5 },
    6566            { "name" : "op_get_by_val_with_this", "length" : 5 },

  • trunk/Source/JavaScriptCore/bytecode/BytecodeUseDef.h

    r201363 r201456  
    160160    case op_get_by_id:
    161161    case op_get_by_id_proto_load:
     162    case op_get_by_id_unset:
    162163    case op_get_array_length:
    163164    case op_typeof:
    … …  
    395396    case op_get_by_id:
    396397    case op_get_by_id_proto_load:
     398    case op_get_by_id_unset:
    397399    case op_get_by_id_with_this:
    398400    case op_get_by_val_with_this:

  • trunk/Source/JavaScriptCore/bytecode/CodeBlock.cpp

    r201363 r201456  
    350350        op = "get_by_id_proto_load";
    351351        break;
     352    case op_get_by_id_unset:
     353        op = "get_by_id_unset";
     354        break;
    352355    case op_get_array_length:
    353356        op = "array_length";
    … …  
    11201123        case op_get_by_id:
    11211124        case op_get_by_id_proto_load:
     1125        case op_get_by_id_unset:
    11221126        case op_get_array_length: {
    11231127            printGetByIdOp(out, exec, location, it);
    … …  
    28072811        switch (interpreter->getOpcodeID(curInstruction[0].u.opcode)) {
    28082812        case op_get_by_id:
    2809         case op_get_by_id_proto_load: {
     2813        case op_get_by_id_proto_load:
     2814        case op_get_by_id_unset: {
    28102815            StructureID oldStructureID = curInstruction[4].u.structureID;
    28112816            if (!oldStructureID || Heap::isMarked(m_vm->heap.structureIDTable().get(oldStructureID)))

  • trunk/Source/JavaScriptCore/bytecode/GetByIdStatus.cpp

    r201363 r201456  
    7979    Opcode opcode = instruction[0].u.opcode;
    8080
     81    ASSERT(opcode == LLInt::getOpcode(op_get_array_length) || opcode == LLInt::getOpcode(op_try_get_by_id) || opcode == LLInt::getOpcode(op_get_by_id_proto_load) || opcode == LLInt::getOpcode(op_get_by_id) || opcode == LLInt::getOpcode(op_get_by_id_unset));
     82
    8183    // FIXME: We should not just bail if we see a try_get_by_id or a get_by_id_proto_load.
    8284    // https://bugs.webkit.org/show_bug.cgi?id=158039
    83     if (opcode == LLInt::getOpcode(op_get_array_length) || opcode == LLInt::getOpcode(op_try_get_by_id) || opcode == LLInt::getOpcode(op_get_by_id_proto_load))
     85    if (opcode != LLInt::getOpcode(op_get_by_id))
    8486        return GetByIdStatus(NoInformation, false);
    8587

  • trunk/Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp

    r201363 r201456  
    40834083        case op_get_by_id:
    40844084        case op_get_by_id_proto_load:
     4085        case op_get_by_id_unset:
    40854086        case op_get_array_length: {
    40864087            SpeculatedType prediction = getPrediction();

  • trunk/Source/JavaScriptCore/dfg/DFGCapabilities.cpp

    r201363 r201456  
    156156    case op_get_by_id:
    157157    case op_get_by_id_proto_load:
     158    case op_get_by_id_unset:
    158159    case op_get_by_id_with_this:
    159160    case op_get_by_val_with_this:

  • trunk/Source/JavaScriptCore/jit/JIT.cpp

    r201363 r201456  
    242242        case op_get_array_length:
    243243        case op_get_by_id_proto_load:
     244        case op_get_by_id_unset:
    244245        DEFINE_OP(op_get_by_id)
    245246        DEFINE_OP(op_get_by_id_with_this)
    … …  
    425426        case op_get_array_length:
    426427        case op_get_by_id_proto_load:
     428        case op_get_by_id_unset:
    427429        DEFINE_SLOWCASE_OP(op_get_by_id)
    428430        DEFINE_SLOWCASE_OP(op_get_by_val)

  • trunk/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp

    r201363 r201456  
    590590        return;
    591591
    592     ObjectPropertyConditionSet conditions = generateConditionsForPrototypePropertyHit(vm, codeBlock, exec, structure, slot.slotBase(), ident.impl());
     592    ObjectPropertyConditionSet conditions;
     593    if (slot.isUnset())
     594        conditions = generateConditionsForPropertyMiss(vm, codeBlock, exec, structure, ident.impl());
     595    else
     596        conditions = generateConditionsForPrototypePropertyHit(vm, codeBlock, exec, structure, slot.slotBase(), ident.impl());
    593597
    594598    if (!conditions.isValid())
    … …  
    605609        result.iterator->value.add(condition, pc)->install();
    606610    }
    607     ASSERT(offset != invalidOffset);
     611    ASSERT((offset == invalidOffset) == slot.isUnset());
    608612
    609613    ConcurrentJITLocker locker(codeBlock->m_lock);
     614
     615    if (slot.isUnset()) {
     616        pc[0].u.opcode = LLInt::getOpcode(op_get_by_id_unset);
     617        pc[4].u.structureID = structure->id();
     618        return;
     619    }
     620    ASSERT(slot.isValue());
    610621
    611622    pc[0].u.opcode = LLInt::getOpcode(op_get_by_id_proto_load);
    612623    pc[4].u.structureID = structure->id();
    613624    pc[5].u.operand = offset;
    614     // We know that this pointer will remain valid because it is the prototype of some structure, s,
    615     // watchpointed above. If any object with structure s were to change prototypes then the conditions
    616     // for this cache would fail and this value will never be used again.
     625    // We know that this pointer will remain valid because it will be cleared by either a watchpoint fire or
     626    // during GC when we clear the LLInt caches.
    617627    pc[6].u.pointer = slot.slotBase();
    618628}
    … …  
    633643    if (!LLINT_ALWAYS_ACCESS_SLOW
    634644        && baseValue.isCell()
    635         && slot.isCacheableValue()) {
     645        && slot.isCacheable()) {
    636646
    637647        JSCell* baseCell = baseValue.asCell();
    638648        Structure* structure = baseCell->structure();
    639         if (slot.slotBase() == baseValue) {
     649        if (slot.isValue() && slot.slotBase() == baseValue) {
    640650            // Start out by clearing out the old cache.
    641651            pc[0].u.opcode = LLInt::getOpcode(op_get_by_id);
    … …  
    654664                pc[5].u.operand = slot.cachedOffset();
    655665            }
    656         } else if (UNLIKELY(pc[7].u.operand)) {
     666        } else if (UNLIKELY(pc[7].u.operand && (slot.isValue() || slot.isUnset()))) {
    657667            ASSERT(slot.slotBase() != baseValue);
    658668

  • trunk/Source/JavaScriptCore/llint/LLIntSlowPaths.h

    r201363 r201456  
    7272LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_try_get_by_id);
    7373LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_get_by_id);
    74 LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_get_by_id_proto_load);
    7574LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_get_arguments_length);
    7675LLINT_SLOW_PATH_HIDDEN_DECL(slow_path_put_by_id);

  • trunk/Source/JavaScriptCore/llint/LowLevelInterpreter32_64.asm

    r201363 r201456  
    13381338# since ping-ponging is free. At best we get lucky and the get_by_id will continue
    13391339# to take fast path on the new cache. At worst we take slow path, which is what
    1340 # we would have been doing anyway. For prototype properties, we will attempt to
    1341 # convert opcode into a get_by_id_proto_load after a execution counter hits zero.
     1340# we would have been doing anyway. For prototype/unset properties, we will attempt to
     1341# convert opcode into a get_by_id_proto_load/get_by_id_unset, respectively, after an
     1342# execution counter hits zero.
    13421343
    13431344_llint_op_get_by_id:
    … …  
    13581359    callSlowPath(_llint_slow_path_get_by_id)
    13591360    dispatch(9)
    1360 
    13611361
    13621362
    … …  
    13771377
    13781378.opGetByIdProtoSlow:
     1379    callSlowPath(_llint_slow_path_get_by_id)
     1380    dispatch(9)
     1381
     1382
     1383_llint_op_get_by_id_unset:
     1384    traceExecution()
     1385    loadi 8[PC], t0
     1386    loadi 16[PC], t1
     1387    loadConstantOrVariablePayload(t0, CellTag, t3, .opGetByIdUnsetSlow)
     1388    bineq JSCell::m_structureID[t3], t1, .opGetByIdUnsetSlow
     1389    loadi 4[PC], t2
     1390    storei UndefinedTag, TagOffset[cfr, t2, 8]
     1391    storei 0, PayloadOffset[cfr, t2, 8]
     1392    valueProfile(UndefinedTag, 0, 32, t2)
     1393    dispatch(9)
     1394
     1395.opGetByIdUnsetSlow:
    13791396    callSlowPath(_llint_slow_path_get_by_id)
    13801397    dispatch(9)

  • trunk/Source/JavaScriptCore/llint/LowLevelInterpreter64.asm

    r201363 r201456  
    12531253
    12541254
     1255_llint_op_get_by_id_unset:
     1256    traceExecution()
     1257    loadisFromInstruction(2, t0)
     1258    loadConstantOrVariableCell(t0, t3, .opGetByIdUnsetSlow)
     1259    loadi JSCell::m_structureID[t3], t1
     1260    loadisFromInstruction(4, t2)
     1261    bineq t2, t1, .opGetByIdUnsetSlow
     1262    loadisFromInstruction(1, t2)
     1263    storeq ValueUndefined, [cfr, t2, 8]
     1264    valueProfile(ValueUndefined, 8, t1)
     1265    dispatch(9)
     1266
     1267.opGetByIdUnsetSlow:
     1268    callSlowPath(_llint_slow_path_get_by_id)
     1269    dispatch(9)
     1270
     1271
    12551272_llint_op_get_array_length:
    12561273    traceExecution()
Note: See TracChangeset for help on using the changeset viewer.