JSC::Symbol should be hash-consed
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Jul 2016 07:15:01 +0000 (07:15 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 29 Jul 2016 07:15:01 +0000 (07:15 +0000)
https://bugs.webkit.org/show_bug.cgi?id=158908

Reviewed by Filip Pizlo.

Previously, SymbolImpls held by symbols represent identity of symbols.
When we check the equality between symbols, we need to load SymbolImpls of symbols and compare them.

This patch performs hash-consing onto the symbols. We cache symbols in per-VM's SymbolImpl-keyed WeakGCMap.
When creating a new symbol from SymbolImpl, we first query to this map and reuse the previously created symbol
if it is found. This ensures that one-on-one correspondence between SymbolImpl and symbol. So now, we can use
pointer-comparison to query the equality of symbols.

This change drops SymbolImpl loads when checking the equality. Furthermore, we can use DFG CheckCell to symbol
when we would like to ensure that the given value is the expected symbol. This cleans up GetByVal's symbol-keyd
caching. Then, we changed CheckIdent to CheckStringIdent since it only checks the string case now. The symbol
case is handled by CheckCell.

Additionally, this patch also cleans up Map / Set implementation since we can use the logic for JSCell to symbols.

The performance effects in the related benchmarks are the followings.

                                                       baseline                   patch

    bigswitch-indirect-symbol-or-undefined         85.6214+-1.0063     ^     63.0522+-0.8615        ^ definitely 1.3579x faster
    bigswitch-indirect-symbol                      84.9653+-0.6258     ^     80.4900+-0.8008        ^ definitely 1.0556x faster
    fold-put-by-val-with-symbol-to-multi-put-by-offset
                                                    9.4396+-0.3726            9.2941+-0.3311          might be 1.0157x faster
    inlined-put-by-val-with-symbol-transition
                                                   49.5477+-0.2401     ?     49.7533+-0.3369        ?
    get-by-val-with-symbol-self-or-proto           11.9740+-0.0798     ?     12.1706+-0.2723        ? might be 1.0164x slower
    get-by-val-with-symbol-quadmorphic-check-structure-elimination-simple
                                                    4.1364+-0.0841            4.0872+-0.0925          might be 1.0120x faster
    put-by-val-with-symbol                         11.3709+-0.0223           11.3613+-0.0264
    get-by-val-with-symbol-proto-or-self           11.8984+-0.0706     ?     11.9030+-0.0787        ?
    polymorphic-put-by-val-with-symbol             31.4176+-0.0558           31.3825+-0.0447
    implicit-bigswitch-indirect-symbol             61.3115+-0.6577     ^     58.0098+-0.1212        ^ definitely 1.0569x faster
    get-by-val-with-symbol-bimorphic-check-structure-elimination-simple
                                                    3.3139+-0.0565     ^      2.9947+-0.0732        ^ definitely 1.1066x faster
    get-by-val-with-symbol-chain-from-try-block
                                                    2.2316+-0.0179            2.2137+-0.0210
    get-by-val-with-symbol-bimorphic-check-structure-elimination
                                                   10.6031+-0.2216     ^     10.0939+-0.1977        ^ definitely 1.0504x faster
    get-by-val-with-symbol-check-structure-elimination
                                                    8.5576+-0.1521     ^      7.7107+-0.1308        ^ definitely 1.1098x faster
    put-by-val-with-symbol-slightly-polymorphic
                                                    3.1957+-0.0538     ^      2.9181+-0.0708        ^ definitely 1.0951x faster
    put-by-val-with-symbol-replace-and-transition
                                                   11.8253+-0.0757     ^     11.6590+-0.0351        ^ definitely 1.0143x faster

    <geometric>                                    13.3911+-0.0527     ^     12.7376+-0.0457        ^ definitely 1.0513x faster

* bytecode/ByValInfo.h:
* bytecode/CodeBlock.cpp:
(JSC::CodeBlock::stronglyVisitStrongReferences):
* dfg/DFGAbstractInterpreterInlines.h:
(JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
* dfg/DFGByteCodeParser.cpp:
(JSC::DFG::ByteCodeParser::parseBlock):
* dfg/DFGClobberize.h:
(JSC::DFG::clobberize):
* dfg/DFGConstantFoldingPhase.cpp:
(JSC::DFG::ConstantFoldingPhase::foldConstants):
* dfg/DFGDoesGC.cpp:
(JSC::DFG::doesGC):
* dfg/DFGFixupPhase.cpp:
(JSC::DFG::FixupPhase::fixupNode):
* dfg/DFGNode.h:
(JSC::DFG::Node::hasUidOperand):
* dfg/DFGNodeType.h:
* dfg/DFGPredictionPropagationPhase.cpp:
* dfg/DFGSafeToExecute.h:
(JSC::DFG::safeToExecute):
* dfg/DFGSpeculativeJIT.cpp:
(JSC::DFG::SpeculativeJIT::compileSymbolEquality):
(JSC::DFG::SpeculativeJIT::compilePeepHoleSymbolEquality):
(JSC::DFG::SpeculativeJIT::compileCheckStringIdent):
(JSC::DFG::SpeculativeJIT::extractStringImplFromBinarySymbols): Deleted.
(JSC::DFG::SpeculativeJIT::compileCheckIdent): Deleted.
(JSC::DFG::SpeculativeJIT::compileSymbolUntypedEquality): Deleted.
* dfg/DFGSpeculativeJIT.h:
* dfg/DFGSpeculativeJIT32_64.cpp:
(JSC::DFG::SpeculativeJIT::compileSymbolUntypedEquality):
(JSC::DFG::SpeculativeJIT::compile):
* dfg/DFGSpeculativeJIT64.cpp:
(JSC::DFG::SpeculativeJIT::compileSymbolUntypedEquality):
(JSC::DFG::SpeculativeJIT::compile):
* ftl/FTLAbstractHeapRepository.h:
* ftl/FTLCapabilities.cpp:
(JSC::FTL::canCompile):
* ftl/FTLLowerDFGToB3.cpp:
(JSC::FTL::DFG::LowerDFGToB3::compileNode):
(JSC::FTL::DFG::LowerDFGToB3::compileCheckStringIdent):
(JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq):
(JSC::FTL::DFG::LowerDFGToB3::compileCheckIdent): Deleted.
(JSC::FTL::DFG::LowerDFGToB3::lowSymbolUID): Deleted.
* jit/JIT.h:
* jit/JITOperations.cpp:
(JSC::tryGetByValOptimize):
* jit/JITPropertyAccess.cpp:
(JSC::JIT::emitGetByValWithCachedId):
(JSC::JIT::emitPutByValWithCachedId):
(JSC::JIT::emitByValIdentifierCheck):
(JSC::JIT::privateCompileGetByValWithCachedId):
(JSC::JIT::privateCompilePutByValWithCachedId):
(JSC::JIT::emitIdentifierCheck): Deleted.
* jit/JITPropertyAccess32_64.cpp:
(JSC::JIT::emitGetByValWithCachedId):
(JSC::JIT::emitPutByValWithCachedId):
* runtime/JSCJSValue.cpp:
(JSC::JSValue::dumpInContextAssumingStructure):
* runtime/JSCJSValueInlines.h:
(JSC::JSValue::equalSlowCaseInline):
(JSC::JSValue::strictEqualSlowCaseInline): Deleted.
* runtime/JSFunction.cpp:
(JSC::JSFunction::setFunctionName):
* runtime/MapData.h:
* runtime/MapDataInlines.h:
(JSC::JSIterator>::clear): Deleted.
(JSC::JSIterator>::find): Deleted.
(JSC::JSIterator>::add): Deleted.
(JSC::JSIterator>::remove): Deleted.
(JSC::JSIterator>::replaceAndPackBackingStore): Deleted.
* runtime/Symbol.cpp:
(JSC::Symbol::finishCreation):
(JSC::Symbol::create):
* runtime/Symbol.h:
* runtime/VM.cpp:
(JSC::VM::VM):
* runtime/VM.h:
* tests/stress/symbol-equality-over-gc.js: Added.
(shouldBe):
(test):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@203895 268f45cc-cd09-0410-ab3c-d52691b4dbfc

34 files changed:
Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/ByValInfo.h
Source/JavaScriptCore/bytecode/CodeBlock.cpp
Source/JavaScriptCore/dfg/DFGAbstractInterpreterInlines.h
Source/JavaScriptCore/dfg/DFGByteCodeParser.cpp
Source/JavaScriptCore/dfg/DFGClobberize.h
Source/JavaScriptCore/dfg/DFGConstantFoldingPhase.cpp
Source/JavaScriptCore/dfg/DFGDoesGC.cpp
Source/JavaScriptCore/dfg/DFGFixupPhase.cpp
Source/JavaScriptCore/dfg/DFGNode.h
Source/JavaScriptCore/dfg/DFGNodeType.h
Source/JavaScriptCore/dfg/DFGPredictionPropagationPhase.cpp
Source/JavaScriptCore/dfg/DFGSafeToExecute.h
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT.h
Source/JavaScriptCore/dfg/DFGSpeculativeJIT32_64.cpp
Source/JavaScriptCore/dfg/DFGSpeculativeJIT64.cpp
Source/JavaScriptCore/ftl/FTLAbstractHeapRepository.h
Source/JavaScriptCore/ftl/FTLCapabilities.cpp
Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
Source/JavaScriptCore/jit/JIT.h
Source/JavaScriptCore/jit/JITOperations.cpp
Source/JavaScriptCore/jit/JITPropertyAccess.cpp
Source/JavaScriptCore/jit/JITPropertyAccess32_64.cpp
Source/JavaScriptCore/runtime/JSCJSValue.cpp
Source/JavaScriptCore/runtime/JSCJSValueInlines.h
Source/JavaScriptCore/runtime/JSFunction.cpp
Source/JavaScriptCore/runtime/MapData.h
Source/JavaScriptCore/runtime/MapDataInlines.h
Source/JavaScriptCore/runtime/Symbol.cpp
Source/JavaScriptCore/runtime/Symbol.h
Source/JavaScriptCore/runtime/VM.cpp
Source/JavaScriptCore/runtime/VM.h
Source/JavaScriptCore/tests/stress/symbol-equality-over-gc.js [new file with mode: 0644]

index 174525f..31e31ff 100644 (file)
@@ -1,3 +1,139 @@
+2016-07-28  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        JSC::Symbol should be hash-consed
+        https://bugs.webkit.org/show_bug.cgi?id=158908
+
+        Reviewed by Filip Pizlo.
+
+        Previously, SymbolImpls held by symbols represent identity of symbols.
+        When we check the equality between symbols, we need to load SymbolImpls of symbols and compare them.
+
+        This patch performs hash-consing onto the symbols. We cache symbols in per-VM's SymbolImpl-keyed WeakGCMap.
+        When creating a new symbol from SymbolImpl, we first query to this map and reuse the previously created symbol
+        if it is found. This ensures that one-on-one correspondence between SymbolImpl and symbol. So now, we can use
+        pointer-comparison to query the equality of symbols.
+
+        This change drops SymbolImpl loads when checking the equality. Furthermore, we can use DFG CheckCell to symbol
+        when we would like to ensure that the given value is the expected symbol. This cleans up GetByVal's symbol-keyd
+        caching. Then, we changed CheckIdent to CheckStringIdent since it only checks the string case now. The symbol
+        case is handled by CheckCell.
+
+        Additionally, this patch also cleans up Map / Set implementation since we can use the logic for JSCell to symbols.
+
+        The performance effects in the related benchmarks are the followings.
+
+                                                               baseline                   patch
+
+            bigswitch-indirect-symbol-or-undefined         85.6214+-1.0063     ^     63.0522+-0.8615        ^ definitely 1.3579x faster
+            bigswitch-indirect-symbol                      84.9653+-0.6258     ^     80.4900+-0.8008        ^ definitely 1.0556x faster
+            fold-put-by-val-with-symbol-to-multi-put-by-offset
+                                                            9.4396+-0.3726            9.2941+-0.3311          might be 1.0157x faster
+            inlined-put-by-val-with-symbol-transition
+                                                           49.5477+-0.2401     ?     49.7533+-0.3369        ?
+            get-by-val-with-symbol-self-or-proto           11.9740+-0.0798     ?     12.1706+-0.2723        ? might be 1.0164x slower
+            get-by-val-with-symbol-quadmorphic-check-structure-elimination-simple
+                                                            4.1364+-0.0841            4.0872+-0.0925          might be 1.0120x faster
+            put-by-val-with-symbol                         11.3709+-0.0223           11.3613+-0.0264
+            get-by-val-with-symbol-proto-or-self           11.8984+-0.0706     ?     11.9030+-0.0787        ?
+            polymorphic-put-by-val-with-symbol             31.4176+-0.0558           31.3825+-0.0447
+            implicit-bigswitch-indirect-symbol             61.3115+-0.6577     ^     58.0098+-0.1212        ^ definitely 1.0569x faster
+            get-by-val-with-symbol-bimorphic-check-structure-elimination-simple
+                                                            3.3139+-0.0565     ^      2.9947+-0.0732        ^ definitely 1.1066x faster
+            get-by-val-with-symbol-chain-from-try-block
+                                                            2.2316+-0.0179            2.2137+-0.0210
+            get-by-val-with-symbol-bimorphic-check-structure-elimination
+                                                           10.6031+-0.2216     ^     10.0939+-0.1977        ^ definitely 1.0504x faster
+            get-by-val-with-symbol-check-structure-elimination
+                                                            8.5576+-0.1521     ^      7.7107+-0.1308        ^ definitely 1.1098x faster
+            put-by-val-with-symbol-slightly-polymorphic
+                                                            3.1957+-0.0538     ^      2.9181+-0.0708        ^ definitely 1.0951x faster
+            put-by-val-with-symbol-replace-and-transition
+                                                           11.8253+-0.0757     ^     11.6590+-0.0351        ^ definitely 1.0143x faster
+
+            <geometric>                                    13.3911+-0.0527     ^     12.7376+-0.0457        ^ definitely 1.0513x faster
+
+        * bytecode/ByValInfo.h:
+        * bytecode/CodeBlock.cpp:
+        (JSC::CodeBlock::stronglyVisitStrongReferences):
+        * dfg/DFGAbstractInterpreterInlines.h:
+        (JSC::DFG::AbstractInterpreter<AbstractStateType>::executeEffects):
+        * dfg/DFGByteCodeParser.cpp:
+        (JSC::DFG::ByteCodeParser::parseBlock):
+        * dfg/DFGClobberize.h:
+        (JSC::DFG::clobberize):
+        * dfg/DFGConstantFoldingPhase.cpp:
+        (JSC::DFG::ConstantFoldingPhase::foldConstants):
+        * dfg/DFGDoesGC.cpp:
+        (JSC::DFG::doesGC):
+        * dfg/DFGFixupPhase.cpp:
+        (JSC::DFG::FixupPhase::fixupNode):
+        * dfg/DFGNode.h:
+        (JSC::DFG::Node::hasUidOperand):
+        * dfg/DFGNodeType.h:
+        * dfg/DFGPredictionPropagationPhase.cpp:
+        * dfg/DFGSafeToExecute.h:
+        (JSC::DFG::safeToExecute):
+        * dfg/DFGSpeculativeJIT.cpp:
+        (JSC::DFG::SpeculativeJIT::compileSymbolEquality):
+        (JSC::DFG::SpeculativeJIT::compilePeepHoleSymbolEquality):
+        (JSC::DFG::SpeculativeJIT::compileCheckStringIdent):
+        (JSC::DFG::SpeculativeJIT::extractStringImplFromBinarySymbols): Deleted.
+        (JSC::DFG::SpeculativeJIT::compileCheckIdent): Deleted.
+        (JSC::DFG::SpeculativeJIT::compileSymbolUntypedEquality): Deleted.
+        * dfg/DFGSpeculativeJIT.h:
+        * dfg/DFGSpeculativeJIT32_64.cpp:
+        (JSC::DFG::SpeculativeJIT::compileSymbolUntypedEquality):
+        (JSC::DFG::SpeculativeJIT::compile):
+        * dfg/DFGSpeculativeJIT64.cpp:
+        (JSC::DFG::SpeculativeJIT::compileSymbolUntypedEquality):
+        (JSC::DFG::SpeculativeJIT::compile):
+        * ftl/FTLAbstractHeapRepository.h:
+        * ftl/FTLCapabilities.cpp:
+        (JSC::FTL::canCompile):
+        * ftl/FTLLowerDFGToB3.cpp:
+        (JSC::FTL::DFG::LowerDFGToB3::compileNode):
+        (JSC::FTL::DFG::LowerDFGToB3::compileCheckStringIdent):
+        (JSC::FTL::DFG::LowerDFGToB3::compileCompareStrictEq):
+        (JSC::FTL::DFG::LowerDFGToB3::compileCheckIdent): Deleted.
+        (JSC::FTL::DFG::LowerDFGToB3::lowSymbolUID): Deleted.
+        * jit/JIT.h:
+        * jit/JITOperations.cpp:
+        (JSC::tryGetByValOptimize):
+        * jit/JITPropertyAccess.cpp:
+        (JSC::JIT::emitGetByValWithCachedId):
+        (JSC::JIT::emitPutByValWithCachedId):
+        (JSC::JIT::emitByValIdentifierCheck):
+        (JSC::JIT::privateCompileGetByValWithCachedId):
+        (JSC::JIT::privateCompilePutByValWithCachedId):
+        (JSC::JIT::emitIdentifierCheck): Deleted.
+        * jit/JITPropertyAccess32_64.cpp:
+        (JSC::JIT::emitGetByValWithCachedId):
+        (JSC::JIT::emitPutByValWithCachedId):
+        * runtime/JSCJSValue.cpp:
+        (JSC::JSValue::dumpInContextAssumingStructure):
+        * runtime/JSCJSValueInlines.h:
+        (JSC::JSValue::equalSlowCaseInline):
+        (JSC::JSValue::strictEqualSlowCaseInline): Deleted.
+        * runtime/JSFunction.cpp:
+        (JSC::JSFunction::setFunctionName):
+        * runtime/MapData.h:
+        * runtime/MapDataInlines.h:
+        (JSC::JSIterator>::clear): Deleted.
+        (JSC::JSIterator>::find): Deleted.
+        (JSC::JSIterator>::add): Deleted.
+        (JSC::JSIterator>::remove): Deleted.
+        (JSC::JSIterator>::replaceAndPackBackingStore): Deleted.
+        * runtime/Symbol.cpp:
+        (JSC::Symbol::finishCreation):
+        (JSC::Symbol::create):
+        * runtime/Symbol.h:
+        * runtime/VM.cpp:
+        (JSC::VM::VM):
+        * runtime/VM.h:
+        * tests/stress/symbol-equality-over-gc.js: Added.
+        (shouldBe):
+        (test):
+
 2016-07-28  Mark Lam  <mark.lam@apple.com>
 
         ASSERTION FAILED in errorProtoFuncToString() when Error name is a single char string.
index 531481e..89886f9 100644 (file)
@@ -35,6 +35,8 @@
 
 namespace JSC {
 
+class Symbol;
+
 #if ENABLE(JIT)
 
 class StructureStubInfo;
@@ -232,6 +234,7 @@ struct ByValInfo {
     unsigned slowPathCount;
     RefPtr<JITStubRoutine> stubRoutine;
     Identifier cachedId;
+    WriteBarrier<Symbol> cachedSymbol;
     StructureStubInfo* stubInfo;
     bool tookSlowPath : 1;
     bool seen : 1;
index 032e1ea..7ea5357 100644 (file)
@@ -3102,6 +3102,9 @@ void CodeBlock::stronglyVisitStrongReferences(SlotVisitor& visitor)
     for (unsigned i = 0; i < m_objectAllocationProfiles.size(); ++i)
         m_objectAllocationProfiles[i].visitAggregate(visitor);
 
+    for (ByValInfo* byValInfo : m_byValInfos)
+        visitor.append(&byValInfo->cachedSymbol);
+
 #if ENABLE(DFG_JIT)
     if (JITCode::isOptimizingJIT(jitType()))
         visitOSRExitTargets(visitor);
index 5a425f0..03f9ad0 100644 (file)
@@ -1398,7 +1398,7 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
             }
 
             if (node->op() == CompareEq && leftConst.isSymbol() && rightConst.isSymbol()) {
-                setConstant(node, jsBoolean(asSymbol(leftConst)->privateName() == asSymbol(rightConst)->privateName()));
+                setConstant(node, jsBoolean(asSymbol(leftConst) == asSymbol(rightConst)));
                 break;
             }
         }
@@ -2626,29 +2626,21 @@ bool AbstractInterpreter<AbstractStateType>::executeEffects(unsigned clobberLimi
         break;
     }
 
-    case CheckIdent: {
+    case CheckStringIdent: {
         AbstractValue& value = forNode(node->child1());
         UniquedStringImpl* uid = node->uidOperand();
-        ASSERT(uid->isSymbol() ? !(value.m_type & ~SpecSymbol) : !(value.m_type & ~SpecStringIdent)); // Edge filtering should have already ensured this.
+        ASSERT(!(value.m_type & ~SpecStringIdent)); // Edge filtering should have already ensured this.
 
         JSValue childConstant = value.value();
         if (childConstant) {
-            if (uid->isSymbol()) {
-                ASSERT(childConstant.isSymbol());
-                if (asSymbol(childConstant)->privateName().uid() == uid) {
-                    m_state.setFoundConstants(true);
-                    break;
-                }
-            } else {
-                ASSERT(childConstant.isString());
-                if (asString(childConstant)->tryGetValueImpl() == uid) {
-                    m_state.setFoundConstants(true);
-                    break;
-                }
+            ASSERT(childConstant.isString());
+            if (asString(childConstant)->tryGetValueImpl() == uid) {
+                m_state.setFoundConstants(true);
+                break;
             }
         }
 
-        filter(value, uid->isSymbol() ? SpecSymbol : SpecStringIdent);
+        filter(value, SpecStringIdent);
         break;
     }
 
index 026d083..b2bb9b0 100644 (file)
@@ -4063,12 +4063,18 @@ bool ByteCodeParser::parseBlock(unsigned limit)
                 ByValInfo* byValInfo = m_inlineStackTop->m_byValInfos.get(CodeOrigin(currentCodeOrigin().bytecodeIndex));
                 // FIXME: When the bytecode is not compiled in the baseline JIT, byValInfo becomes null.
                 // At that time, there is no information.
-                if (byValInfo && byValInfo->stubInfo && !byValInfo->tookSlowPath && !m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadIdent)) {
+                if (byValInfo && byValInfo->stubInfo && !byValInfo->tookSlowPath && !m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadIdent) && !m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadCell)) {
                     compiledAsGetById = true;
                     identifierNumber = m_graph.identifiers().ensure(byValInfo->cachedId.impl());
                     UniquedStringImpl* uid = m_graph.identifiers()[identifierNumber];
 
-                    addToGraph(CheckIdent, OpInfo(uid), property);
+                    if (Symbol* symbol = byValInfo->cachedSymbol.get()) {
+                        FrozenValue* frozen = m_graph.freezeStrong(symbol);
+                        addToGraph(CheckCell, OpInfo(frozen), property);
+                    } else {
+                        ASSERT(!uid->isSymbol());
+                        addToGraph(CheckStringIdent, OpInfo(uid), property);
+                    }
 
                     getByIdStatus = GetByIdStatus::computeForStubInfo(
                         locker, m_inlineStackTop->m_profiledBlock,
@@ -4110,12 +4116,18 @@ bool ByteCodeParser::parseBlock(unsigned limit)
                 ByValInfo* byValInfo = m_inlineStackTop->m_byValInfos.get(CodeOrigin(currentCodeOrigin().bytecodeIndex));
                 // FIXME: When the bytecode is not compiled in the baseline JIT, byValInfo becomes null.
                 // At that time, there is no information.
-                if (byValInfo && byValInfo->stubInfo && !byValInfo->tookSlowPath && !m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadIdent)) {
+                if (byValInfo && byValInfo->stubInfo && !byValInfo->tookSlowPath && !m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadIdent) && !m_inlineStackTop->m_exitProfile.hasExitSite(m_currentIndex, BadCell)) {
                     compiledAsPutById = true;
                     unsigned identifierNumber = m_graph.identifiers().ensure(byValInfo->cachedId.impl());
                     UniquedStringImpl* uid = m_graph.identifiers()[identifierNumber];
 
-                    addToGraph(CheckIdent, OpInfo(uid), property);
+                    if (Symbol* symbol = byValInfo->cachedSymbol.get()) {
+                        FrozenValue* frozen = m_graph.freezeStrong(symbol);
+                        addToGraph(CheckCell, OpInfo(frozen), property);
+                    } else {
+                        ASSERT(!uid->isSymbol());
+                        addToGraph(CheckStringIdent, OpInfo(uid), property);
+                    }
 
                     PutByIdStatus putByIdStatus = PutByIdStatus::computeForStubInfo(
                         locker, m_inlineStackTop->m_profiledBlock,
index 4cbbe1f..cc51416 100644 (file)
@@ -344,8 +344,8 @@ void clobberize(Graph& graph, Node* node, const ReadFunctor& read, const WriteFu
         def(PureValue(CheckNotEmpty, AdjacencyList(AdjacencyList::Fixed, node->child1())));
         return;
 
-    case CheckIdent:
-        def(PureValue(CheckIdent, AdjacencyList(AdjacencyList::Fixed, node->child1()), node->uidOperand()));
+    case CheckStringIdent:
+        def(PureValue(CheckStringIdent, AdjacencyList(AdjacencyList::Fixed, node->child1()), node->uidOperand()));
         return;
 
     case ConstantStoragePointer:
index 2d11307..5d6329d 100644 (file)
@@ -252,25 +252,20 @@ private:
                 break;
             }
 
-            case CheckIdent: {
+            case CheckStringIdent: {
                 UniquedStringImpl* uid = node->uidOperand();
                 const UniquedStringImpl* constantUid = nullptr;
 
                 JSValue childConstant = m_state.forNode(node->child1()).value();
                 if (childConstant) {
-                    if (uid->isSymbol()) {
-                        if (childConstant.isSymbol())
-                            constantUid = asSymbol(childConstant)->privateName().uid();
-                    } else {
-                        if (childConstant.isString()) {
-                            if (const auto* impl = asString(childConstant)->tryGetValueImpl()) {
-                                // Edge filtering requires that a value here should be StringIdent.
-                                // However, a constant value propagated in DFG is not filtered.
-                                // So here, we check the propagated value is actually an atomic string.
-                                // And if it's not, we just ignore.
-                                if (impl->isAtomic())
-                                    constantUid = static_cast<const UniquedStringImpl*>(impl);
-                            }
+                    if (childConstant.isString()) {
+                        if (const auto* impl = asString(childConstant)->tryGetValueImpl()) {
+                            // Edge filtering requires that a value here should be StringIdent.
+                            // However, a constant value propagated in DFG is not filtered.
+                            // So here, we check the propagated value is actually an atomic string.
+                            // And if it's not, we just ignore.
+                            if (impl->isAtomic())
+                                constantUid = static_cast<const UniquedStringImpl*>(impl);
                         }
                     }
                 }
index 4af7707..082c415 100644 (file)
@@ -129,7 +129,7 @@ bool doesGC(Graph& graph, Node* node)
     case PutGlobalVariable:
     case CheckCell:
     case CheckNotEmpty:
-    case CheckIdent:
+    case CheckStringIdent:
     case RegExpExec:
     case RegExpTest:
     case CompareLess:
index c61a31e..dee2169 100644 (file)
@@ -1214,12 +1214,8 @@ private:
             break;
         }
 
-        case CheckIdent: {
-            UniquedStringImpl* uid = node->uidOperand();
-            if (uid->isSymbol())
-                fixEdge<SymbolUse>(node->child1());
-            else
-                fixEdge<StringIdentUse>(node->child1());
+        case CheckStringIdent: {
+            fixEdge<StringIdentUse>(node->child1());
             break;
         }
             
index 5c48cf1..7371ccc 100644 (file)
@@ -1519,7 +1519,7 @@ public:
 
     bool hasUidOperand()
     {
-        return op() == CheckIdent;
+        return op() == CheckStringIdent;
     }
 
     UniquedStringImpl* uidOperand()
index 44668ce..55e9e7d 100644 (file)
@@ -239,7 +239,7 @@ namespace JSC { namespace DFG {
     macro(CheckNotEmpty, NodeMustGenerate) \
     macro(CheckBadCell, NodeMustGenerate) \
     macro(CheckInBounds, NodeMustGenerate) \
-    macro(CheckIdent, NodeMustGenerate) \
+    macro(CheckStringIdent, NodeMustGenerate) \
     macro(CheckTypeInfoFlags, NodeMustGenerate) /* Takes an OpInfo with the flags you want to test are set */\
     \
     /* Optimizations for array mutation. */\
index 5576e64..ef592ac 100644 (file)
@@ -1045,7 +1045,7 @@ private:
         case CheckStructure:
         case CheckCell:
         case CheckNotEmpty:
-        case CheckIdent:
+        case CheckStringIdent:
         case CheckBadCell:
         case PutStructure:
         case Phantom:
index c8bc603..566a459 100644 (file)
@@ -225,7 +225,7 @@ bool safeToExecute(AbstractStateType& state, Graph& graph, Node* node)
     case CheckCell:
     case CheckBadCell:
     case CheckNotEmpty:
-    case CheckIdent:
+    case CheckStringIdent:
     case RegExpExec:
     case RegExpTest:
     case CompareLess:
index 113a797..0a8991f 100644 (file)
@@ -5310,50 +5310,44 @@ void SpeculativeJIT::compileBooleanCompare(Node* node, MacroAssembler::Relationa
     unblessedBooleanResult(result.gpr(), node);
 }
 
-template<typename Functor>
-void SpeculativeJIT::extractStringImplFromBinarySymbols(Edge leftSymbolEdge, Edge rightSymbolEdge, const Functor& functor)
+void SpeculativeJIT::compileSymbolEquality(Node* node)
 {
-    SpeculateCellOperand left(this, leftSymbolEdge);
-    SpeculateCellOperand right(this, rightSymbolEdge);
-    GPRTemporary leftTemp(this);
-    GPRTemporary rightTemp(this);
+    SpeculateCellOperand left(this, node->child1());
+    SpeculateCellOperand right(this, node->child2());
+    GPRTemporary result(this, Reuse, left, right);
 
     GPRReg leftGPR = left.gpr();
     GPRReg rightGPR = right.gpr();
-    GPRReg leftTempGPR = leftTemp.gpr();
-    GPRReg rightTempGPR = rightTemp.gpr();
-
-    speculateSymbol(leftSymbolEdge, leftGPR);
-    speculateSymbol(rightSymbolEdge, rightGPR);
-
-    m_jit.loadPtr(JITCompiler::Address(leftGPR, Symbol::offsetOfPrivateName()), leftTempGPR);
-    m_jit.loadPtr(JITCompiler::Address(rightGPR, Symbol::offsetOfPrivateName()), rightTempGPR);
+    GPRReg resultGPR = result.gpr();
 
-    functor(leftTempGPR, rightTempGPR);
-}
+    speculateSymbol(node->child1(), leftGPR);
+    speculateSymbol(node->child2(), rightGPR);
 
-void SpeculativeJIT::compileSymbolEquality(Node* node)
-{
-    extractStringImplFromBinarySymbols(node->child1(), node->child2(), [&] (GPRReg leftStringImpl, GPRReg rightStringImpl) {
-        m_jit.comparePtr(JITCompiler::Equal, leftStringImpl, rightStringImpl, leftStringImpl);
-        unblessedBooleanResult(leftStringImpl, node);
-    });
+    m_jit.comparePtr(JITCompiler::Equal, leftGPR, rightGPR, resultGPR);
+    unblessedBooleanResult(resultGPR, node);
 }
 
 void SpeculativeJIT::compilePeepHoleSymbolEquality(Node* node, Node* branchNode)
 {
+    SpeculateCellOperand left(this, node->child1());
+    SpeculateCellOperand right(this, node->child2());
+
+    GPRReg leftGPR = left.gpr();
+    GPRReg rightGPR = right.gpr();
+
+    speculateSymbol(node->child1(), leftGPR);
+    speculateSymbol(node->child2(), rightGPR);
+
     BasicBlock* taken = branchNode->branchData()->taken.block;
     BasicBlock* notTaken = branchNode->branchData()->notTaken.block;
 
-    extractStringImplFromBinarySymbols(node->child1(), node->child2(), [&] (GPRReg leftStringImpl, GPRReg rightStringImpl) {
-        if (taken == nextBlock()) {
-            branchPtr(JITCompiler::NotEqual, leftStringImpl, rightStringImpl, notTaken);
-            jump(taken);
-        } else {
-            branchPtr(JITCompiler::Equal, leftStringImpl, rightStringImpl, taken);
-            jump(notTaken);
-        }
-    });
+    if (taken == nextBlock()) {
+        branchPtr(JITCompiler::NotEqual, leftGPR, rightGPR, notTaken);
+        jump(taken);
+    } else {
+        branchPtr(JITCompiler::Equal, leftGPR, rightGPR, taken);
+        jump(notTaken);
+    }
 }
 
 void SpeculativeJIT::compileStringEquality(
@@ -5997,28 +5991,21 @@ void SpeculativeJIT::compileGetArrayLength(Node* node)
     } }
 }
 
-void SpeculativeJIT::compileCheckIdent(Node* node)
+void SpeculativeJIT::compileCheckStringIdent(Node* node)
 {
-    SpeculateCellOperand operand(this, node->child1());
+    SpeculateCellOperand string(this, node->child1());
+    GPRTemporary storage(this);
+
+    GPRReg stringGPR = string.gpr();
+    GPRReg storageGPR = storage.gpr();
+
+    speculateString(node->child1(), stringGPR);
+    speculateStringIdentAndLoadStorage(node->child1(), stringGPR, storageGPR);
+
     UniquedStringImpl* uid = node->uidOperand();
-    if (uid->isSymbol()) {
-        speculateSymbol(node->child1(), operand.gpr());
-        speculationCheck(
-            BadIdent, JSValueSource(), nullptr,
-            m_jit.branchPtr(
-                JITCompiler::NotEqual,
-                JITCompiler::Address(operand.gpr(), Symbol::offsetOfPrivateName()),
-                TrustedImmPtr(uid)));
-    } else {
-        speculateString(node->child1(), operand.gpr());
-        speculateStringIdent(node->child1(), operand.gpr());
-        speculationCheck(
-            BadIdent, JSValueSource(), nullptr,
-            m_jit.branchPtr(
-                JITCompiler::NotEqual,
-                JITCompiler::Address(operand.gpr(), JSString::offsetOfValue()),
-                TrustedImmPtr(uid)));
-    }
+    speculationCheck(
+        BadIdent, JSValueSource(), nullptr,
+        m_jit.branchPtr(JITCompiler::NotEqual, storageGPR, TrustedImmPtr(uid)));
     noResult(node);
 }
 
@@ -8320,36 +8307,6 @@ void SpeculativeJIT::compileCompareEqPtr(Node* node)
     blessedBooleanResult(resultGPR, node);
 }
 
-void SpeculativeJIT::compileSymbolUntypedEquality(Node* node, Edge symbolEdge, Edge untypedEdge)
-{
-    SpeculateCellOperand symbol(this, symbolEdge);
-    JSValueOperand untyped(this, untypedEdge);
-    GPRTemporary leftTemp(this);
-    GPRTemporary rightTemp(this);
-    
-    GPRReg symbolGPR = symbol.gpr();
-    JSValueRegs untypedRegs = untyped.jsValueRegs();
-    GPRReg leftTempGPR = leftTemp.gpr();
-    GPRReg rightTempGPR = rightTemp.gpr();
-    
-    speculateSymbol(symbolEdge, symbolGPR);
-    
-    JITCompiler::Jump notCell = m_jit.branchIfNotCell(untypedRegs);
-    JITCompiler::Jump isSymbol = m_jit.branchIfSymbol(untypedRegs.payloadGPR());
-    
-    notCell.link(&m_jit);
-    m_jit.move(TrustedImm32(0), leftTempGPR);
-    JITCompiler::Jump done = m_jit.jump();
-    
-    isSymbol.link(&m_jit);
-    m_jit.loadPtr(JITCompiler::Address(symbolGPR, Symbol::offsetOfPrivateName()), leftTempGPR);
-    m_jit.loadPtr(JITCompiler::Address(untypedRegs.payloadGPR(), Symbol::offsetOfPrivateName()), rightTempGPR);
-    m_jit.comparePtr(JITCompiler::Equal, leftTempGPR, rightTempGPR, leftTempGPR);
-    
-    done.link(&m_jit);
-    unblessedBooleanResult(leftTempGPR, node);
-}
-
 } } // namespace JSC::DFG
 
 #endif
index 35984d8..5285451 100644 (file)
@@ -2348,8 +2348,6 @@ public:
     void compileStringZeroLength(Node*);
     void compileMiscStrictEq(Node*);
 
-    template<typename Functor>
-    void extractStringImplFromBinarySymbols(Edge leftSymbolEdge, Edge rightSymbolEdge, const Functor&);
     void compileSymbolEquality(Node*);
     void compilePeepHoleSymbolEquality(Node*, Node* branchNode);
     void compileSymbolUntypedEquality(Node*, Edge symbolEdge, Edge untypedEdge);
@@ -2436,7 +2434,7 @@ public:
     void compileGetArrayLength(Node*);
 
     void compileCheckTypeInfoFlags(Node*);
-    void compileCheckIdent(Node*);
+    void compileCheckStringIdent(Node*);
     
     void compileValueRep(Node*);
     void compileDoubleRep(Node*);
index d226725..bbf0055 100644 (file)
@@ -1525,6 +1525,33 @@ void SpeculativeJIT::compilePeepHoleObjectToObjectOrOtherEquality(Edge leftChild
     jump(notTaken);
 }
 
+void SpeculativeJIT::compileSymbolUntypedEquality(Node* node, Edge symbolEdge, Edge untypedEdge)
+{
+    SpeculateCellOperand symbol(this, symbolEdge);
+    JSValueOperand untyped(this, untypedEdge);
+
+    GPRReg symbolGPR = symbol.gpr();
+    GPRReg untypedGPR = untyped.payloadGPR();
+
+    speculateSymbol(symbolEdge, symbolGPR);
+
+    GPRTemporary resultPayload(this, Reuse, symbol);
+    GPRReg resultPayloadGPR = resultPayload.gpr();
+
+    MacroAssembler::Jump untypedCellJump = m_jit.branchIfCell(untyped.jsValueRegs());
+
+    m_jit.move(TrustedImm32(0), resultPayloadGPR);
+    MacroAssembler::Jump untypedNotCellJump = m_jit.jump();
+
+    // At this point we know that we can perform a straight-forward equality comparison on pointer
+    // values because we are doing strict equality.
+    untypedCellJump.link(&m_jit);
+    m_jit.compare32(MacroAssembler::Equal, symbolGPR, untypedGPR, resultPayloadGPR);
+
+    untypedNotCellJump.link(&m_jit);
+    booleanResult(resultPayloadGPR, node);
+}
+
 void SpeculativeJIT::compileInt32Compare(Node* node, MacroAssembler::RelationalCondition condition)
 {
     SpeculateInt32Operand op1(this, node->child1());
@@ -4332,8 +4359,8 @@ void SpeculativeJIT::compile(Node* node)
         break;
     }
 
-    case CheckIdent:
-        compileCheckIdent(node);
+    case CheckStringIdent:
+        compileCheckStringIdent(node);
         break;
 
     case GetExecutable: {
index b489956..5f93b1a 100644 (file)
@@ -1607,6 +1607,24 @@ void SpeculativeJIT::compilePeepHoleObjectToObjectOrOtherEquality(Edge leftChild
     jump(notTaken);
 }
 
+void SpeculativeJIT::compileSymbolUntypedEquality(Node* node, Edge symbolEdge, Edge untypedEdge)
+{
+    SpeculateCellOperand symbol(this, symbolEdge);
+    JSValueOperand untyped(this, untypedEdge);
+    GPRTemporary result(this, Reuse, symbol, untyped);
+
+    GPRReg symbolGPR = symbol.gpr();
+    GPRReg untypedGPR = untyped.gpr();
+    GPRReg resultGPR = result.gpr();
+
+    speculateSymbol(symbolEdge, symbolGPR);
+
+    // At this point we know that we can perform a straight-forward equality comparison on pointer
+    // values because we are doing strict equality.
+    m_jit.compare64(MacroAssembler::Equal, symbolGPR, untypedGPR, resultGPR);
+    unblessedBooleanResult(resultGPR, node);
+}
+
 void SpeculativeJIT::compileInt32Compare(Node* node, MacroAssembler::RelationalCondition condition)
 {
     if (node->child1()->isInt32Constant()) {
@@ -4281,8 +4299,8 @@ void SpeculativeJIT::compile(Node* node)
         break;
     }
 
-    case CheckIdent:
-        compileCheckIdent(node);
+    case CheckStringIdent:
+        compileCheckStringIdent(node);
         break;
 
     case GetExecutable: {
index 212580f..cff05a8 100644 (file)
@@ -102,8 +102,7 @@ namespace JSC { namespace FTL {
     macro(Structure_classInfo, Structure::classInfoOffset()) \
     macro(Structure_globalObject, Structure::globalObjectOffset()) \
     macro(Structure_prototype, Structure::prototypeOffset()) \
-    macro(Structure_structureID, Structure::structureIDOffset()) \
-    macro(Symbol_privateName, Symbol::offsetOfPrivateName())
+    macro(Structure_structureID, Structure::structureIDOffset())
 
 #define FOR_EACH_INDEXED_ABSTRACT_HEAP(macro) \
     macro(DirectArguments_storage, DirectArguments::storageOffset(), sizeof(EncodedJSValue)) \
index 3e392e9..afc5272 100644 (file)
@@ -128,7 +128,7 @@ inline CapabilityLevel canCompile(Node* node)
     case CheckCell:
     case CheckBadCell:
     case CheckNotEmpty:
-    case CheckIdent:
+    case CheckStringIdent:
     case CheckWatchdogTimer:
     case StringCharCodeAt:
     case StringFromCharCode:
index 53c505c..2a24199 100644 (file)
@@ -602,8 +602,8 @@ private:
         case CheckBadCell:
             compileCheckBadCell();
             break;
-        case CheckIdent:
-            compileCheckIdent();
+        case CheckStringIdent:
+            compileCheckStringIdent();
             break;
         case GetExecutable:
             compileGetExecutable();
@@ -2477,17 +2477,12 @@ private:
         speculate(TDZFailure, noValue(), nullptr, m_out.isZero64(lowJSValue(m_node->child1())));
     }
 
-    void compileCheckIdent()
+    void compileCheckStringIdent()
     {
         UniquedStringImpl* uid = m_node->uidOperand();
-        if (uid->isSymbol()) {
-            LValue stringImpl = lowSymbolUID(m_node->child1());
-            speculate(BadIdent, noValue(), nullptr, m_out.notEqual(stringImpl, m_out.constIntPtr(uid)));
-        } else {
-            LValue string = lowStringIdent(m_node->child1());
-            LValue stringImpl = m_out.loadPtr(string, m_heaps.JSString_value);
-            speculate(BadIdent, noValue(), nullptr, m_out.notEqual(stringImpl, m_out.constIntPtr(uid)));
-        }
+        LValue string = lowStringIdent(m_node->child1());
+        LValue stringImpl = m_out.loadPtr(string, m_heaps.JSString_value);
+        speculate(BadIdent, noValue(), nullptr, m_out.notEqual(stringImpl, m_out.constIntPtr(uid)));
     }
 
     void compileGetExecutable()
@@ -5039,9 +5034,9 @@ private:
         }
 
         if (m_node->isBinaryUseKind(SymbolUse)) {
-            LValue leftStringImpl = lowSymbolUID(m_node->child1());
-            LValue rightStringImpl = lowSymbolUID(m_node->child2());
-            setBoolean(m_out.equal(leftStringImpl, rightStringImpl));
+            LValue leftSymbol = lowSymbol(m_node->child1());
+            LValue rightSymbol = lowSymbol(m_node->child2());
+            setBoolean(m_out.equal(leftSymbol, rightSymbol));
             return;
         }
         
@@ -5052,33 +5047,10 @@ private:
             if (symbolEdge.useKind() != SymbolUse)
                 std::swap(symbolEdge, untypedEdge);
             
-            LValue leftStringImpl = lowSymbolUID(symbolEdge);
+            LValue leftSymbol = lowSymbol(symbolEdge);
             LValue untypedValue = lowJSValue(untypedEdge);
-            
-            LBasicBlock isCellCase = m_out.newBlock();
-            LBasicBlock isSymbolCase = m_out.newBlock();
-            LBasicBlock continuation = m_out.newBlock();
-            
-            ValueFromBlock notSymbolResult = m_out.anchor(m_out.booleanFalse);
-            m_out.branch(
-                isCell(untypedValue, provenType(untypedEdge)),
-                unsure(isCellCase), unsure(continuation));
-            
-            LBasicBlock lastNext = m_out.appendTo(isCellCase, isSymbolCase);
-            m_out.branch(
-                isSymbol(untypedValue, provenType(untypedEdge)),
-                unsure(isSymbolCase), unsure(continuation));
-            
-            m_out.appendTo(isSymbolCase, continuation);
-            ValueFromBlock symbolResult =
-                m_out.anchor(
-                    m_out.equal(
-                        m_out.loadPtr(untypedValue, m_heaps.Symbol_privateName),
-                        leftStringImpl));
-            m_out.jump(continuation);
-            
-            m_out.appendTo(continuation, lastNext);
-            setBoolean(m_out.phi(Int32, notSymbolResult, symbolResult));
+
+            setBoolean(m_out.equal(leftSymbol, untypedValue));
             return;
         }
         
@@ -9872,16 +9844,6 @@ private:
         return result;
     }
     
-    LValue lowSymbolUID(Edge edge, OperandSpeculationMode mode = AutomaticOperandSpeculation)
-    {
-        if (Symbol* symbol = edge->dynamicCastConstant<Symbol*>())
-            return m_out.constIntPtr(symbol->privateName().uid());
-        LValue symbol = lowSymbol(edge, mode);
-        // FIXME: We could avoid this load if we had hash-consed Symbols.
-        // https://bugs.webkit.org/show_bug.cgi?id=158908
-        return m_out.loadPtr(symbol, m_heaps.Symbol_privateName);
-    }
-
     LValue lowNonNullObject(Edge edge, OperandSpeculationMode mode = AutomaticOperandSpeculation)
     {
         ASSERT_UNUSED(mode, mode == ManualOperandSpeculation || edge.useKind() == ObjectUse);
index 5b1abb8..5c36066 100644 (file)
@@ -395,10 +395,10 @@ namespace JSC {
         JumpList emitFloatTypedArrayPutByVal(Instruction*, PatchableJump& badType, TypedArrayType);
 
         // Identifier check helper for GetByVal and PutByVal.
-        void emitIdentifierCheck(RegisterID cell, RegisterID scratch, const Identifier&, JumpList& slowCases);
+        void emitByValIdentifierCheck(ByValInfo*, RegisterID cell, RegisterID scratch, const Identifier&, JumpList& slowCases);
 
-        JITGetByIdGenerator emitGetByValWithCachedId(Instruction*, const Identifier&, Jump& fastDoneCase, Jump& slowDoneCase, JumpList& slowCases);
-        JITPutByIdGenerator emitPutByValWithCachedId(Instruction*, PutKind, const Identifier&, JumpList& doneCases, JumpList& slowCases);
+        JITGetByIdGenerator emitGetByValWithCachedId(ByValInfo*, Instruction*, const Identifier&, Jump& fastDoneCase, Jump& slowDoneCase, JumpList& slowCases);
+        JITPutByIdGenerator emitPutByValWithCachedId(ByValInfo*, Instruction*, PutKind, const Identifier&, JumpList& doneCases, JumpList& slowCases);
 
         enum FinalObjectMode { MayBeFinal, KnownNotFinal };
 
index ed20c91..9b4c1f9 100644 (file)
@@ -612,7 +612,7 @@ static OptimizationResult tryPutByValOptimize(ExecState* exec, JSValue baseValue
 
     if (baseValue.isObject() && isStringOrSymbol(subscript)) {
         const Identifier propertyName = subscript.toPropertyKey(exec);
-        if (!subscript.isString() || !parseIndex(propertyName)) {
+        if (subscript.isSymbol() || !parseIndex(propertyName)) {
             ASSERT(exec->bytecodeOffset());
             ASSERT(!byValInfo->stubRoutine);
             if (byValInfo->seen) {
@@ -624,8 +624,12 @@ static OptimizationResult tryPutByValOptimize(ExecState* exec, JSValue baseValue
                     optimizationResult = OptimizationResult::GiveUp;
                 }
             } else {
+                CodeBlock* codeBlock = exec->codeBlock();
+                ConcurrentJITLocker locker(codeBlock->m_lock);
                 byValInfo->seen = true;
                 byValInfo->cachedId = propertyName;
+                if (subscript.isSymbol())
+                    byValInfo->cachedSymbol.set(vm, codeBlock, asSymbol(subscript));
                 optimizationResult = OptimizationResult::SeenOnce;
             }
         }
@@ -690,9 +694,7 @@ static OptimizationResult tryDirectPutByValOptimize(ExecState* exec, JSObject* o
             optimizationResult = OptimizationResult::GiveUp;
     } else if (isStringOrSymbol(subscript)) {
         const Identifier propertyName = subscript.toPropertyKey(exec);
-        Optional<uint32_t> index = parseIndex(propertyName);
-
-        if (!subscript.isString() || !index) {
+        if (subscript.isSymbol() || !parseIndex(propertyName)) {
             ASSERT(exec->bytecodeOffset());
             ASSERT(!byValInfo->stubRoutine);
             if (byValInfo->seen) {
@@ -704,8 +706,12 @@ static OptimizationResult tryDirectPutByValOptimize(ExecState* exec, JSObject* o
                     optimizationResult = OptimizationResult::GiveUp;
                 }
             } else {
+                CodeBlock* codeBlock = exec->codeBlock();
+                ConcurrentJITLocker locker(codeBlock->m_lock);
                 byValInfo->seen = true;
                 byValInfo->cachedId = propertyName;
+                if (subscript.isSymbol())
+                    byValInfo->cachedSymbol.set(vm, codeBlock, asSymbol(subscript));
                 optimizationResult = OptimizationResult::SeenOnce;
             }
         }
@@ -1686,7 +1692,7 @@ static OptimizationResult tryGetByValOptimize(ExecState* exec, JSValue baseValue
 
     if (baseValue.isObject() && isStringOrSymbol(subscript)) {
         const Identifier propertyName = subscript.toPropertyKey(exec);
-        if (!subscript.isString() || !parseIndex(propertyName)) {
+        if (subscript.isSymbol() || !parseIndex(propertyName)) {
             ASSERT(exec->bytecodeOffset());
             ASSERT(!byValInfo->stubRoutine);
             if (byValInfo->seen) {
@@ -1698,11 +1704,14 @@ static OptimizationResult tryGetByValOptimize(ExecState* exec, JSValue baseValue
                     optimizationResult = OptimizationResult::GiveUp;
                 }
             } else {
+                CodeBlock* codeBlock = exec->codeBlock();
+                ConcurrentJITLocker locker(codeBlock->m_lock);
                 byValInfo->seen = true;
                 byValInfo->cachedId = propertyName;
+                if (subscript.isSymbol())
+                    byValInfo->cachedSymbol.set(vm, codeBlock, asSymbol(subscript));
                 optimizationResult = OptimizationResult::SeenOnce;
             }
-
         }
     }
 
index 9d1344e..14fb940 100644 (file)
@@ -209,7 +209,7 @@ JIT::JumpList JIT::emitArrayStorageLoad(Instruction*, PatchableJump& badType)
     return slowCases;
 }
 
-JITGetByIdGenerator JIT::emitGetByValWithCachedId(Instruction* currentInstruction, const Identifier& propertyName, Jump& fastDoneCase, Jump& slowDoneCase, JumpList& slowCases)
+JITGetByIdGenerator JIT::emitGetByValWithCachedId(ByValInfo* byValInfo, Instruction* currentInstruction, const Identifier& propertyName, Jump& fastDoneCase, Jump& slowDoneCase, JumpList& slowCases)
 {
     // base: regT0
     // property: regT1
@@ -218,7 +218,7 @@ JITGetByIdGenerator JIT::emitGetByValWithCachedId(Instruction* currentInstructio
     int dst = currentInstruction[1].u.operand;
 
     slowCases.append(emitJumpIfNotJSCell(regT1));
-    emitIdentifierCheck(regT1, regT3, propertyName, slowCases);
+    emitByValIdentifierCheck(byValInfo, regT1, regT3, propertyName, slowCases);
 
     JITGetByIdGenerator gen(
         m_codeBlock, CodeOrigin(m_bytecodeOffset), CallSiteIndex(m_bytecodeOffset), RegisterSet::stubUnavailableRegisters(),
@@ -428,7 +428,7 @@ JIT::JumpList JIT::emitArrayStoragePutByVal(Instruction* currentInstruction, Pat
     return slowCases;
 }
 
-JITPutByIdGenerator JIT::emitPutByValWithCachedId(Instruction* currentInstruction, PutKind putKind, const Identifier& propertyName, JumpList& doneCases, JumpList& slowCases)
+JITPutByIdGenerator JIT::emitPutByValWithCachedId(ByValInfo* byValInfo, Instruction* currentInstruction, PutKind putKind, const Identifier& propertyName, JumpList& doneCases, JumpList& slowCases)
 {
     // base: regT0
     // property: regT1
@@ -438,7 +438,7 @@ JITPutByIdGenerator JIT::emitPutByValWithCachedId(Instruction* currentInstructio
     int value = currentInstruction[3].u.operand;
 
     slowCases.append(emitJumpIfNotJSCell(regT1));
-    emitIdentifierCheck(regT1, regT1, propertyName, slowCases);
+    emitByValIdentifierCheck(byValInfo, regT1, regT1, propertyName, slowCases);
 
     // Write barrier breaks the registers. So after issuing the write barrier,
     // reload the registers.
@@ -1253,16 +1253,15 @@ void JIT::emitWriteBarrier(JSCell* owner)
         callOperation(operationUnconditionalWriteBarrier, owner);
 }
 
-void JIT::emitIdentifierCheck(RegisterID cell, RegisterID scratch, const Identifier& propertyName, JumpList& slowCases)
+void JIT::emitByValIdentifierCheck(ByValInfo* byValInfo, RegisterID cell, RegisterID scratch, const Identifier& propertyName, JumpList& slowCases)
 {
-    if (propertyName.isSymbol()) {
-        slowCases.append(branchStructure(NotEqual, Address(cell, JSCell::structureIDOffset()), m_vm->symbolStructure.get()));
-        loadPtr(Address(cell, Symbol::offsetOfPrivateName()), scratch);
-    } else {
+    if (propertyName.isSymbol())
+        slowCases.append(branchPtr(NotEqual, cell, TrustedImmPtr(byValInfo->cachedSymbol.get())));
+    else {
         slowCases.append(branchStructure(NotEqual, Address(cell, JSCell::structureIDOffset()), m_vm->stringStructure.get()));
         loadPtr(Address(cell, JSString::offsetOfValue()), scratch);
+        slowCases.append(branchPtr(NotEqual, scratch, TrustedImmPtr(propertyName.impl())));
     }
-    slowCases.append(branchPtr(NotEqual, scratch, TrustedImmPtr(propertyName.impl())));
 }
 
 void JIT::privateCompileGetByVal(ByValInfo* byValInfo, ReturnAddressPtr returnAddress, JITArrayMode arrayMode)
@@ -1325,7 +1324,7 @@ void JIT::privateCompileGetByValWithCachedId(ByValInfo* byValInfo, ReturnAddress
     Jump slowDoneCase;
     JumpList slowCases;
 
-    JITGetByIdGenerator gen = emitGetByValWithCachedId(currentInstruction, propertyName, fastDoneCase, slowDoneCase, slowCases);
+    JITGetByIdGenerator gen = emitGetByValWithCachedId(byValInfo, currentInstruction, propertyName, fastDoneCase, slowDoneCase, slowCases);
 
     ConcurrentJITLocker locker(m_codeBlock->m_lock);
     LinkBuffer patchBuffer(*m_vm, *this, m_codeBlock);
@@ -1414,7 +1413,7 @@ void JIT::privateCompilePutByValWithCachedId(ByValInfo* byValInfo, ReturnAddress
     JumpList doneCases;
     JumpList slowCases;
 
-    JITPutByIdGenerator gen = emitPutByValWithCachedId(currentInstruction, putKind, propertyName, doneCases, slowCases);
+    JITPutByIdGenerator gen = emitPutByValWithCachedId(byValInfo, currentInstruction, putKind, propertyName, doneCases, slowCases);
 
     ConcurrentJITLocker locker(m_codeBlock->m_lock);
     LinkBuffer patchBuffer(*m_vm, *this, m_codeBlock);
index 1d65624..2ce50a2 100644 (file)
@@ -279,7 +279,7 @@ JIT::JumpList JIT::emitArrayStorageLoad(Instruction*, PatchableJump& badType)
     return slowCases;
 }
 
-JITGetByIdGenerator JIT::emitGetByValWithCachedId(Instruction* currentInstruction, const Identifier& propertyName, Jump& fastDoneCase, Jump& slowDoneCase, JumpList& slowCases)
+JITGetByIdGenerator JIT::emitGetByValWithCachedId(ByValInfo* byValInfo, Instruction* currentInstruction, const Identifier& propertyName, Jump& fastDoneCase, Jump& slowDoneCase, JumpList& slowCases)
 {
     int dst = currentInstruction[1].u.operand;
 
@@ -288,7 +288,7 @@ JITGetByIdGenerator JIT::emitGetByValWithCachedId(Instruction* currentInstructio
     // scratch: regT4
 
     slowCases.append(branch32(NotEqual, regT3, TrustedImm32(JSValue::CellTag)));
-    emitIdentifierCheck(regT2, regT4, propertyName, slowCases);
+    emitByValIdentifierCheck(byValInfo, regT2, regT4, propertyName, slowCases);
 
     JITGetByIdGenerator gen(
         m_codeBlock, CodeOrigin(m_bytecodeOffset), CallSiteIndex(currentInstruction), RegisterSet::stubUnavailableRegisters(),
@@ -485,7 +485,7 @@ JIT::JumpList JIT::emitArrayStoragePutByVal(Instruction* currentInstruction, Pat
     return slowCases;
 }
 
-JITPutByIdGenerator JIT::emitPutByValWithCachedId(Instruction* currentInstruction, PutKind putKind, const Identifier& propertyName, JumpList& doneCases, JumpList& slowCases)
+JITPutByIdGenerator JIT::emitPutByValWithCachedId(ByValInfo* byValInfo, Instruction* currentInstruction, PutKind putKind, const Identifier& propertyName, JumpList& doneCases, JumpList& slowCases)
 {
     // base: tag(regT1), payload(regT0)
     // property: tag(regT3), payload(regT2)
@@ -494,7 +494,7 @@ JITPutByIdGenerator JIT::emitPutByValWithCachedId(Instruction* currentInstructio
     int value = currentInstruction[3].u.operand;
 
     slowCases.append(branch32(NotEqual, regT3, TrustedImm32(JSValue::CellTag)));
-    emitIdentifierCheck(regT2, regT2, propertyName, slowCases);
+    emitByValIdentifierCheck(byValInfo, regT2, regT2, propertyName, slowCases);
 
     // Write barrier breaks the registers. So after issuing the write barrier,
     // reload the registers.
index b8dd22a..d766e42 100644 (file)
@@ -252,7 +252,9 @@ void JSValue::dumpInContextAssumingStructure(
             } else
                 out.print(" (unresolved)");
             out.print(": ", impl);
-        } else if (structure->classInfo()->isSubClassOf(Structure::info()))
+        } else if (structure->classInfo()->isSubClassOf(Symbol::info()))
+            out.print("Symbol: ", RawPointer(asCell()));
+        else if (structure->classInfo()->isSubClassOf(Structure::info()))
             out.print("Structure: ", inContext(*jsCast<Structure*>(asCell()), context));
         else {
             out.print("Cell: ", RawPointer(asCell()));
index 802eb27..34f3eda 100644 (file)
@@ -941,7 +941,7 @@ ALWAYS_INLINE bool JSValue::equalSlowCaseInline(ExecState* exec, JSValue v1, JSV
         bool sym2 = v2.isSymbol();
         if (sym1 || sym2) {
             if (sym1 && sym2)
-                return asSymbol(v1)->privateName() == asSymbol(v2)->privateName();
+                return asSymbol(v1) == asSymbol(v2);
             return false;
         }
 
@@ -970,9 +970,6 @@ ALWAYS_INLINE bool JSValue::strictEqualSlowCaseInline(ExecState* exec, JSValue v
 
     if (v1.asCell()->isString() && v2.asCell()->isString())
         return WTF::equal(*asString(v1)->value(exec).impl(), *asString(v2)->value(exec).impl());
-    if (v1.asCell()->isSymbol() && v2.asCell()->isSymbol())
-        return asSymbol(v1)->privateName() == asSymbol(v2)->privateName();
-
     return v1 == v2;
 }
 
index 58bf23a..eae79c5 100644 (file)
@@ -575,7 +575,7 @@ void JSFunction::setFunctionName(ExecState* exec, JSValue value)
         if (uid->isNullSymbol())
             name = emptyString();
         else
-            name = makeString("[", String(asSymbol(value)->privateName().uid()), ']');
+            name = makeString('[', String(uid), ']');
     } else {
         VM& vm = exec->vm();
         JSString* jsStr = value.toString(exec);
index 74b2f98..a207058 100644 (file)
@@ -119,7 +119,6 @@ private:
     typedef HashMap<JSCell*, int32_t, typename WTF::DefaultHash<JSCell*>::Hash, WTF::HashTraits<JSCell*>, IndexTraits> CellKeyedMap;
     typedef HashMap<EncodedJSValue, int32_t, EncodedJSValueHash, EncodedJSValueHashTraits, IndexTraits> ValueKeyedMap;
     typedef HashMap<StringImpl*, int32_t, typename WTF::DefaultHash<StringImpl*>::Hash, WTF::HashTraits<StringImpl*>, IndexTraits> StringKeyedMap;
-    typedef HashMap<SymbolImpl*, int32_t, typename WTF::PtrHash<SymbolImpl*>, WTF::HashTraits<SymbolImpl*>, IndexTraits> SymbolKeyedMap;
 
     ALWAYS_INLINE Entry* find(ExecState*, KeyType);
     ALWAYS_INLINE Entry* add(ExecState*, JSCell* owner, KeyType);
@@ -134,7 +133,6 @@ private:
     CellKeyedMap m_cellKeyedTable;
     ValueKeyedMap m_valueKeyedTable;
     StringKeyedMap m_stringKeyedTable;
-    SymbolKeyedMap m_symbolKeyedTable;
     int32_t m_capacity;
     int32_t m_size;
     int32_t m_deletedCount;
index 38f5ffe..54f9ae1 100644 (file)
@@ -43,7 +43,6 @@ inline void MapDataImpl<Entry, JSIterator>::clear()
     m_cellKeyedTable.clear();
     m_valueKeyedTable.clear();
     m_stringKeyedTable.clear();
-    m_symbolKeyedTable.clear();
     m_capacity = 0;
     m_size = 0;
     m_deletedCount = 0;
@@ -62,12 +61,6 @@ inline Entry* MapDataImpl<Entry, JSIterator>::find(ExecState* exec, KeyType key)
             return 0;
         return &m_entries.get()[iter->value];
     }
-    if (key.value.isSymbol()) {
-        auto iter = m_symbolKeyedTable.find(asSymbol(key.value)->privateName().uid());
-        if (iter == m_symbolKeyedTable.end())
-            return 0;
-        return &m_entries.get()[iter->value];
-    }
     if (key.value.isCell()) {
         auto iter = m_cellKeyedTable.find(key.value.asCell());
         if (iter == m_cellKeyedTable.end())
@@ -120,8 +113,6 @@ inline Entry* MapDataImpl<Entry, JSIterator>::add(ExecState* exec, JSCell* owner
 {
     if (key.value.isString())
         return add(exec, owner, m_stringKeyedTable, asString(key.value)->value(exec).impl(), key);
-    if (key.value.isSymbol())
-        return add(exec, owner, m_symbolKeyedTable, asSymbol(key.value)->privateName().uid(), key);
     if (key.value.isCell())
         return add(exec, owner, m_cellKeyedTable, key.value.asCell(), key);
     return add(exec, owner, m_valueKeyedTable, JSValue::encode(key.value), key);
@@ -145,12 +136,6 @@ inline bool MapDataImpl<Entry, JSIterator>::remove(ExecState* exec, KeyType key)
             return false;
         location = iter->value;
         m_stringKeyedTable.remove(iter);
-    }  else if (key.value.isSymbol()) {
-        auto iter = m_symbolKeyedTable.find(asSymbol(key.value)->privateName().uid());
-        if (iter == m_symbolKeyedTable.end())
-            return false;
-        location = iter->value;
-        m_symbolKeyedTable.remove(iter);
     } else if (key.value.isCell()) {
         auto iter = m_cellKeyedTable.find(key.value.asCell());
         if (iter == m_cellKeyedTable.end())
@@ -200,8 +185,6 @@ inline void MapDataImpl<Entry, JSIterator>::replaceAndPackBackingStore(Entry* de
         ptr->value = m_entries.get()[ptr->value].key().get().asInt32();
     for (auto ptr = m_stringKeyedTable.begin(); ptr != m_stringKeyedTable.end(); ++ptr)
         ptr->value = m_entries.get()[ptr->value].key().get().asInt32();
-    for (auto ptr = m_symbolKeyedTable.begin(); ptr != m_symbolKeyedTable.end(); ++ptr)
-        ptr->value = m_entries.get()[ptr->value].key().get().asInt32();
 
     ASSERT((m_size - newEnd) == m_deletedCount);
     m_deletedCount = 0;
index 6ac790a..4c45823 100644 (file)
@@ -1,6 +1,6 @@
 /*
  * Copyright (C) 2012 Apple Inc. All rights reserved.
- * Copyright (C) 2015 Yusuke Suzuki <utatane.tea@gmail.com>.
+ * Copyright (C) 2015-2016 Yusuke Suzuki <utatane.tea@gmail.com>.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -53,6 +53,14 @@ Symbol::Symbol(VM& vm, SymbolImpl& uid)
 {
 }
 
+void Symbol::finishCreation(VM& vm)
+{
+    Base::finishCreation(vm);
+    ASSERT(inherits(info()));
+
+    vm.symbolImplToSymbolMap.set(m_privateName.uid(), this);
+}
+
 inline SymbolObject* SymbolObject::create(VM& vm, JSGlobalObject* globalObject, Symbol* symbol)
 {
     SymbolObject* object = new (NotNull, allocateCell<SymbolObject>(vm.heap)) SymbolObject(vm, globalObject->symbolObjectStructure());
@@ -93,4 +101,30 @@ String Symbol::descriptiveString() const
     return makeString("Symbol(", String(privateName().uid()), ')');
 }
 
+Symbol* Symbol::create(VM& vm)
+{
+    Symbol* symbol = new (NotNull, allocateCell<Symbol>(vm.heap)) Symbol(vm);
+    symbol->finishCreation(vm);
+    return symbol;
+}
+
+Symbol* Symbol::create(ExecState* exec, JSString* description)
+{
+    VM& vm = exec->vm();
+    String desc = description->value(exec);
+    Symbol* symbol = new (NotNull, allocateCell<Symbol>(vm.heap)) Symbol(vm, desc);
+    symbol->finishCreation(vm);
+    return symbol;
+}
+
+Symbol* Symbol::create(VM& vm, SymbolImpl& uid)
+{
+    if (Symbol* symbol = vm.symbolImplToSymbolMap.get(&uid))
+        return symbol;
+
+    Symbol* symbol = new (NotNull, allocateCell<Symbol>(vm.heap)) Symbol(vm, uid);
+    symbol->finishCreation(vm);
+    return symbol;
+}
+
 } // namespace JSC
index acaa702..933a2e5 100644 (file)
@@ -1,7 +1,7 @@
 /*
  * Copyright (C) 2012 Apple Inc. All rights reserved.
  * Copyright (C) 2014 Apple Inc. All rights reserved.
- * Copyright (C) 2015 Yusuke Suzuki <utatane.tea@gmail.com>.
+ * Copyright (C) 2015-2016 Yusuke Suzuki <utatane.tea@gmail.com>.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
@@ -28,7 +28,6 @@
 #ifndef Symbol_h
 #define Symbol_h
 
-#include "JSCell.h"
 #include "JSString.h"
 #include "PrivateName.h"
 
@@ -48,28 +47,9 @@ public:
         return Structure::create(vm, globalObject, prototype, TypeInfo(SymbolType, StructureFlags), info());
     }
 
-    static Symbol* create(VM& vm)
-    {
-        Symbol* symbol = new (NotNull, allocateCell<Symbol>(vm.heap)) Symbol(vm);
-        symbol->finishCreation(vm);
-        return symbol;
-    }
-
-    static Symbol* create(ExecState* exec, JSString* description)
-    {
-        VM& vm = exec->vm();
-        String desc = description->value(exec);
-        Symbol* symbol = new (NotNull, allocateCell<Symbol>(vm.heap)) Symbol(vm, desc);
-        symbol->finishCreation(vm);
-        return symbol;
-    }
-
-    static Symbol* create(VM& vm, SymbolImpl& uid)
-    {
-        Symbol* symbol = new (NotNull, allocateCell<Symbol>(vm.heap)) Symbol(vm, uid);
-        symbol->finishCreation(vm);
-        return symbol;
-    }
+    static Symbol* create(VM&);
+    static Symbol* create(ExecState*, JSString* description);
+    static Symbol* create(VM&, SymbolImpl& uid);
 
     const PrivateName& privateName() const { return m_privateName; }
     String descriptiveString() const;
@@ -79,8 +59,6 @@ public:
     JSObject* toObject(ExecState*, JSGlobalObject*) const;
     double toNumber(ExecState*) const;
 
-    static size_t offsetOfPrivateName() { return OBJECT_OFFSETOF(Symbol, m_privateName); }
-
 protected:
     static void destroy(JSCell*);
 
@@ -88,11 +66,7 @@ protected:
     Symbol(VM&, const String&);
     Symbol(VM&, SymbolImpl& uid);
 
-    void finishCreation(VM& vm)
-    {
-        Base::finishCreation(vm);
-        ASSERT(inherits(info()));
-    }
+    void finishCreation(VM&);
 
     PrivateName m_privateName;
 };
index 9416ea9..cd538d8 100644 (file)
@@ -165,6 +165,7 @@ VM::VM(VMType vmType, HeapType heapType)
     , emptyList(new MarkedArgumentBuffer)
     , customGetterSetterFunctionMap(*this)
     , stringCache(*this)
+    , symbolImplToSymbolMap(*this)
     , prototypeMap(*this)
     , interpreter(0)
     , jsArrayClassInfo(JSArray::info())
index 4e9f21c..069a009 100644 (file)
@@ -108,6 +108,7 @@ class Structure;
 #if ENABLE(REGEXP_TRACING)
 class RegExp;
 #endif
+class Symbol;
 class UnlinkedCodeBlock;
 class UnlinkedEvalCodeBlock;
 class UnlinkedFunctionExecutable;
@@ -336,6 +337,8 @@ public:
     AtomicStringTable* atomicStringTable() const { return m_atomicStringTable; }
     WTF::SymbolRegistry& symbolRegistry() { return m_symbolRegistry; }
 
+    WeakGCMap<SymbolImpl*, Symbol, PtrHash<SymbolImpl*>> symbolImplToSymbolMap;
+
     enum class DeletePropertyMode {
         // Default behaviour of deleteProperty, matching the spec.
         Default,
diff --git a/Source/JavaScriptCore/tests/stress/symbol-equality-over-gc.js b/Source/JavaScriptCore/tests/stress/symbol-equality-over-gc.js
new file mode 100644 (file)
index 0000000..180e2fb
--- /dev/null
@@ -0,0 +1,23 @@
+function shouldBe(actual, expected)
+{
+    if (actual !== expected)
+        throw new Error('bad value: ' + actual);
+}
+
+function test()
+{
+    let symbol = Symbol();
+    let object1 = {
+        [symbol]: 42
+    }
+    let object2 = {
+        [symbol]: 42
+    }
+    symbol = null;
+    fullGC();
+    shouldBe(Object.getOwnPropertySymbols(object1)[0], Object.getOwnPropertySymbols(object2)[0]);
+}
+noInline(test);
+
+for (let i = 0; i < 1000; ++i)
+    test();