Changeset 252763 in webkit


Ignore:
Timestamp:
Nov 21, 2019 9:41:56 PM (4 years ago)
Author:
sbarati@apple.com
Message:

GetByStatus should not say it took the slow path for multiple identifiers and should have a way to indicate if the StructureStubInfo it saw took the slow path
​https://bugs.webkit.org/show_bug.cgi?id=204435

Reviewed by Tadeu Zagallo.

JSTests:

  • microbenchmarks/get-by-val-polymorphic-ic-1.js: Added.

(assert):
(test1.foo):
(test1):

  • microbenchmarks/get-by-val-polymorphic-ic-2.js: Added.

(assert):
(test2.foo):
(test2):

  • microbenchmarks/get-by-val-polymorphic-ic-3.js: Added.

(assert):
(test3.foo):
(test3.args):
(test3):

  • microbenchmarks/get-by-val-polymorphic-ic-4.js: Added.

(assert):
(test4.foo):
(test4.capture):
(test4.args):
(test4):

  • microbenchmarks/get-by-val-polymorphic-ic-5.js: Added.

(assert):
(test5.foo):
(test5.capture):
(test5.args):
(test5):

  • microbenchmarks/get-by-val-polymorphic-ic-6.js: Added.

(assert):
(test6.foo):
(test6.capture):
(test6.args):
(test6):

  • microbenchmarks/get-by-val-polymorphic-ic-7.js: Added.

(assert):
(test7.foo):
(test7):

  • microbenchmarks/get-by-val-polymorphic-ic-8.js: Added.

(assert):
(test7.foo):
(test7):

  • microbenchmarks/get-by-val-polymorphic-ic-9.js: Added.

(assert):
(test7.foo):
(test7):

Source/JavaScriptCore:

I discovered some issues with get by val ICs when re-running the microbenchmarks
I wrote. I noticed that we were faster when not running with the DFG. The reason
for this is that we were only emitting a get by val IC in the DFG/FTL when we
observe the GetByStatus says it didn't "go to the slow path". The logic in GetByStatus
for building up a variant list was wrong for ICs with multiple identifiers. We have
a consistency check when building up the list to ensure that no two variants have
structure sets which overlap, because we wouldn't know which one to choose. However,
we were accidentally saying two GetByIdVariants overlap when they had different identifiers.
This patch fixes that bug by also doing an identifier comparison check. Two GetByIdVariants
with different identifiers do not overlap.

We also used to say a GetByStatus "goes to the slow path" if any of the cases were an
array-like load. I wrote that code thinking that ArrayProfile would just handle it.
However, sometimes we have a get by val IC that both has string properties and int32 properties.
In these kinds of scenarios, an IC is super profitable. This patch now distinguishes
between a GetByStatus saying "we're a slow path" and if we actually observed the StructureStubInfo
go to the slow path. In the DFG/FTL, we only forgo emitting a get by val IC when observing a
prior StructureStubInfo that went to the slow path.

I also realized are call to StructureStubInfo::considerCaching was wrong for get by val ICs.
We were only considering the Structure in isolation, not the { Structure, Identifier }
pair. For get by val, we need to consider the pair together, since {s1, "a"}
and {s1, "b"} will be two different access cases.

This patch demonstrates that on these microbenchmarks, get by val ICs can
be between 50-200% faster.

  • bytecode/GetByIdVariant.cpp:

(JSC::GetByIdVariant::dumpInContext const):

  • bytecode/GetByIdVariant.h:

(JSC::GetByIdVariant::overlaps):

  • bytecode/GetByStatus.cpp:

(JSC::GetByStatus::GetByStatus):
(JSC::GetByStatus::computeForStubInfoWithoutExitSiteFeedback):
(JSC::GetByStatus::makesCalls const):
(JSC::GetByStatus::slowVersion const):
(JSC::GetByStatus::merge):
(JSC::GetByStatus::dump const):

  • bytecode/GetByStatus.h:

(JSC::GetByStatus::GetByStatus):
(JSC::GetByStatus::takesSlowPath const):
(JSC::GetByStatus::observedStructureStubInfoSlowPath const):

  • bytecode/ICStatusUtils.h:

(JSC::appendICStatusVariant):

  • bytecode/InByIdVariant.h:

(JSC::InByIdVariant::overlaps):

  • bytecode/InstanceOfVariant.h:

(JSC::InstanceOfVariant::overlaps):

  • bytecode/PutByIdVariant.h:

(JSC::PutByIdVariant::overlaps):

  • bytecode/StructureStubInfo.cpp:

(JSC::StructureStubInfo::visitWeakReferences):

  • bytecode/StructureStubInfo.h:

(JSC::StructureStubInfo::considerCaching):

  • dfg/DFGByteCodeParser.cpp:

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

  • jit/JITOperations.cpp:
