Ignore:
Timestamp:
May 27, 2020, 7:43:25 PM (5 years ago)
Author:
[email protected]
Message:

hasOwnProperty inside structure property for-in loop should use an opcode like has_structure_property but for hasOwnProperty
​https://bugs.webkit.org/show_bug.cgi?id=212248

Reviewed by Keith Miller.

JSTests:

  • microbenchmarks/has-own-property-for-in-loop.js: Added.
  • stress/has-own-property-structure-for-in-loop-correctness.js: Added.

Source/JavaScriptCore:

This patch applies the same principles from r262083 but to hasOwnProperty.

In this patch, we have a fast path for this syntactic pattern when
iterating structure properties:

for (let <p> in <o>)

if (<o>.hasOwnProperty(<p>))

We look for both <p> and <o> as resolve nodes, and we look for them being the
same values both in the header and inside the body.

Using a simple static analysis, when we detect this pattern, we compare the
result of <o>.hasOwnProperty to the original hasOwnProperty function. If
it's the same, we execute the fast path new bytecode has_own_structure_property,
which on the fast path is two loads, a compare and branch, and a materialization of
the boolean constant true.

On the slow path, has_own_structure_property just executes the runtime code
for hasOwnProperty.

In my testing, this seems like it might be 3-5% faster on Speedometer 2's
react subtests. I was getting some noise when running the tests locally,
so I can't say for certain it's a definite speedup. But the data implies
it has a good chance at being a speedup.

  • builtins/BuiltinNames.h:
  • bytecode/BytecodeList.rb:
  • bytecode/BytecodeUseDef.cpp:

(JSC::computeUsesForBytecodeIndexImpl):
(JSC::computeDefsForBytecodeIndexImpl):

  • bytecode/LinkTimeConstant.h:
  • bytecompiler/BytecodeGenerator.cpp:

(JSC::BytecodeGenerator::emitWideJumpIfNotFunctionHasOwnProperty):
(JSC::BytecodeGenerator::recordHasOwnStructurePropertyInForInLoop):
(JSC::BytecodeGenerator::emitHasOwnStructureProperty):
(JSC::BytecodeGenerator::pushStructureForInScope):
(JSC::StructureForInContext::finalize):
(JSC::BytecodeGenerator::findStructureForInContext):

  • bytecompiler/BytecodeGenerator.h:

(JSC::StructureForInContext::StructureForInContext):
(JSC::StructureForInContext::base const):
(JSC::StructureForInContext::addHasOwnPropertyJump):

  • bytecompiler/Label.h:

(JSC::GenericBoundLabel::GenericBoundLabel):

  • bytecompiler/NodesCodegen.cpp:

(JSC::HasOwnPropertyFunctionCallDotNode::emitBytecode):
(JSC::ForInNode::emitBytecode):

  • dfg/DFGAbstractInterpreterInlines.h:

(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):

  • dfg/DFGByteCodeParser.cpp:

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

  • dfg/DFGCapabilities.cpp:

(JSC::DFG::capabilityLevel):

  • dfg/DFGClobberize.h:

(JSC::DFG::clobberize):

  • dfg/DFGDoesGC.cpp:

(JSC::DFG::doesGC):

  • dfg/DFGFixupPhase.cpp:

(JSC::DFG::FixupPhase::fixupNode):

  • dfg/DFGNodeType.h:
  • dfg/DFGOperations.cpp:
  • dfg/DFGOperations.h:
  • dfg/DFGPredictionPropagationPhase.cpp:
  • dfg/DFGSafeToExecute.h:

(JSC::DFG::safeToExecute):

  • dfg/DFGSpeculativeJIT.cpp:

(JSC::DFG::SpeculativeJIT::compileHasStructureProperty):
(JSC::DFG::SpeculativeJIT::compileHasOwnStructurePropertyImpl):
(JSC::DFG::SpeculativeJIT::compileHasOwnStructureProperty):
(JSC::DFG::SpeculativeJIT::compileInStructureProperty):

  • dfg/DFGSpeculativeJIT.h:
  • dfg/DFGSpeculativeJIT32_64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

  • dfg/DFGSpeculativeJIT64.cpp:

(JSC::DFG::SpeculativeJIT::compile):

  • ftl/FTLCapabilities.cpp:

(JSC::FTL::canCompile):

  • ftl/FTLLowerDFGToB3.cpp:

(JSC::FTL::DFG::LowerDFGToB3::compileNode):
(JSC::FTL::DFG::LowerDFGToB3::compileHasOwnStructureProperty):

  • jit/JIT.cpp:

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

  • jit/JIT.h:
  • jit/JITOpcodes.cpp:

(JSC::JIT::emit_op_has_structure_propertyImpl):
(JSC::JIT::emit_op_has_structure_property):
(JSC::JIT::emit_op_has_own_structure_property):
(JSC::JIT::emit_op_in_structure_property):

  • jit/JITOpcodes32_64.cpp:

(JSC::JIT::emit_op_has_structure_propertyImpl):
(JSC::JIT::emit_op_has_structure_property):
(JSC::JIT::emit_op_has_own_structure_property):
(JSC::JIT::emit_op_in_structure_property):

  • llint/LowLevelInterpreter.asm:
  • llint/LowLevelInterpreter64.asm:
  • parser/ASTBuilder.h:

(JSC::ASTBuilder::makeFunctionCallNode):

  • parser/NodeConstructors.h:

(JSC::HasOwnPropertyFunctionCallDotNode::HasOwnPropertyFunctionCallDotNode):

  • parser/Nodes.h:
  • runtime/CommonSlowPaths.cpp:

(JSC::SLOW_PATH_DECL):

  • runtime/CommonSlowPaths.h:
  • runtime/JSGlobalObject.cpp:

(JSC::JSGlobalObject::init):

  • runtime/ObjectPrototype.cpp:

(JSC::objectPrototypeHasOwnProperty):
(JSC::objectProtoFuncHasOwnProperty):

  • runtime/ObjectPrototype.h:
Location:
trunk/Source/JavaScriptCore/bytecompiler
Files:
4 edited

Legend:

Unmodified
Added
Removed

  • trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp

    r262165 r262233  
    12921292        m_codeBlock->addJumpTarget(instructions().size());
    12931293        // This disables peephole optimizations when an instruction is a jump target
    1294         m_lastOpcodeID = op_end;
     1294        disablePeepholeOptimization();
    12951295    }
    12961296}
    … …  
    13151315{
    13161316    ASSERT(m_lastInstruction.isValid());
    1317     m_lastOpcodeID = op_end;
     1317    disablePeepholeOptimization();
    13181318    m_writer.rewind(m_lastInstruction);
    13191319}
    … …  
    14571457{
    14581458    OpJneqPtr::emit(this, cond, moveLinkTimeConstant(nullptr, LinkTimeConstant::applyFunction), target.bind(this));
     1459}
     1460
     1461unsigned BytecodeGenerator::emitWideJumpIfNotFunctionHasOwnProperty(RegisterID* cond, Label& target)
     1462{
     1463    OpJneqPtr::emit<OpcodeSize::Wide32>(this, cond, moveLinkTimeConstant(nullptr, LinkTimeConstant::hasOwnPropertyFunction), target.bind(this));
     1464    return m_lastInstruction.offset();
     1465}
     1466
     1467void BytecodeGenerator::recordHasOwnStructurePropertyInForInLoop(StructureForInContext& structureContext, unsigned branchOffset, Label& genericPath)
     1468{
     1469    RELEASE_ASSERT(genericPath.isBound());
     1470    RELEASE_ASSERT(!genericPath.isForward());
     1471    structureContext.addHasOwnPropertyJump(branchOffset, genericPath.location());
    14591472}
    14601473
    … …  
    43294342}
    43304343
     4344RegisterID* BytecodeGenerator::emitHasOwnStructureProperty(RegisterID* dst, RegisterID* base, RegisterID* propertyName, RegisterID* enumerator)
     4345{
     4346    OpHasOwnStructureProperty::emit(this, dst, base, propertyName, enumerator);
     4347    return dst;
     4348}
     4349
    43314350RegisterID* BytecodeGenerator::emitGetPropertyEnumerator(RegisterID* dst, RegisterID* base)
    43324351{
    … …  
    45154534}
    45164535
    4517 void BytecodeGenerator::pushStructureForInScope(RegisterID* localRegister, RegisterID* indexRegister, RegisterID* propertyRegister, RegisterID* enumeratorRegister)
     4536void BytecodeGenerator::pushStructureForInScope(RegisterID* localRegister, RegisterID* indexRegister, RegisterID* propertyRegister, RegisterID* enumeratorRegister, RegisterID* baseRegister)
    45184537{
    45194538    if (!localRegister)
    45204539        return;
    45214540    unsigned bodyBytecodeStartOffset = instructions().size();
    4522     m_forInContextStack.append(adoptRef(*new StructureForInContext(localRegister, indexRegister, propertyRegister, enumeratorRegister, bodyBytecodeStartOffset)));
     4541    m_forInContextStack.append(adoptRef(*new StructureForInContext(localRegister, indexRegister, propertyRegister, enumeratorRegister, baseRegister, bodyBytecodeStartOffset)));
    45234542}
    45244543
    … …  
    51865205    auto bytecode = instruction->as<OldOpType>();
    51875206
    5188     // disable peephole optimizations
    5189     generator.m_lastOpcodeID = op_end;
     5207    generator.disablePeepholeOptimization();
    51905208
    51915209    // Change the opcode to get_by_val.
    … …  
    52225240    for (const auto& instTuple : m_inInsts)
    52235241        rewriteOp<OpInStructureProperty>(generator, instTuple);
     5242
     5243    for (const auto& hasOwnPropertyTuple : m_hasOwnPropertyJumpInsts) {
     5244        static_assert(sizeof(OpJmp) <= sizeof(OpJneqPtr));
     5245        unsigned branchInstIndex = std::get<0>(hasOwnPropertyTuple);
     5246        unsigned newBranchTarget = std::get<1>(hasOwnPropertyTuple);
     5247
     5248        auto instruction = generator.m_writer.ref(branchInstIndex);
     5249        RELEASE_ASSERT(instruction->is<OpJneqPtr>());
     5250        RELEASE_ASSERT(instruction->isWide32());
     5251        auto end = branchInstIndex + instruction->size();
     5252
     5253        generator.m_writer.seek(branchInstIndex);
     5254
     5255        generator.disablePeepholeOptimization();
     5256
     5257        OpJmp::emit(&generator, BoundLabel(static_cast<int>(newBranchTarget) - static_cast<int>(branchInstIndex)));
     5258
     5259        while (generator.m_writer.position() < end)
     5260            OpNop::emit<OpcodeSize::Narrow>(&generator);
     5261    }
    52245262
    52255263    generator.m_writer.seek(generator.m_writer.size());
    … …  
    52715309}
    52725310
     5311StructureForInContext* BytecodeGenerator::findStructureForInContext(RegisterID* property)
     5312{
     5313    for (size_t i = m_forInContextStack.size(); i--; ) {
     5314        ForInContext& context = m_forInContextStack[i].get();
     5315        if (context.local() != property)
     5316            continue;
     5317
     5318        if (!context.isStructureForInContext())
     5319            break;
     5320
     5321        return &context.asStructureForInContext();
     5322    }
     5323
     5324    return nullptr;
     5325}
     5326
    52735327} // namespace JSC
    52745328

  • trunk/Source/JavaScriptCore/bytecompiler/BytecodeGenerator.h

    r262083 r262233  
    240240        using GetInst = std::tuple<unsigned, int>;
    241241        using InInst = GetInst;
    242 
    243         StructureForInContext(RegisterID* localRegister, RegisterID* indexRegister, RegisterID* propertyRegister, RegisterID* enumeratorRegister, unsigned bodyBytecodeStartOffset)
     242        using HasOwnPropertyJumpInst = std::tuple<unsigned, unsigned>;
     243
     244        StructureForInContext(RegisterID* localRegister, RegisterID* indexRegister, RegisterID* propertyRegister, RegisterID* enumeratorRegister, RegisterID* base, unsigned bodyBytecodeStartOffset)
    244245            : ForInContext(localRegister, Type::StructureForIn, bodyBytecodeStartOffset)
    245246            , m_indexRegister(indexRegister)
    246247            , m_propertyRegister(propertyRegister)
    247248            , m_enumeratorRegister(enumeratorRegister)
     249            , m_base(base)
    248250        {
    249251        }
    … …  
    252254        RegisterID* property() const { return m_propertyRegister.get(); }
    253255        RegisterID* enumerator() const { return m_enumeratorRegister.get(); }
     256        RegisterID* base() const { return m_base.get(); }
    254257
    255258        void addGetInst(unsigned instIndex, int propertyRegIndex)
    … …  
    261264        {
    262265            m_inInsts.append(InInst { instIndex, propertyRegIndex });
     266        }
     267
     268        void addHasOwnPropertyJump(unsigned branchInstIndex, unsigned genericPathTarget)
     269        {
     270            m_hasOwnPropertyJumpInsts.append(HasOwnPropertyJumpInst { branchInstIndex, genericPathTarget });
    263271        }
    264272
    … …  
    269277        RefPtr<RegisterID> m_propertyRegister;
    270278        RefPtr<RegisterID> m_enumeratorRegister;
     279        RefPtr<RegisterID> m_base;
    271280        Vector<GetInst> m_getInsts;
    272281        Vector<InInst> m_inInsts;
     282        Vector<HasOwnPropertyJumpInst> m_hasOwnPropertyJumpInsts;
    273283    };
    274284
    … …  
    665675
    666676        void hoistSloppyModeFunctionIfNecessary(const Identifier& functionName);
     677
     678        StructureForInContext* findStructureForInContext(RegisterID* property);
    667679
    668680    private:
    … …  
    871883        void emitJumpIfNotFunctionCall(RegisterID* cond, Label& target);
    872884        void emitJumpIfNotFunctionApply(RegisterID* cond, Label& target);
     885        unsigned emitWideJumpIfNotFunctionHasOwnProperty(RegisterID* cond, Label& target);
     886        void recordHasOwnStructurePropertyInForInLoop(StructureForInContext&, unsigned branchOffset, Label& genericPath);
    873887
    874888        template<typename BinOp, typename JmpOp>
    … …  
    883897        RegisterID* emitHasIndexedProperty(RegisterID* dst, RegisterID* base, RegisterID* propertyName);
    884898        RegisterID* emitHasStructureProperty(RegisterID* dst, RegisterID* base, RegisterID* propertyName, RegisterID* enumerator);
     899        RegisterID* emitHasOwnStructureProperty(RegisterID* dst, RegisterID* base, RegisterID* propertyName, RegisterID* enumerator);
    885900        RegisterID* emitHasGenericProperty(RegisterID* dst, RegisterID* base, RegisterID* propertyName);
    886901        RegisterID* emitGetPropertyEnumerator(RegisterID* dst, RegisterID* base);
    … …  
    10111026        void pushIndexedForInScope(RegisterID* local, RegisterID* index);
    10121027        void popIndexedForInScope(RegisterID* local);
    1013         void pushStructureForInScope(RegisterID* local, RegisterID* index, RegisterID* property, RegisterID* enumerator);
     1028        void pushStructureForInScope(RegisterID* local, RegisterID* index, RegisterID* property, RegisterID* enumerator, RegisterID* base);
    10141029        void popStructureForInScope(RegisterID* local);
    10151030
    … …  
    10751090        RegisterID* emitMove(RegisterID* dst, RegisterID* src);
    10761091
     1092    public:
     1093        void disablePeepholeOptimization() { m_lastOpcodeID = op_end; }
     1094    private:
    10771095        bool canDoPeepholeOptimization() const { return m_lastOpcodeID != op_end; }
    10781096
    … …  
    12161234            auto prevLastInstruction = m_lastInstruction;
    12171235            m_writer.swap(writer);
    1218             m_lastOpcodeID = op_end;
     1236            disablePeepholeOptimization();
    12191237            m_lastInstruction = m_writer.ref();
    12201238            fn();

  • trunk/Source/JavaScriptCore/bytecompiler/Label.h

    r252800 r262233  
    5353        { }
    5454
    55         explicit GenericBoundLabel(int target)
     55        explicit GenericBoundLabel(int offset)
    5656            : m_type(Offset)
    5757            , m_generator(nullptr)
    58             , m_target(target)
     58            , m_target(offset)
    5959        { }
    6060

  • trunk/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp

    r261895 r262233  
    18251825        generator.emitLabel(end.get());
    18261826    }
     1827    generator.emitProfileType(returnValue.get(), divotStart(), divotEnd());
     1828    return returnValue.get();
     1829}
     1830
     1831RegisterID* HasOwnPropertyFunctionCallDotNode::emitBytecode(BytecodeGenerator& generator, RegisterID* dst)
     1832{
     1833    RefPtr<RegisterID> returnValue = generator.finalDestination(dst);
     1834    RefPtr<RegisterID> base = generator.emitNode(m_base);
     1835
     1836    if (m_base->isOptionalChainBase())
     1837        generator.emitOptionalCheck(base.get());
     1838
     1839    generator.emitExpressionInfo(subexpressionDivot(), subexpressionStart(), subexpressionEnd());
     1840
     1841    RefPtr<RegisterID> function = generator.emitGetById(generator.newTemporary(), base.get(), generator.propertyNames().hasOwnProperty);
     1842    if (isOptionalChainBase())
     1843        generator.emitOptionalCheck(function.get());
     1844
     1845    RELEASE_ASSERT(m_args->m_listNode && m_args->m_listNode->m_expr && !m_args->m_listNode->m_next); 
     1846    ExpressionNode* argument = m_args->m_listNode->m_expr;
     1847    RELEASE_ASSERT(argument->isResolveNode());
     1848    StructureForInContext* structureContext = nullptr;
     1849    Variable argumentVariable = generator.variable(static_cast<ResolveNode*>(argument)->identifier());
     1850    if (argumentVariable.isLocal()) {
     1851        RegisterID* property = argumentVariable.local();
     1852        structureContext = generator.findStructureForInContext(property);
     1853    }
     1854
     1855    if (structureContext && structureContext->base() == base.get()) {
     1856        Ref<Label> realCall = generator.newLabel();
     1857        Ref<Label> end = generator.newLabel();
     1858
     1859        unsigned branchInsnOffset = generator.emitWideJumpIfNotFunctionHasOwnProperty(function.get(), realCall.get());
     1860        generator.emitHasOwnStructureProperty(returnValue.get(), base.get(), generator.emitNode(argument), structureContext->enumerator());
     1861        generator.emitJump(end.get());
     1862
     1863        generator.emitLabel(realCall.get());
     1864        {
     1865            CallArguments callArguments(generator, m_args);
     1866            generator.move(callArguments.thisRegister(), base.get());
     1867            generator.emitCallInTailPosition(returnValue.get(), function.get(), NoExpectedFunction, callArguments, divot(), divotStart(), divotEnd(), DebuggableCall::Yes);
     1868        }
     1869
     1870        generator.emitLabel(end.get());
     1871
     1872        generator.recordHasOwnStructurePropertyInForInLoop(*structureContext, branchInsnOffset, realCall);
     1873    } else {
     1874        CallArguments callArguments(generator, m_args);
     1875        generator.move(callArguments.thisRegister(), base.get());
     1876        generator.emitCallInTailPosition(returnValue.get(), function.get(), NoExpectedFunction, callArguments, divot(), divotStart(), divotEnd(), DebuggableCall::Yes);
     1877    }
     1878
    18271879    generator.emitProfileType(returnValue.get(), divotStart(), divotEnd());
    18281880    return returnValue.get();
    … …  
    36963748        generator.emitNode(generator.ignoredResult(), m_lexpr);
    36973749
    3698     RefPtr<RegisterID> base = generator.newTemporary();
    36993750    RefPtr<RegisterID> length;
    37003751    RefPtr<RegisterID> enumerator;
    37013752
    3702     generator.emitNode(base.get(), m_expr);
     3753    RefPtr<RegisterID> base = generator.emitNode(m_expr);
    37033754    RefPtr<RegisterID> local = this->tryGetBoundLocal(generator);
    37043755    RefPtr<RegisterID> enumeratorIndex;
    … …  
    37783829        generator.emitProfileControlFlow(profilerStartOffset);
    37793830
    3780         generator.pushStructureForInScope(local.get(), enumeratorIndex.get(), propertyName.get(), enumerator.get());
     3831        generator.pushStructureForInScope(local.get(), enumeratorIndex.get(), propertyName.get(), enumerator.get(), base.get());
    37813832        generator.emitNode(dst, m_statement);
    37823833        generator.popStructureForInScope(local.get());
Note: See TracChangeset for help on using the changeset viewer.