Location:
trunk
Files:
9 added
14 edited

Legend:

Unmodified
Added
Removed

  • trunk/JSTests/ChangeLog

    r252758 r252763  
     12019-11-21  Saam Barati  <sbarati@apple.com>
     2
     3        GetByStatus should not say it took the slow path for multiple identifiers and should have a way to indicate if the StructureStubInfo it saw took the slow path
     4        https://bugs.webkit.org/show_bug.cgi?id=204435
     5
     6        Reviewed by Tadeu Zagallo.
     7
     8        * microbenchmarks/get-by-val-polymorphic-ic-1.js: Added.
     9        (assert):
     10        (test1.foo):
     11        (test1):
     12        * microbenchmarks/get-by-val-polymorphic-ic-2.js: Added.
     13        (assert):
     14        (test2.foo):
     15        (test2):
     16        * microbenchmarks/get-by-val-polymorphic-ic-3.js: Added.
     17        (assert):
     18        (test3.foo):
     19        (test3.args):
     20        (test3):
     21        * microbenchmarks/get-by-val-polymorphic-ic-4.js: Added.
     22        (assert):
     23        (test4.foo):
     24        (test4.capture):
     25        (test4.args):
     26        (test4):
     27        * microbenchmarks/get-by-val-polymorphic-ic-5.js: Added.
     28        (assert):
     29        (test5.foo):
     30        (test5.capture):
     31        (test5.args):
     32        (test5):
     33        * microbenchmarks/get-by-val-polymorphic-ic-6.js: Added.
     34        (assert):
     35        (test6.foo):
     36        (test6.capture):
     37        (test6.args):
     38        (test6):
     39        * microbenchmarks/get-by-val-polymorphic-ic-7.js: Added.
     40        (assert):
     41        (test7.foo):
     42        (test7):
     43        * microbenchmarks/get-by-val-polymorphic-ic-8.js: Added.
     44        (assert):
     45        (test7.foo):
     46        (test7):
     47        * microbenchmarks/get-by-val-polymorphic-ic-9.js: Added.
     48        (assert):
     49        (test7.foo):
     50        (test7):
     51
    1522019-11-21  Mark Lam  <mark.lam@apple.com>
    253

  • trunk/Source/JavaScriptCore/ChangeLog

    r252758 r252763  
     12019-11-21  Saam Barati  <sbarati@apple.com>
     2
     3        GetByStatus should not say it took the slow path for multiple identifiers and should have a way to indicate if the StructureStubInfo it saw took the slow path
     4        https://bugs.webkit.org/show_bug.cgi?id=204435
     5
     6        Reviewed by Tadeu Zagallo.
     7
     8        I discovered some issues with get by val ICs when re-running the microbenchmarks
     9        I wrote. I noticed that we were faster when not running with the DFG. The reason
     10        for this is that we were only emitting a get by val IC in the DFG/FTL when we
     11        observe the GetByStatus says it didn't "go to the slow path". The logic in GetByStatus
     12        for building up a variant list was wrong for ICs with multiple identifiers. We have
     13        a consistency check when building up the list to ensure that no two variants have
     14        structure sets which overlap, because we wouldn't know which one to choose. However,
     15        we were accidentally saying two GetByIdVariants overlap when they had different identifiers.
     16        This patch fixes that bug by also doing an identifier comparison check. Two GetByIdVariants
     17        with different identifiers do not overlap.
     18       
     19        We also used to say a GetByStatus "goes to the slow path" if any of the cases were an
     20        array-like load. I wrote that code thinking that ArrayProfile would just handle it.
     21        However, sometimes we have a get by val IC that both has string properties and int32 properties.
     22        In these kinds of scenarios, an IC is super profitable. This patch now distinguishes
     23        between a GetByStatus saying "we're a slow path" and if we actually observed the StructureStubInfo
     24        go to the slow path. In the DFG/FTL, we only forgo emitting a get by val IC when observing a
     25        prior StructureStubInfo that went to the slow path.
     26       
     27        I also realized are call to StructureStubInfo::considerCaching was wrong for get by val ICs.
     28        We were only considering the Structure in isolation, not the { Structure, Identifier }
     29        pair. For get by val, we need to  consider the pair together, since {s1, "a"}
     30        and {s1, "b"} will be two different access cases.
     31       
     32        This patch demonstrates that on these microbenchmarks, get by val ICs can
     33        be between 50-200% faster.
     34
     35        * bytecode/GetByIdVariant.cpp:
     36        (JSC::GetByIdVariant::dumpInContext const):
     37        * bytecode/GetByIdVariant.h:
     38        (JSC::GetByIdVariant::overlaps):
     39        * bytecode/GetByStatus.cpp:
     40        (JSC::GetByStatus::GetByStatus):
     41        (JSC::GetByStatus::computeForStubInfoWithoutExitSiteFeedback):
     42        (JSC::GetByStatus::makesCalls const):
     43        (JSC::GetByStatus::slowVersion const):
     44        (JSC::GetByStatus::merge):
     45        (JSC::GetByStatus::dump const):
     46        * bytecode/GetByStatus.h:
     47        (JSC::GetByStatus::GetByStatus):
     48        (JSC::GetByStatus::takesSlowPath const):
     49        (JSC::GetByStatus::observedStructureStubInfoSlowPath const):
     50        * bytecode/ICStatusUtils.h:
     51        (JSC::appendICStatusVariant):
     52        * bytecode/InByIdVariant.h:
     53        (JSC::InByIdVariant::overlaps):
     54        * bytecode/InstanceOfVariant.h:
     55        (JSC::InstanceOfVariant::overlaps):
     56        * bytecode/PutByIdVariant.h:
     57        (JSC::PutByIdVariant::overlaps):
     58        * bytecode/StructureStubInfo.cpp:
     59        (JSC::StructureStubInfo::visitWeakReferences):
     60        * bytecode/StructureStubInfo.h:
     61        (JSC::StructureStubInfo::considerCaching):
     62        * dfg/DFGByteCodeParser.cpp:
     63        (JSC::DFG::ByteCodeParser::parseBlock):
     64        * jit/JITOperations.cpp:
     65
    1662019-11-21  Mark Lam  <mark.lam@apple.com>
    267

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

    r252684 r252763  
    182182void GetByIdVariant::dumpInContext(PrintStream& out, DumpContext* context) const
    183183{
     184    out.print("<");
     185    out.print("id='", m_identifier ? *m_identifier : Identifier(), "', ");
    184186    if (!isSet()) {
    185         out.print("<empty>");
     187        out.print("empty>");
    186188        return;
    187189    }
    188    
    189     out.print(
    190         "<", inContext(structureSet(), context), ", ", inContext(m_conditionSet, context));
     190    out.print(inContext(structureSet(), context), ", ", inContext(m_conditionSet, context));
    191191    out.print(", offset = ", offset());
    192192    if (m_callLinkStatus)

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

    r252684 r252763  
    8484
    8585    Box<Identifier> identifier() const { return m_identifier; }
     86
     87    bool overlaps(const GetByIdVariant& other)
     88    {
     89        if (!!m_identifier != !!other.m_identifier)
     90            return true;
     91        if (m_identifier) {
     92            if (m_identifier->impl() != other.m_identifier->impl())
     93                return false;
     94        }
     95        return structureSet().overlaps(other.structureSet());
     96    }
    8697   
    8798private:

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

    r252684 r252763  
    5353}
    5454
    55 GetByStatus GetByStatus::computeFromLLInt(CodeBlock* profiledBlock, BytecodeIndex bytecodeIndex)
     55GetByStatus GetByStatus::computeFromLLInt(CodeBlock* profiledBlock, BytecodeIndex bytecodeIndex, TrackIdentifiers trackIdentifiers)
    5656{
    5757    VM& vm = profiledBlock->vm();
    … …  
    9292    }
    9393
     94    ASSERT_UNUSED(trackIdentifiers, trackIdentifiers == TrackIdentifiers::No); // We could make this work in the future, but nobody needs it right now.
     95
    9496    if (!structureID)
    9597        return GetByStatus(NoInformation, false);
    … …  
    112114}
    113115
    114 GetByStatus GetByStatus::computeFor(CodeBlock* profiledBlock, ICStatusMap& map, BytecodeIndex bytecodeIndex, ExitFlag didExit, CallLinkStatus::ExitSiteData callExitSiteData)
     116GetByStatus GetByStatus::computeFor(CodeBlock* profiledBlock, ICStatusMap& map, BytecodeIndex bytecodeIndex, ExitFlag didExit, CallLinkStatus::ExitSiteData callExitSiteData, TrackIdentifiers trackIdentifiers)
    115117{
    116118    ConcurrentJSLocker locker(profiledBlock->m_lock);
    … …  
    120122#if ENABLE(DFG_JIT)
    121123    result = computeForStubInfoWithoutExitSiteFeedback(
    122         locker, profiledBlock, map.get(CodeOrigin(bytecodeIndex)).stubInfo, callExitSiteData);
     124        locker, profiledBlock, map.get(CodeOrigin(bytecodeIndex)).stubInfo, callExitSiteData, trackIdentifiers);
    123125   
    124126    if (didExit)
    … …  
    131133
    132134    if (!result)
    133         return computeFromLLInt(profiledBlock, bytecodeIndex);
     135        return computeFromLLInt(profiledBlock, bytecodeIndex, trackIdentifiers);
    134136   
    135137    return result;
    … …  
    137139
    138140#if ENABLE(JIT)
     141GetByStatus::GetByStatus(StubInfoSummary summary, StructureStubInfo& stubInfo)
     142    : m_wasSeenInJIT(true)
     143{
     144    switch (summary) {
     145    case StubInfoSummary::NoInformation:
     146        m_state = NoInformation;
     147        return;
     148    case StubInfoSummary::Simple:
     149    case StubInfoSummary::MakesCalls:
     150        RELEASE_ASSERT_NOT_REACHED();
     151        return;
     152    case StubInfoSummary::TakesSlowPath:
     153        m_state = stubInfo.tookSlowPath ? ObservedTakesSlowPath : LikelyTakesSlowPath;
     154        return;
     155    case StubInfoSummary::TakesSlowPathAndMakesCalls:
     156        m_state = stubInfo.tookSlowPath ? ObservedSlowPathAndMakesCalls : MakesCalls;
     157        return;
     158    }
     159    RELEASE_ASSERT_NOT_REACHED();
     160}
     161
    139162GetByStatus::GetByStatus(const ModuleNamespaceAccessCase& accessCase)
    140163    : m_moduleNamespaceData(Box<ModuleNamespaceData>::create(ModuleNamespaceData { accessCase.moduleNamespaceObject(), accessCase.moduleEnvironment(), accessCase.scopeOffset(), accessCase.identifier() }))
    … …  
    145168
    146169GetByStatus GetByStatus::computeForStubInfoWithoutExitSiteFeedback(
    147     const ConcurrentJSLocker& locker, CodeBlock* profiledBlock, StructureStubInfo* stubInfo, CallLinkStatus::ExitSiteData callExitSiteData)
     170    const ConcurrentJSLocker& locker, CodeBlock* profiledBlock, StructureStubInfo* stubInfo, CallLinkStatus::ExitSiteData callExitSiteData, TrackIdentifiers trackIdentifiers)
    148171{
    149172    StubInfoSummary summary = StructureStubInfo::summary(stubInfo);
    150173    if (!isInlineable(summary))
    151         return GetByStatus(summary);
     174        return GetByStatus(summary, *stubInfo);
    152175   
    153176    // Finally figure out if we can derive an access strategy.
    … …  
    162185        Structure* structure = stubInfo->u.byIdSelf.baseObjectStructure.get();
    163186        if (structure->takesSlowPathInDFGForImpureProperty())
    164             return GetByStatus(JSC::slowVersion(summary));
     187            return GetByStatus(JSC::slowVersion(summary), *stubInfo);
    165188        Box<Identifier> identifier = stubInfo->getByIdSelfIdentifier();
    166189        UniquedStringImpl* uid = identifier->impl();
    167190        RELEASE_ASSERT(uid);
     191        if (trackIdentifiers == TrackIdentifiers::No)
     192            identifier = nullptr;
    168193        GetByIdVariant variant(WTFMove(identifier));
    169194        unsigned attributes;
    170195        variant.m_offset = structure->getConcurrently(uid, attributes);
    171196        if (!isValidOffset(variant.m_offset))
    172             return GetByStatus(JSC::slowVersion(summary));
     197            return GetByStatus(JSC::slowVersion(summary), *stubInfo);
    173198        if (attributes & PropertyAttribute::CustomAccessorOrValue)
    174             return GetByStatus(JSC::slowVersion(summary));
     199            return GetByStatus(JSC::slowVersion(summary), *stubInfo);
    175200       
    176201        variant.m_structureSet.add(structure);
    … …  
    195220            const AccessCase& access = list->at(listIndex);
    196221            if (access.viaProxy())
    197                 return GetByStatus(JSC::slowVersion(summary));
     222                return GetByStatus(JSC::slowVersion(summary), *stubInfo);
    198223
    199224            if (access.usesPolyProto())
    200                 return GetByStatus(JSC::slowVersion(summary));
     225                return GetByStatus(JSC::slowVersion(summary), *stubInfo);
    201226
    202227            if (!access.requiresIdentifierNameMatch()) {
    … …  
    204229                // information, and probably better than ArrayProfile when it's available.
    205230                // https://bugs.webkit.org/show_bug.cgi?id=204215
    206                 return GetByStatus(JSC::slowVersion(summary));
     231                return GetByStatus(JSC::slowVersion(summary), *stubInfo);
    207232            }
    208233           
    … …  
    215240                // could have told us. But, it works well enough. So, our only concern here is to not
    216241                // crash on null structure.
    217                 return GetByStatus(JSC::slowVersion(summary));
     242                return GetByStatus(JSC::slowVersion(summary), *stubInfo);
    218243            }
    219244           
    … …  
    226251                 
    227252            case ComplexGetStatus::TakesSlowPath:
    228                 return GetByStatus(JSC::slowVersion(summary));
     253                return GetByStatus(JSC::slowVersion(summary), *stubInfo);
    229254                 
    230255            case ComplexGetStatus::Inlineable: {
    … …  
    256281                    customAccessorGetter = access.as<GetterSetterAccessCase>().customAccessor();
    257282                    if (!access.as<GetterSetterAccessCase>().domAttribute())
    258                         return GetByStatus(JSC::slowVersion(summary));
     283                        return GetByStatus(JSC::slowVersion(summary), *stubInfo);
    259284                    domAttribute = WTF::makeUnique<DOMAttributeAnnotation>(*access.as<GetterSetterAccessCase>().domAttribute());
    260285                    haveDOMAttribute = true;
    … …  
    265290                    // FIXME: It would be totally sweet to support more of these at some point in the
    266291                    // future. https://bugs.webkit.org/show_bug.cgi?id=133052
    267                     return GetByStatus(JSC::slowVersion(summary));
     292                    return GetByStatus(JSC::slowVersion(summary), *stubInfo);
    268293                } }
    269294
    270295                ASSERT((AccessCase::Miss == access.type()) == (access.offset() == invalidOffset));
    271296                GetByIdVariant variant(
    272                     access.identifier(), StructureSet(structure), complexGetStatus.offset(),
     297                    trackIdentifiers == TrackIdentifiers::Yes ? access.identifier() : Box<Identifier>(nullptr), StructureSet(structure), complexGetStatus.offset(),
    273298                    complexGetStatus.conditionSet(), WTFMove(callLinkStatus),
    274299                    intrinsicFunction,
    … …  
    277302
    278303                if (!result.appendVariant(variant))
    279                     return GetByStatus(JSC::slowVersion(summary));
     304                    return GetByStatus(JSC::slowVersion(summary), *stubInfo);
    280305
    281306                if (haveDOMAttribute) {
    282307                    // Give up when custom accesses are not merged into one.
    283308                    if (result.numVariants() != 1)
    284                         return GetByStatus(JSC::slowVersion(summary));
     309                        return GetByStatus(JSC::slowVersion(summary), *stubInfo);
    285310                } else {
    286311                    // Give up when custom access and simple access are mixed.
    287312                    if (result.m_state == Custom)
    288                         return GetByStatus(JSC::slowVersion(summary));
     313                        return GetByStatus(JSC::slowVersion(summary), *stubInfo);
    289314                }
    290315                break;
    … …  
    296321       
    297322    default:
    298         return GetByStatus(JSC::slowVersion(summary));
     323        return GetByStatus(JSC::slowVersion(summary), *stubInfo);
    299324    }
    300325   
    … …  
    305330GetByStatus GetByStatus::computeFor(
    306331    CodeBlock* profiledBlock, ICStatusMap& baselineMap,
    307     ICStatusContextStack& icContextStack, CodeOrigin codeOrigin)
     332    ICStatusContextStack& icContextStack, CodeOrigin codeOrigin, TrackIdentifiers trackIdentifiers)
    308333{
    309334    BytecodeIndex bytecodeIndex = codeOrigin.bytecodeIndex();
    … …  
    320345                GetByStatus baselineResult = computeFor(
    321346                    profiledBlock, baselineMap, bytecodeIndex, didExit,
    322                     callExitSiteData);
     347                    callExitSiteData, trackIdentifiers);
    323348                baselineResult.merge(result);
    324349                return baselineResult;
    … …  
    334359                ConcurrentJSLocker locker(context->optimizedCodeBlock->m_lock);
    335360                result = computeForStubInfoWithoutExitSiteFeedback(
    336                     locker, context->optimizedCodeBlock, status.stubInfo, callExitSiteData);
     361                    locker, context->optimizedCodeBlock, status.stubInfo, callExitSiteData, trackIdentifiers);
    337362            }
    338363            if (result.isSet())
    … …  
    344369    }
    345370   
    346     return computeFor(profiledBlock, baselineMap, bytecodeIndex, didExit, callExitSiteData);
     371    return computeFor(profiledBlock, baselineMap, bytecodeIndex, didExit, callExitSiteData, trackIdentifiers);
    347372}
    348373
    … …  
    360385
    361386    if (parseIndex(*uid))
    362         return GetByStatus(TakesSlowPath);
     387        return GetByStatus(LikelyTakesSlowPath);
    363388   
    364389    GetByStatus result;
    … …  
    368393        Structure* structure = set[i];
    369394        if (structure->typeInfo().overridesGetOwnPropertySlot() && structure->typeInfo().type() != GlobalObjectType)
    370             return GetByStatus(TakesSlowPath);
     395            return GetByStatus(LikelyTakesSlowPath);
    371396       
    372397        if (!structure->propertyAccessesAreCacheable())
    373             return GetByStatus(TakesSlowPath);
     398            return GetByStatus(LikelyTakesSlowPath);
    374399       
    375400        unsigned attributes;
    376401        PropertyOffset offset = structure->getConcurrently(uid, attributes);
    377402        if (!isValidOffset(offset))
    378             return GetByStatus(TakesSlowPath); // It's probably a prototype lookup. Give up on life for now, even though we could totally be way smarter about it.
     403            return GetByStatus(LikelyTakesSlowPath); // It's probably a prototype lookup. Give up on life for now, even though we could totally be way smarter about it.
    379404        if (attributes & PropertyAttribute::Accessor)
    380405            return GetByStatus(MakesCalls); // We could be smarter here, like strength-reducing this to a Call.
    381406        if (attributes & PropertyAttribute::CustomAccessorOrValue)
    382             return GetByStatus(TakesSlowPath);
     407            return GetByStatus(LikelyTakesSlowPath);
    383408       
    384409        if (!result.appendVariant(GetByIdVariant(nullptr, structure, offset)))
    385             return GetByStatus(TakesSlowPath);
     410            return GetByStatus(LikelyTakesSlowPath);
    386411    }
    387412   
    … …  
    394419    switch (m_state) {
    395420    case NoInformation:
    396     case TakesSlowPath:
     421    case LikelyTakesSlowPath:
     422    case ObservedTakesSlowPath:
    397423    case Custom:
    398424    case ModuleNamespace:
    … …  
    405431        return false;
    406432    case MakesCalls:
     433    case ObservedSlowPathAndMakesCalls:
    407434        return true;
    408435    }
    … …  
    414441GetByStatus GetByStatus::slowVersion() const
    415442{
    416     return GetByStatus(makesCalls() ? MakesCalls : TakesSlowPath, wasSeenInJIT());
     443    if (observedStructureStubInfoSlowPath())
     444        return GetByStatus(makesCalls() ? ObservedSlowPathAndMakesCalls : ObservedTakesSlowPath, wasSeenInJIT());
     445    return GetByStatus(makesCalls() ? MakesCalls : LikelyTakesSlowPath, wasSeenInJIT());
    417446}
    418447
    … …  
    423452   
    424453    auto mergeSlow = [&] () {
    425         *this = GetByStatus((makesCalls() || other.makesCalls()) ? MakesCalls : TakesSlowPath);
     454        if (observedStructureStubInfoSlowPath() || other.observedStructureStubInfoSlowPath())
     455            *this = GetByStatus((makesCalls() || other.makesCalls()) ? ObservedSlowPathAndMakesCalls : ObservedTakesSlowPath);
     456        else
     457            *this = GetByStatus((makesCalls() || other.makesCalls()) ? MakesCalls : LikelyTakesSlowPath);
    426458    };
    427459   
    … …  
    457489        return;
    458490       
    459     case TakesSlowPath:
     491    case LikelyTakesSlowPath:
     492    case ObservedTakesSlowPath:
    460493    case MakesCalls:
     494    case ObservedSlowPathAndMakesCalls:
    461495        return mergeSlow();
    462496    }
    … …  
    538572        out.print("ModuleNamespace");
    539573        break;
    540     case TakesSlowPath:
    541         out.print("TakesSlowPath");
     574    case LikelyTakesSlowPath:
     575        out.print("LikelyTakesSlowPath");
     576        break;
     577    case ObservedTakesSlowPath:
     578        out.print("ObservedTakesSlowPath");
    542579        break;
    543580    case MakesCalls:
    544581        out.print("MakesCalls");
    545582        break;
     583    case ObservedSlowPathAndMakesCalls:
     584        out.print("ObservedSlowPathAndMakesCalls");
     585        break;
    546586    }
    547587    out.print(", ", listDump(m_variants), ", seenInJIT = ", m_wasSeenInJIT, ")");

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

    r252684 r252763  
    5757        // It's cached for an access to a module namespace object's binding.
    5858        ModuleNamespace,
    59         // It's known to often take slow path.
    60         TakesSlowPath,
    61         // It's known to take paths that make calls.
     59        // It will likely take the slow path.
     60        LikelyTakesSlowPath,
     61        // It's known to take slow path. We also observed that the slow path was taken on StructureStubInfo.
     62        ObservedTakesSlowPath,
     63        // It will likely take the slow path and will make calls.
    6264        MakesCalls,
     65        // It known to take paths that make calls. We also observed that the slow path was taken on StructureStubInfo.
     66        ObservedSlowPathAndMakesCalls ,
     67    };
     68
     69    enum class TrackIdentifiers : uint8_t {
     70        No,  // Used for get_by_id
     71        Yes, // Used for get_by_val
    6372    };
    6473
    … …  
    7180        : m_state(state)
    7281    {
    73         ASSERT(state == NoInformation || state == TakesSlowPath || state == MakesCalls);
     82        ASSERT(state == NoInformation || state == LikelyTakesSlowPath || state == ObservedTakesSlowPath || state == MakesCalls || state == ObservedSlowPathAndMakesCalls);
    7483    }
    7584   
    76     explicit GetByStatus(StubInfoSummary summary)
    77         : m_wasSeenInJIT(true)
    78     {
    79         switch (summary) {
    80         case StubInfoSummary::NoInformation:
    81             m_state = NoInformation;
    82             return;
    83         case StubInfoSummary::Simple:
    84         case StubInfoSummary::MakesCalls:
    85             RELEASE_ASSERT_NOT_REACHED();
    86             return;
    87         case StubInfoSummary::TakesSlowPath:
    88             m_state = TakesSlowPath;
    89             return;
    90         case StubInfoSummary::TakesSlowPathAndMakesCalls:
    91             m_state = MakesCalls;
    92             return;
    93         }
    94         RELEASE_ASSERT_NOT_REACHED();
    95     }
     85    explicit GetByStatus(StubInfoSummary, StructureStubInfo&);
    9686   
    9787    GetByStatus(
    … …  
    10292    }
    10393   
    104     static GetByStatus computeFor(CodeBlock* baselineBlock, ICStatusMap& baselineMap, ICStatusContextStack& dfgContextStack, CodeOrigin);
     94    static GetByStatus computeFor(CodeBlock* baselineBlock, ICStatusMap& baselineMap, ICStatusContextStack& dfgContextStack, CodeOrigin, TrackIdentifiers);
    10595    static GetByStatus computeFor(const StructureSet&, UniquedStringImpl*);
    10696
    … …  
    118108    const GetByIdVariant& operator[](size_t index) const { return at(index); }
    119109
    120     bool takesSlowPath() const { return m_state == TakesSlowPath || m_state == MakesCalls || m_state == Custom || m_state == ModuleNamespace; }
     110    bool takesSlowPath() const { return m_state == LikelyTakesSlowPath || m_state == ObservedTakesSlowPath || m_state == MakesCalls || m_state == ObservedSlowPathAndMakesCalls || m_state == Custom || m_state == ModuleNamespace; }
     111    bool observedStructureStubInfoSlowPath() const { return m_state == ObservedTakesSlowPath || m_state == ObservedSlowPathAndMakesCalls; }
    121112    bool makesCalls() const;
    122113   
    … …  
    124115   
    125116    bool wasSeenInJIT() const { return m_wasSeenInJIT; }
    126    
    127     void merge(const GetByStatus&);
    128117   
    129118    // Attempts to reduce the set of variants to fit the given structure set. This may be approximate.
    … …  
    144133   
    145134private:
     135    void merge(const GetByStatus&);
     136   
    146137#if ENABLE(JIT)
    147138    GetByStatus(const ModuleNamespaceAccessCase&);
    148139    static GetByStatus computeForStubInfoWithoutExitSiteFeedback(
    149         const ConcurrentJSLocker&, CodeBlock* profiledBlock, StructureStubInfo*, CallLinkStatus::ExitSiteData);
     140        const ConcurrentJSLocker&, CodeBlock* profiledBlock, StructureStubInfo*, CallLinkStatus::ExitSiteData, TrackIdentifiers);
    150141#endif
    151     static GetByStatus computeFromLLInt(CodeBlock*, BytecodeIndex);
    152     static GetByStatus computeFor(CodeBlock*, ICStatusMap&, BytecodeIndex, ExitFlag, CallLinkStatus::ExitSiteData);
     142    static GetByStatus computeFromLLInt(CodeBlock*, BytecodeIndex, TrackIdentifiers);
     143    static GetByStatus computeFor(CodeBlock*, ICStatusMap&, BytecodeIndex, ExitFlag, CallLinkStatus::ExitSiteData, TrackIdentifiers);
    153144
    154145    struct ModuleNamespaceData {

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

    r251690 r252763  
    4444                if (i == j)
    4545                    continue;
    46                 if (variants[j].structureSet().overlaps(mergedVariant.structureSet()))
     46                if (variants[j].overlaps(mergedVariant))
    4747                    return false;
    4848            }
    … …  
    5555    // defensive and bail if we detect crazy.
    5656    for (unsigned i = 0; i < variants.size(); ++i) {
    57         if (variants[i].structureSet().overlaps(variant.structureSet()))
     57        if (variants[i].overlaps(variant))
    5858            return false;
    5959    }

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

    r248546 r252763  
    6464    void dumpInContext(PrintStream&, DumpContext*) const;
    6565
     66    bool overlaps(const InByIdVariant& other)
     67    {
     68        return structureSet().overlaps(other.structureSet());
     69    }
     70
    6671private:
    6772    friend class InByIdStatus;

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

    r248546 r252763  
    5454    void dump(PrintStream&) const;
    5555    void dumpInContext(PrintStream&, DumpContext*) const;
     56
     57    bool overlaps(const InstanceOfVariant& other)
     58    {
     59        return structureSet().overlaps(other.structureSet());
     60    }
    5661   
    5762private:

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

    r248546 r252763  
    139139    void dumpInContext(PrintStream&, DumpContext*) const;
    140140
     141    bool overlaps(const PutByIdVariant& other)
     142    {
     143        return structureSet().overlaps(other.structureSet());
     144    }
     145
    141146private:
    142147    bool attemptToMergeTransitionWithReplace(const PutByIdVariant& replace);

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

    r252684 r252763  
    284284    VM& vm = codeBlock->vm();
    285285   
    286     bufferedStructures.genericFilter(
    287         [&] (Structure* structure) -> bool {
    288             return vm.heap.isMarked(structure);
     286    bufferedStructures.removeIf(
     287        [&] (auto& pair) -> bool {
     288            Structure* structure = pair.first;
     289            return !vm.heap.isMarked(structure);
    289290        });
    290291

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

    r252684 r252763  
    9595    bool propagateTransitions(SlotVisitor&);
    9696       
    97     ALWAYS_INLINE bool considerCaching(VM& vm, CodeBlock* codeBlock, Structure* structure)
     97    ALWAYS_INLINE bool considerCaching(VM& vm, CodeBlock* codeBlock, Structure* structure, UniquedStringImpl* impl = nullptr)
    9898    {
    9999        DisallowGC disallowGC;
    … …  
    155155            // the base's structure. That seems unlikely for the canonical use of instanceof, where
    156156            // the prototype is fixed.
    157             bool isNewlyAdded = bufferedStructures.add(structure);
     157            bool isNewlyAdded = bufferedStructures.add({ structure, impl }).isNewEntry;
    158158            if (isNewlyAdded)
    159159                vm.heap.writeBarrier(codeBlock);
    … …  
    189189    }
    190190   
     191private:
    191192    // Represents those structures that already have buffered AccessCases in the PolymorphicAccess.
    192193    // Note that it's always safe to clear this. If we clear it prematurely, then if we see the same
    193194    // structure again during this buffering countdown, we will create an AccessCase object for it.
    194195    // That's not so bad - we'll get rid of the redundant ones once we regenerate.
    195     StructureSet bufferedStructures;
     196    HashSet<std::pair<Structure*, RefPtr<UniquedStringImpl>>> bufferedStructures;
     197public:
    196198   
    197199    struct {

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

    r252684 r252763  
    47144714        m_inlineStackTop->m_profiledBlock,
    47154715        m_inlineStackTop->m_baselineMap, m_icContextStack,
    4716         currentCodeOrigin());
     4716        currentCodeOrigin(), GetByStatus::TrackIdentifiers::No);
    47174717
    47184718    AccessType type = AccessType::GetById;
    … …  
    56235623            Node* property = get(bytecode.m_property);
    56245624            bool shouldCompileAsGetById = false;
    5625             GetByStatus getByStatus = GetByStatus::computeFor(m_inlineStackTop->m_profiledBlock, m_inlineStackTop->m_baselineMap, m_icContextStack, currentCodeOrigin());
     5625            GetByStatus getByStatus = GetByStatus::computeFor(m_inlineStackTop->m_profiledBlock, m_inlineStackTop->m_baselineMap, m_icContextStack, currentCodeOrigin(), GetByStatus::TrackIdentifiers::Yes);
    56265626            unsigned identifierNumber = 0;
    56275627            {
    … …  
    56565656                m_exitOK = false; // GetByVal must be treated as if it clobbers exit state, since FixupPhase may make it generic.
    56575657                set(bytecode.m_dst, getByVal);
    5658                 if (getByStatus.takesSlowPath())
     5658                if (getByStatus.observedStructureStubInfoSlowPath())
    56595659                    m_graph.m_slowGetByVal.add(getByVal);
    56605660            }

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

    r252756 r252763  
    20372037                LOG_IC((ICEvent::OperationGetByValOptimize, baseValue.classInfoOrNull(vm), propertyName, baseValue == slot.slotBase()));
    20382038               
    2039                 if (stubInfo->considerCaching(vm, codeBlock, baseValue.structureOrNull()))
     2039                if (stubInfo->considerCaching(vm, codeBlock, baseValue.structureOrNull(), propertyName.impl()))
    20402040                    repatchGetBy(globalObject, codeBlock, baseValue, propertyName, slot, *stubInfo, GetByKind::NormalByVal);
    20412041                return found ? slot.getValue(globalObject, propertyName) : jsUndefined();
Note: See TracChangeset for help on using the changeset viewer.