Attachment 361284 Details for Bug 194277 – Patch

[patch] Patch

bug-194277-20190206020044.patch (text/plain), 51.48 KB, created by Yusuke Suzuki on 2019-02-06 02:00:45 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2019-02-06 02:00:45 PST

Size: 51.48 KB

patch

obsolete

>Subversion Revision: 241001
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index fb8389d38d980e86d6792f3b8103ca902ad6f5e2..95528c2034fd49cfe440a85de9f396671ff114df 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,74 @@
>+2019-02-06  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [JSC] PrivateName to PublicName hash table is wasteful
>+        https://bugs.webkit.org/show_bug.cgi?id=194277
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        PrivateNames take much memory in intial JSC footprint. Since BuiltinNames have Identifier fields corresponding to these PrivateNames,
>+        sizeof(BuiltinNames) is about 6KB. It also maintains hash tables for "PublicName to PrivateName" and "PrivateName to PublicName",
>+        each of which takes 16KB memory. While "PublicName to PrivateName" functionality is used in builtin JS (parsing "@xxx" and get a private
>+        name for "xxx"), "PrivateName to PublicName" is rarely used. Holding 16KB hash table for rarely used feature is costly.
>+
>+        In this patch, we add some rules to remove "PrivateName to PublicName" hash table.
>+
>+        1. PrivateName's content should be the same to PublicName.
>+        2. If PrivateName is not actually a private name (we introduced hacky mapping like "@iteratorSymbol" => Symbol.iterator),
>+           the public name should be easily crafted from the given PrivateName.
>+
>+        We modify the content of private names to ensure (1). And for (2), we can meet this requirement by ensuring that the "@xxxSymbol"
>+        is converted to "Symbol.xxx". (1) and (2) allow us to convert a private name to a public name without a large hash table.
>+
>+        We also remove unused identifiers in CommonIdentifiers. And we also move some of them to WebCore's WebCoreBuiltinNames if it is only used in
>+        WebCore.
>+
>+        * builtins/BuiltinNames.cpp:
>+        (JSC::BuiltinNames::BuiltinNames):
>+        * builtins/BuiltinNames.h:
>+        (JSC::BuiltinNames::lookUpPrivateName const):
>+        (JSC::BuiltinNames::getPublicName const):
>+        (JSC::BuiltinNames::checkPublicToPrivateMapConsistency):
>+        (JSC::BuiltinNames::appendExternalName):
>+        (JSC::BuiltinNames::lookUpPublicName const): Deleted.
>+        * builtins/BuiltinUtils.h:
>+        * bytecode/BytecodeDumper.cpp:
>+        (JSC::BytecodeDumper<Block>::dumpIdentifiers):
>+        * bytecompiler/NodesCodegen.cpp:
>+        (JSC::BytecodeIntrinsicNode::emit_intrinsic_getByIdDirectPrivate):
>+        (JSC::BytecodeIntrinsicNode::emit_intrinsic_putByIdDirectPrivate):
>+        * parser/Lexer.cpp:
>+        (JSC::Lexer<LChar>::parseIdentifier):
>+        (JSC::Lexer<UChar>::parseIdentifier):
>+        * parser/Parser.cpp:
>+        (JSC::Parser<LexerType>::createGeneratorParameters):
>+        (JSC::Parser<LexerType>::parseFunctionDeclaration):
>+        (JSC::Parser<LexerType>::parseAsyncFunctionDeclaration):
>+        (JSC::Parser<LexerType>::parseClassDeclaration):
>+        (JSC::Parser<LexerType>::parseExportDeclaration):
>+        (JSC::Parser<LexerType>::parseMemberExpression):
>+        * parser/ParserArena.h:
>+        (JSC::IdentifierArena::makeIdentifier):
>+        * runtime/CachedTypes.cpp:
>+        (JSC::CachedUniquedStringImpl::encode):
>+        (JSC::CachedUniquedStringImpl::decode const):
>+        * runtime/CommonIdentifiers.cpp:
>+        (JSC::CommonIdentifiers::CommonIdentifiers):
>+        (JSC::CommonIdentifiers::lookUpPrivateName const):
>+        (JSC::CommonIdentifiers::getPublicName const):
>+        (JSC::CommonIdentifiers::lookUpPublicName const): Deleted.
>+        * runtime/CommonIdentifiers.h:
>+        * runtime/ExceptionHelpers.cpp:
>+        (JSC::createUndefinedVariableError):
>+        * runtime/Identifier.cpp:
>+        (JSC::Identifier::dump const):
>+        * runtime/Identifier.h:
>+        * runtime/IdentifierInlines.h:
>+        (JSC::Identifier::fromUid):
>+        * runtime/JSTypedArrayViewPrototype.cpp:
>+        (JSC::JSTypedArrayViewPrototype::finishCreation):
>+        * tools/JSDollarVM.cpp:
>+        (JSC::functionGetPrivateProperty):
>+
> 2019-02-05  Mark Lam  <mark.lam@apple.com>
> 
>         Fix DFG's doesGC() for a few more nodes.
>diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
>index 9c8b0913f8c1d0cec8ca9973de66271f58cd518a..4c4ee4dc2c3983941a4a803ca80d9cdcac399054 100644
>--- a/Source/WebCore/ChangeLog
>+++ b/Source/WebCore/ChangeLog
>@@ -1,3 +1,19 @@
>+2019-02-06  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [JSC] PrivateName to PublicName hash table is wasteful
>+        https://bugs.webkit.org/show_bug.cgi?id=194277
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Use WebCoreBuiltinNames instead of adding WebCore names to JSC CommonIdentifiers.
>+
>+        * bindings/js/JSDOMWindowCustom.cpp:
>+        (WebCore::addCrossOriginPropertyNames):
>+        * bindings/js/JSLocationCustom.cpp:
>+        (WebCore::getOwnPropertySlotCommon):
>+        (WebCore::putCommon):
>+        * bindings/js/WebCoreBuiltinNames.h:
>+
> 2019-02-05  Keith Rollin  <krollin@apple.com>
> 
>         Enable the automatic checking and regenerations of .xcfilelists during builds
>diff --git a/Source/JavaScriptCore/builtins/BuiltinNames.cpp b/Source/JavaScriptCore/builtins/BuiltinNames.cpp
>index a0f853a6e05dc4618ff04de59019b1ea13d6b37f..ad79fedb6279fddad18c51cba0b7c61d50dffeb5 100644
>--- a/Source/JavaScriptCore/builtins/BuiltinNames.cpp
>+++ b/Source/JavaScriptCore/builtins/BuiltinNames.cpp
>@@ -38,37 +38,58 @@ namespace Symbols {
> JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_WELL_KNOWN_SYMBOL(INITIALIZE_BUILTIN_STATIC_SYMBOLS)
> #undef INITIALIZE_BUILTIN_STATIC_SYMBOLS
> 
>-#define INITIALIZE_BUILTIN_PRIVATE_NAMES(name) SymbolImpl::StaticSymbolImpl name##PrivateName { "PrivateSymbol." #name, SymbolImpl::s_flagIsPrivate };
>+#define INITIALIZE_BUILTIN_PRIVATE_NAMES(name) SymbolImpl::StaticSymbolImpl name##PrivateName { #name, SymbolImpl::s_flagIsPrivate };
> JSC_FOREACH_BUILTIN_FUNCTION_NAME(INITIALIZE_BUILTIN_PRIVATE_NAMES)
> JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_PROPERTY_NAME(INITIALIZE_BUILTIN_PRIVATE_NAMES)
> #undef INITIALIZE_BUILTIN_PRIVATE_NAMES
> 
>-SymbolImpl::StaticSymbolImpl dollarVMPrivateName { "PrivateSymbol.$vm", SymbolImpl::s_flagIsPrivate };
>-SymbolImpl::StaticSymbolImpl polyProtoPrivateName { "PrivateSymbol.PolyProto", SymbolImpl::s_flagIsPrivate };
>+SymbolImpl::StaticSymbolImpl dollarVMPrivateName { "$vm", SymbolImpl::s_flagIsPrivate };
>+SymbolImpl::StaticSymbolImpl polyProtoPrivateName { "PolyProto", SymbolImpl::s_flagIsPrivate };
> 
> } // namespace Symbols
> 
>+#define INITIALIZE_BUILTIN_NAMES_IN_JSC(name) , m_##name(JSC::Identifier::fromString(vm, #name))
>+#define INITIALIZE_BUILTIN_SYMBOLS_IN_JSC(name) , m_##name##Symbol(JSC::Identifier::fromUid(vm, &static_cast<SymbolImpl&>(JSC::Symbols::name##Symbol))), m_##name##SymbolPrivateIdentifier(JSC::Identifier::fromString(vm, #name "Symbol"))
>+
>+#define INITIALIZE_PUBLIC_TO_PRIVATE_ENTRY(name) \
>+    do { \
>+        SymbolImpl* symbol = &static_cast<SymbolImpl&>(JSC::Symbols::name##PrivateName); \
>+        checkPublicToPrivateMapConsistency(m_##name.impl(), symbol); \
>+        m_publicToPrivateMap.add(m_##name.impl(), symbol); \
>+    } while (0);
>+
>+// We commandeer the publicToPrivateMap to allow us to convert private symbol names into the appropriate symbol.
>+// e.g. @iteratorSymbol points to Symbol.iterator in this map rather than to a an actual private name.
>+// FIXME: This is a weird hack and we shouldn't need to do this.
>+#define INITIALIZE_SYMBOL_PUBLIC_TO_PRIVATE_ENTRY(name) \
>+    do { \
>+        SymbolImpl* symbol = static_cast<SymbolImpl*>(m_##name##Symbol.impl()); \
>+        checkPublicToPrivateMapConsistency(m_##name##SymbolPrivateIdentifier.impl(), symbol); \
>+        m_publicToPrivateMap.add(m_##name##SymbolPrivateIdentifier.impl(), symbol); \
>+    } while (0);
>+
> // We treat the dollarVM name as a special case below for $vm (because CommonIdentifiers does not
> // yet support the $ character).
> BuiltinNames::BuiltinNames(VM* vm, CommonIdentifiers* commonIdentifiers)
>     : m_emptyIdentifier(commonIdentifiers->emptyIdentifier)
>     JSC_FOREACH_BUILTIN_FUNCTION_NAME(INITIALIZE_BUILTIN_NAMES_IN_JSC)
>     JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_PROPERTY_NAME(INITIALIZE_BUILTIN_NAMES_IN_JSC)
>-    JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_WELL_KNOWN_SYMBOL(INITIALIZE_BUILTIN_SYMBOLS)
>+    JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_WELL_KNOWN_SYMBOL(INITIALIZE_BUILTIN_SYMBOLS_IN_JSC)
>     , m_dollarVMName(Identifier::fromString(vm, "$vm"))
>     , m_dollarVMPrivateName(Identifier::fromUid(vm, &static_cast<SymbolImpl&>(Symbols::dollarVMPrivateName)))
>     , m_polyProtoPrivateName(Identifier::fromUid(vm, &static_cast<SymbolImpl&>(Symbols::polyProtoPrivateName)))
> {
>-    JSC_FOREACH_BUILTIN_FUNCTION_NAME(INITIALIZE_PRIVATE_TO_PUBLIC_ENTRY)
>-    JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_PROPERTY_NAME(INITIALIZE_PRIVATE_TO_PUBLIC_ENTRY)
>     JSC_FOREACH_BUILTIN_FUNCTION_NAME(INITIALIZE_PUBLIC_TO_PRIVATE_ENTRY)
>     JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_PROPERTY_NAME(INITIALIZE_PUBLIC_TO_PRIVATE_ENTRY)
>-    JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_WELL_KNOWN_SYMBOL(INITIALIZE_SYMBOL_PRIVATE_TO_PUBLIC_ENTRY)
>     JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_WELL_KNOWN_SYMBOL(INITIALIZE_SYMBOL_PUBLIC_TO_PRIVATE_ENTRY)
>-    m_privateToPublicMap.add(m_dollarVMPrivateName.impl(), &m_dollarVMName);
>-    m_publicToPrivateMap.add(m_dollarVMName.impl(), &m_dollarVMPrivateName);
>+    m_publicToPrivateMap.add(m_dollarVMName.impl(), static_cast<SymbolImpl*>(m_dollarVMPrivateName.impl()));
> }
> 
>+#undef INITIALIZE_BUILTIN_NAMES_IN_JSC
>+#undef INITIALIZE_BUILTIN_SYMBOLS_IN_JSC
>+#undef INITIALIZE_PUBLIC_TO_PRIVATE_ENTRY
>+#undef INITIALIZE_SYMBOL_PUBLIC_TO_PRIVATE_ENTRY
>+
> } // namespace JSC
> 
> #if COMPILER(MSVC)
>diff --git a/Source/JavaScriptCore/builtins/BuiltinNames.h b/Source/JavaScriptCore/builtins/BuiltinNames.h
>index 4a701448e2a910bbf37c76220810f3532b6996b6..a6e331d0d0e7bc5976373b85e085fd058c2b8ae0 100644
>--- a/Source/JavaScriptCore/builtins/BuiltinNames.h
>+++ b/Source/JavaScriptCore/builtins/BuiltinNames.h
>@@ -32,11 +32,14 @@
> 
> namespace JSC {
> 
>-#define INITIALIZE_BUILTIN_NAMES_IN_JSC(name) , m_##name(JSC::Identifier::fromString(vm, #name)), m_##name##PrivateName(JSC::Identifier::fromUid(vm, &static_cast<SymbolImpl&>(JSC::Symbols::name##PrivateName)))
>-#define INITIALIZE_BUILTIN_SYMBOLS(name) , m_##name##Symbol(JSC::Identifier::fromUid(vm, &static_cast<SymbolImpl&>(JSC::Symbols::name##Symbol))), m_##name##SymbolPrivateIdentifier(JSC::Identifier::fromString(vm, #name "Symbol"))
>-#define DECLARE_BUILTIN_SYMBOLS(name) const JSC::Identifier m_##name##Symbol; const JSC::Identifier m_##name##SymbolPrivateIdentifier;
>+#define DECLARE_BUILTIN_NAMES_IN_JSC(name) const JSC::Identifier m_##name;
>+#define DECLARE_BUILTIN_SYMBOLS_IN_JSC(name) const JSC::Identifier m_##name##Symbol; const JSC::Identifier m_##name##SymbolPrivateIdentifier;
> #define DECLARE_BUILTIN_SYMBOL_ACCESSOR(name) \
>     const JSC::Identifier& name##Symbol() const { return m_##name##Symbol; }
>+#define DECLARE_BUILTIN_IDENTIFIER_ACCESSOR_IN_JSC(name) \
>+    const JSC::Identifier& name##PublicName() const { return m_##name; } \
>+    JSC::Identifier name##PrivateName() const { return JSC::Identifier::fromUid(*bitwise_cast<SymbolImpl*>(&JSC::Symbols::name##PrivateName)); }
>+
> 
> #define JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_PROPERTY_NAME(macro) \
>     JSC_COMMON_BYTECODE_INTRINSIC_FUNCTIONS_EACH_NAME(macro) \
>@@ -192,84 +195,95 @@ namespace JSC {
>     macro(webAssemblyInstantiateStreamingInternal) \
> 
> namespace Symbols {
>-#define DECLARE_BUILTIN_STATIC_SYMBOLS(name) extern SymbolImpl::StaticSymbolImpl name##Symbol;
>+#define DECLARE_BUILTIN_STATIC_SYMBOLS(name) extern JS_EXPORT_PRIVATE SymbolImpl::StaticSymbolImpl name##Symbol;
> JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_WELL_KNOWN_SYMBOL(DECLARE_BUILTIN_STATIC_SYMBOLS)
> #undef DECLARE_BUILTIN_STATIC_SYMBOLS
> 
>-#define DECLARE_BUILTIN_PRIVATE_NAMES(name) extern SymbolImpl::StaticSymbolImpl name##PrivateName;
>+#define DECLARE_BUILTIN_PRIVATE_NAMES(name) extern JS_EXPORT_PRIVATE SymbolImpl::StaticSymbolImpl name##PrivateName;
> JSC_FOREACH_BUILTIN_FUNCTION_NAME(DECLARE_BUILTIN_PRIVATE_NAMES)
> JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_PROPERTY_NAME(DECLARE_BUILTIN_PRIVATE_NAMES)
> #undef DECLARE_BUILTIN_PRIVATE_NAMES
> 
>-extern SymbolImpl::StaticSymbolImpl dollarVMPrivateName;
>-extern SymbolImpl::StaticSymbolImpl polyProtoPrivateName;
>+extern JS_EXPORT_PRIVATE SymbolImpl::StaticSymbolImpl dollarVMPrivateName;
>+extern JS_EXPORT_PRIVATE SymbolImpl::StaticSymbolImpl polyProtoPrivateName;
> }
> 
>-#define INITIALIZE_PRIVATE_TO_PUBLIC_ENTRY(name) m_privateToPublicMap.add(m_##name##PrivateName.impl(), &m_##name);
>-#define INITIALIZE_PUBLIC_TO_PRIVATE_ENTRY(name) m_publicToPrivateMap.add(m_##name.impl(), &m_##name##PrivateName);
>-
>-// We commandeer the publicToPrivateMap to allow us to convert private symbol names into the appropriate symbol.
>-// e.g. @iteratorSymbol points to Symbol.iterator in this map rather than to a an actual private name.
>-// FIXME: This is a weird hack and we shouldn't need to do this.
>-#define INITIALIZE_SYMBOL_PRIVATE_TO_PUBLIC_ENTRY(name) m_privateToPublicMap.add(m_##name##Symbol.impl(), &m_##name##SymbolPrivateIdentifier);
>-#define INITIALIZE_SYMBOL_PUBLIC_TO_PRIVATE_ENTRY(name) m_publicToPrivateMap.add(m_##name##SymbolPrivateIdentifier.impl(), &m_##name##Symbol);
>-
> class BuiltinNames {
>     WTF_MAKE_NONCOPYABLE(BuiltinNames); WTF_MAKE_FAST_ALLOCATED;
>     
> public:
>     BuiltinNames(VM*, CommonIdentifiers*);
> 
>-    const Identifier* lookUpPrivateName(const Identifier&) const;
>-    const Identifier& lookUpPublicName(const Identifier&) const;
>+    SymbolImpl* lookUpPrivateName(const Identifier&) const;
>+    Identifier getPublicName(VM&, SymbolImpl*) const;
>     
>     void appendExternalName(const Identifier& publicName, const Identifier& privateName);
> 
>-    JSC_FOREACH_BUILTIN_FUNCTION_NAME(DECLARE_BUILTIN_IDENTIFIER_ACCESSOR)
>-    JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_PROPERTY_NAME(DECLARE_BUILTIN_IDENTIFIER_ACCESSOR)
>+    JSC_FOREACH_BUILTIN_FUNCTION_NAME(DECLARE_BUILTIN_IDENTIFIER_ACCESSOR_IN_JSC)
>+    JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_PROPERTY_NAME(DECLARE_BUILTIN_IDENTIFIER_ACCESSOR_IN_JSC)
>     JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_WELL_KNOWN_SYMBOL(DECLARE_BUILTIN_SYMBOL_ACCESSOR)
>     const JSC::Identifier& dollarVMPublicName() const { return m_dollarVMName; }
>     const JSC::Identifier& dollarVMPrivateName() const { return m_dollarVMPrivateName; }
>     const JSC::Identifier& polyProtoName() const { return m_polyProtoPrivateName; }
> 
> private:
>+    void checkPublicToPrivateMapConsistency(UniquedStringImpl* publicName, UniquedStringImpl* privateName);
>+
>     Identifier m_emptyIdentifier;
>-    JSC_FOREACH_BUILTIN_FUNCTION_NAME(DECLARE_BUILTIN_NAMES)
>-    JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_PROPERTY_NAME(DECLARE_BUILTIN_NAMES)
>-    JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_WELL_KNOWN_SYMBOL(DECLARE_BUILTIN_SYMBOLS)
>+    JSC_FOREACH_BUILTIN_FUNCTION_NAME(DECLARE_BUILTIN_NAMES_IN_JSC)
>+    JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_PROPERTY_NAME(DECLARE_BUILTIN_NAMES_IN_JSC)
>+    JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_WELL_KNOWN_SYMBOL(DECLARE_BUILTIN_SYMBOLS_IN_JSC)
>     const JSC::Identifier m_dollarVMName;
>     const JSC::Identifier m_dollarVMPrivateName;
>     const JSC::Identifier m_polyProtoPrivateName;
>-    typedef HashMap<RefPtr<UniquedStringImpl>, const Identifier*, IdentifierRepHash> BuiltinNamesMap;
>+    typedef HashMap<RefPtr<UniquedStringImpl>, SymbolImpl*, IdentifierRepHash> BuiltinNamesMap;
>     BuiltinNamesMap m_publicToPrivateMap;
>-    BuiltinNamesMap m_privateToPublicMap;
> };
> 
>-inline const Identifier* BuiltinNames::lookUpPrivateName(const Identifier& ident) const
>+inline SymbolImpl* BuiltinNames::lookUpPrivateName(const Identifier& ident) const
> {
>     auto iter = m_publicToPrivateMap.find(ident.impl());
>     if (iter != m_publicToPrivateMap.end())
>         return iter->value;
>-    return 0;
>+    return nullptr;
> }
> 
>-inline const Identifier& BuiltinNames::lookUpPublicName(const Identifier& ident) const
>+inline Identifier BuiltinNames::getPublicName(VM& vm, SymbolImpl* symbol) const
> {
>-    auto iter = m_privateToPublicMap.find(ident.impl());
>-    if (iter != m_privateToPublicMap.end())
>-        return *iter->value;
>-    return m_emptyIdentifier;
>+    if (symbol->isPrivate())
>+        return Identifier::fromString(&vm, symbol);
>+    // We have special handling for well-known symbols.
>+    ASSERT(symbol->startsWith("Symbol."));
>+    return Identifier::fromString(&vm, makeString(String(symbol->substring(strlen("Symbol."))), "Symbol"));
> }
> 
>-inline void BuiltinNames::appendExternalName(const Identifier& publicName, const Identifier& privateName)
>+inline void BuiltinNames::checkPublicToPrivateMapConsistency(UniquedStringImpl* publicName, UniquedStringImpl* privateName)
> {
> #ifndef NDEBUG
>     for (const auto& key : m_publicToPrivateMap.keys())
>-        ASSERT(publicName.string() != *key);
>+        ASSERT(String(publicName) != *key);
>+
>+    ASSERT(privateName->isSymbol());
>+    SymbolImpl* symbol = static_cast<SymbolImpl*>(privateName);
>+    if (symbol->isPrivate()) {
>+        // This guarantees that we can get public symbols from private symbols by using content of private symbols.
>+        ASSERT(String(symbol) == *publicName);
>+    } else {
>+        // We have a hack in m_publicToPrivateMap: adding non-private Symbol with readable name to use it
>+        // in builtin code. The example is @iteratorSymbol => Symbol.iterator mapping. To allow the reverse
>+        // transformation, we ensure that non-private symbol mapping has xxxSymbol => Symbol.xxx.
>+        ASSERT(makeString(String(symbol), "Symbol") == makeString("Symbol.", String(publicName)));
>+    }
>+#else
>+    UNUSED_PARAM(publicName);
>+    UNUSED_PARAM(privateName);
> #endif
>+}
> 
>-    m_privateToPublicMap.add(privateName.impl(), &publicName);
>-    m_publicToPrivateMap.add(publicName.impl(), &privateName);
>+inline void BuiltinNames::appendExternalName(const Identifier& publicName, const Identifier& privateName)
>+{
>+    checkPublicToPrivateMapConsistency(publicName.impl(), privateName.impl());
>+    m_publicToPrivateMap.add(publicName.impl(), static_cast<SymbolImpl*>(privateName.impl()));
> }
> 
> } // namespace JSC
>diff --git a/Source/JavaScriptCore/builtins/BuiltinUtils.h b/Source/JavaScriptCore/builtins/BuiltinUtils.h
>index 941e529ff1fa8c39c278a320c98b99ac5a98c2f4..963bdee20a156c47e918567f0b80abbb7cd78e8b 100644
>--- a/Source/JavaScriptCore/builtins/BuiltinUtils.h
>+++ b/Source/JavaScriptCore/builtins/BuiltinUtils.h
>@@ -30,7 +30,7 @@
> 
> namespace JSC {
> 
>-#define INITIALIZE_BUILTIN_NAMES(name) , m_##name(JSC::Identifier::fromString(vm, #name)), m_##name##PrivateName(JSC::Identifier::fromUid(JSC::PrivateName(JSC::PrivateName::PrivateSymbol, "PrivateSymbol." #name ""_s)))
>+#define INITIALIZE_BUILTIN_NAMES(name) , m_##name(JSC::Identifier::fromString(vm, #name)), m_##name##PrivateName(JSC::Identifier::fromUid(JSC::PrivateName(JSC::PrivateName::PrivateSymbol, #name ""_s)))
> #define DECLARE_BUILTIN_NAMES(name) const JSC::Identifier m_##name; const JSC::Identifier m_##name##PrivateName;
> #define DECLARE_BUILTIN_IDENTIFIER_ACCESSOR(name) \
>     const JSC::Identifier& name##PublicName() const { return m_##name; } \
>diff --git a/Source/JavaScriptCore/bytecode/BytecodeDumper.cpp b/Source/JavaScriptCore/bytecode/BytecodeDumper.cpp
>index 620dc9f6f63bfa101db2f44798e75fd0e6d45bbc..721d390552c92a8b3a264b08e9525812f8244f8e 100644
>--- a/Source/JavaScriptCore/bytecode/BytecodeDumper.cpp
>+++ b/Source/JavaScriptCore/bytecode/BytecodeDumper.cpp
>@@ -104,7 +104,7 @@ void BytecodeDumper<Block>::dumpIdentifiers()
>         m_out.printf("\nIdentifiers:\n");
>         size_t i = 0;
>         do {
>-            m_out.printf("  id%u = %s\n", static_cast<unsigned>(i), identifier(i).string().utf8().data());
>+            m_out.print("  id", static_cast<unsigned>(i), " = ", identifier(i), "\n");
>             ++i;
>         } while (i != count);
>     }
>diff --git a/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp b/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
>index 6d5045b831b9ee87e0f28b8eea706a945c52423f..15a8d3d651cd40d689591abdcb94f94625932fc6 100644
>--- a/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
>+++ b/Source/JavaScriptCore/bytecompiler/NodesCodegen.cpp
>@@ -967,10 +967,10 @@ RegisterID* BytecodeIntrinsicNode::emit_intrinsic_getByIdDirectPrivate(BytecodeG
>     RefPtr<RegisterID> base = generator.emitNode(node);
>     node = node->m_next;
>     ASSERT(node->m_expr->isString());
>-    const Identifier* ident = generator.vm()->propertyNames->lookUpPrivateName(static_cast<StringNode*>(node->m_expr)->value());
>-    ASSERT(ident);
>+    SymbolImpl* symbol = generator.vm()->propertyNames->lookUpPrivateName(static_cast<StringNode*>(node->m_expr)->value());
>+    ASSERT(symbol);
>     ASSERT(!node->m_next);
>-    return generator.emitDirectGetById(generator.finalDestination(dst), base.get(), *ident);
>+    return generator.emitDirectGetById(generator.finalDestination(dst), base.get(), generator.parserArena().identifierArena().makeIdentifier(generator.vm(), symbol));
> }
> 
> RegisterID* BytecodeIntrinsicNode::emit_intrinsic_argument(BytecodeGenerator& generator, RegisterID* dst)
>@@ -1018,14 +1018,14 @@ RegisterID* BytecodeIntrinsicNode::emit_intrinsic_putByIdDirectPrivate(BytecodeG
>     RefPtr<RegisterID> base = generator.emitNode(node);
>     node = node->m_next;
>     ASSERT(node->m_expr->isString());
>-    const Identifier* ident = generator.vm()->propertyNames->lookUpPrivateName(static_cast<StringNode*>(node->m_expr)->value());
>-    ASSERT(ident);
>+    SymbolImpl* symbol = generator.vm()->propertyNames->lookUpPrivateName(static_cast<StringNode*>(node->m_expr)->value());
>+    ASSERT(symbol);
>     node = node->m_next;
>     RefPtr<RegisterID> value = generator.emitNode(node);
> 
>     ASSERT(!node->m_next);
> 
>-    return generator.move(dst, generator.emitDirectPutById(base.get(), *ident, value.get(), PropertyNode::KnownDirect));
>+    return generator.move(dst, generator.emitDirectPutById(base.get(), generator.parserArena().identifierArena().makeIdentifier(generator.vm(), symbol), value.get(), PropertyNode::KnownDirect));
> }
> 
> RegisterID* BytecodeIntrinsicNode::emit_intrinsic_putByValDirect(BytecodeGenerator& generator, RegisterID* dst)
>diff --git a/Source/JavaScriptCore/parser/Lexer.cpp b/Source/JavaScriptCore/parser/Lexer.cpp
>index 122c4d91d4b17c54e714faf428b3194412f0ea18..2402a97f52f9825136df0b1ba09909764ace3b73 100644
>--- a/Source/JavaScriptCore/parser/Lexer.cpp
>+++ b/Source/JavaScriptCore/parser/Lexer.cpp
>@@ -975,9 +975,9 @@ template <bool shouldCreateIdentifier> ALWAYS_INLINE JSTokenType Lexer<LChar>::p
>                 return ERRORTOK;
>             }
>             if (isPrivateName)
>-                ident = m_vm->propertyNames->lookUpPrivateName(*ident);
>+                ident = &m_arena->makeIdentifier(m_vm, m_vm->propertyNames->lookUpPrivateName(*ident));
>             else if (*ident == m_vm->propertyNames->undefinedKeyword)
>-                tokenData->ident = &m_vm->propertyNames->builtinNames().undefinedPrivateName();
>+                tokenData->ident = &m_vm->propertyNames->undefinedPrivateName;
>             if (!ident)
>                 return INVALID_PRIVATE_NAME_ERRORTOK;
>         }
>@@ -1053,9 +1053,9 @@ template <bool shouldCreateIdentifier> ALWAYS_INLINE JSTokenType Lexer<UChar>::p
>                 return ERRORTOK;
>             }
>             if (isPrivateName)
>-                ident = m_vm->propertyNames->lookUpPrivateName(*ident);
>+                ident = &m_arena->makeIdentifier(m_vm, m_vm->propertyNames->lookUpPrivateName(*ident));
>             else if (*ident == m_vm->propertyNames->undefinedKeyword)
>-                tokenData->ident = &m_vm->propertyNames->builtinNames().undefinedPrivateName();
>+                tokenData->ident = &m_vm->propertyNames->undefinedPrivateName;
>             if (!ident)
>                 return INVALID_PRIVATE_NAME_ERRORTOK;
>         }
>diff --git a/Source/JavaScriptCore/parser/Parser.cpp b/Source/JavaScriptCore/parser/Parser.cpp
>index 1d223bc3a1236cd785bf04aee9e02e104c2b97bc..8c1466c5c6193f15007f8d03eca54dbad6dc44ce 100644
>--- a/Source/JavaScriptCore/parser/Parser.cpp
>+++ b/Source/JavaScriptCore/parser/Parser.cpp
>@@ -2284,15 +2284,15 @@ template <class TreeBuilder> typename TreeBuilder::FormalParameterList Parser<Le
>     };
> 
>     // @generator
>-    addParameter(m_vm->propertyNames->builtinNames().generatorPrivateName());
>+    addParameter(m_vm->propertyNames->generatorPrivateName);
>     // @generatorState
>-    addParameter(m_vm->propertyNames->builtinNames().generatorStatePrivateName());
>+    addParameter(m_vm->propertyNames->generatorStatePrivateName);
>     // @generatorValue
>-    addParameter(m_vm->propertyNames->builtinNames().generatorValuePrivateName());
>+    addParameter(m_vm->propertyNames->generatorValuePrivateName);
>     // @generatorResumeMode
>-    addParameter(m_vm->propertyNames->builtinNames().generatorResumeModePrivateName());
>+    addParameter(m_vm->propertyNames->generatorResumeModePrivateName);
>     // @generatorFrame
>-    addParameter(m_vm->propertyNames->builtinNames().generatorFramePrivateName());
>+    addParameter(m_vm->propertyNames->generatorFramePrivateName);
> 
>     return parameters;
> }
>@@ -2666,7 +2666,7 @@ template <class TreeBuilder> TreeStatement Parser<LexerType>::parseFunctionDecla
>         //
>         // In this case, we use "*default*" as this function declaration's name.
>         requirements = FunctionNameRequirements::None;
>-        functionInfo.name = &m_vm->propertyNames->builtinNames().starDefaultPrivateName();
>+        functionInfo.name = &m_vm->propertyNames->starDefaultPrivateName;
>     }
> 
>     failIfFalse((parseFunctionInfo(context, requirements, parseMode, true, ConstructorKind::None, SuperBinding::NotNeeded, functionKeywordStart, functionInfo, FunctionDefinitionType::Declaration, functionConstructorParametersEndPosition)), "Cannot parse this function");
>@@ -2724,7 +2724,7 @@ template <class TreeBuilder> TreeStatement Parser<LexerType>::parseAsyncFunction
>         //
>         // In this case, we use "*default*" as this function declaration's name.
>         requirements = FunctionNameRequirements::None;
>-        functionInfo.name = &m_vm->propertyNames->builtinNames().starDefaultPrivateName();
>+        functionInfo.name = &m_vm->propertyNames->starDefaultPrivateName;
>     }
> 
>     failIfFalse((parseFunctionInfo(context, requirements, parseMode, true, ConstructorKind::None, SuperBinding::NotNeeded, functionKeywordStart, functionInfo, FunctionDefinitionType::Declaration, functionConstructorParametersEndPosition)), "Cannot parse this async function");
>@@ -2770,7 +2770,7 @@ template <class TreeBuilder> TreeStatement Parser<LexerType>::parseClassDeclarat
>         //
>         // In this case, we use "*default*" as this class declaration's name.
>         requirements = FunctionNameRequirements::None;
>-        info.className = &m_vm->propertyNames->builtinNames().starDefaultPrivateName();
>+        info.className = &m_vm->propertyNames->starDefaultPrivateName;
>     }
> 
>     TreeClassExpression classExpr = parseClass(context, requirements, info);
>@@ -3407,7 +3407,7 @@ template <class TreeBuilder> TreeStatement Parser<LexerType>::parseExportDeclara
>         }
> 
>         if (!localName)
>-            localName = &m_vm->propertyNames->builtinNames().starDefaultPrivateName();
>+            localName = &m_vm->propertyNames->starDefaultPrivateName;
> 
>         if (isFunctionOrClassDeclaration) {
>             if (startsWithFunction) {
>@@ -3439,11 +3439,11 @@ template <class TreeBuilder> TreeStatement Parser<LexerType>::parseExportDeclara
>             TreeExpression expression = parseAssignmentExpression(context);
>             failIfFalse(expression, "Cannot parse expression");
> 
>-            DeclarationResultMask declarationResult = declareVariable(&m_vm->propertyNames->builtinNames().starDefaultPrivateName(), DeclarationType::ConstDeclaration);
>+            DeclarationResultMask declarationResult = declareVariable(&m_vm->propertyNames->starDefaultPrivateName, DeclarationType::ConstDeclaration);
>             if (declarationResult & DeclarationResult::InvalidDuplicateDeclaration)
>                 internalFailWithMessage(false, "Only one 'default' export is allowed");
> 
>-            TreeExpression assignment = context.createAssignResolve(location, m_vm->propertyNames->builtinNames().starDefaultPrivateName(), expression, start, start, tokenEndPosition(), AssignmentContext::ConstDeclarationStatement);
>+            TreeExpression assignment = context.createAssignResolve(location, m_vm->propertyNames->starDefaultPrivateName, expression, start, start, tokenEndPosition(), AssignmentContext::ConstDeclarationStatement);
>             result = context.createExprStatement(location, assignment, start, tokenEndPosition());
>             failIfFalse(autoSemiColon(), "Expected a ';' following a targeted export declaration");
>         }
>@@ -4733,7 +4733,7 @@ template <class TreeBuilder> TreeExpression Parser<LexerType>::parseMemberExpres
>                 semanticFailIfFalse(m_scriptMode == JSParserScriptMode::Module, "import.meta is only valid inside modules");
> 
>                 JSTokenLocation location(tokenLocation());
>-                base = context.createImportMetaExpr(location, createResolveAndUseVariable(context, &m_vm->propertyNames->builtinNames().metaPrivateName(), false, expressionStart, location));
>+                base = context.createImportMetaExpr(location, createResolveAndUseVariable(context, &m_vm->propertyNames->metaPrivateName, false, expressionStart, location));
>                 next();
>             } else {
>                 failIfTrue(match(IDENT), "\"import.\" can only followed with meta");
>diff --git a/Source/JavaScriptCore/parser/ParserArena.h b/Source/JavaScriptCore/parser/ParserArena.h
>index 85b48559717675f8e5ac9f22fdc5b76d1a7e4b44..67b782dd3dc6cb52c178f336df338a726d986969 100644
>--- a/Source/JavaScriptCore/parser/ParserArena.h
>+++ b/Source/JavaScriptCore/parser/ParserArena.h
>@@ -47,6 +47,7 @@ namespace JSC {
>         ALWAYS_INLINE const Identifier& makeIdentifier(VM*, const T* characters, size_t length);
>         ALWAYS_INLINE const Identifier& makeEmptyIdentifier(VM*);
>         ALWAYS_INLINE const Identifier& makeIdentifierLCharFromUChar(VM*, const UChar* characters, size_t length);
>+        ALWAYS_INLINE const Identifier& makeIdentifier(VM*, SymbolImpl*);
> 
>         const Identifier& makeNumericIdentifier(VM*, double number);
> 
>@@ -92,6 +93,13 @@ namespace JSC {
>         return m_identifiers.last();
>     }
> 
>+    ALWAYS_INLINE const Identifier& IdentifierArena::makeIdentifier(VM*, SymbolImpl* symbol)
>+    {
>+        ASSERT(symbol);
>+        m_identifiers.append(Identifier::fromUid(*symbol));
>+        return m_identifiers.last();
>+    }
>+
>     ALWAYS_INLINE const Identifier& IdentifierArena::makeEmptyIdentifier(VM* vm)
>     {
>         return vm->propertyNames->emptyIdentifier;
>diff --git a/Source/JavaScriptCore/runtime/CachedTypes.cpp b/Source/JavaScriptCore/runtime/CachedTypes.cpp
>index b53dfd62aac4fd2511935f2f45f08e34c09370da..4358171e6c33f5dbaea05ec3cfebd1e5c64395d1 100644
>--- a/Source/JavaScriptCore/runtime/CachedTypes.cpp
>+++ b/Source/JavaScriptCore/runtime/CachedTypes.cpp
>@@ -534,12 +534,14 @@ class CachedUniquedStringImpl : public VariableLengthObject<UniquedStringImpl> {
>     {
>         m_isAtomic = string.isAtomic();
>         m_isSymbol = string.isSymbol();
>-        StringImpl* impl = const_cast<StringImpl*>(&string);
>+        RefPtr<StringImpl> impl = const_cast<StringImpl*>(&string);
>+
>         if (m_isSymbol) {
>-            SymbolImpl* symbol = static_cast<SymbolImpl*>(impl);
>+            SymbolImpl* symbol = static_cast<SymbolImpl*>(impl.get());
>             if (!symbol->isNullSymbol()) {
>-                Identifier uid = Identifier::fromUid(&encoder.vm(), symbol);
>-                impl = encoder.vm().propertyNames->lookUpPublicName(uid).string().impl();
>+                // We have special handling for well-known symbols.
>+                if (!symbol->isPrivate())
>+                    impl = encoder.vm().propertyNames->getPublicName(encoder.vm(), symbol).impl();
>             }
>         }
> 
>@@ -572,7 +574,7 @@ class CachedUniquedStringImpl : public VariableLengthObject<UniquedStringImpl> {
>                 return &SymbolImpl::createNullSymbol().leakRef();
> 
>             Identifier ident = Identifier::fromString(&decoder.vm(), buffer, m_length);
>-            String str = decoder.vm().propertyNames->lookUpPrivateName(ident)->string();
>+            String str = decoder.vm().propertyNames->lookUpPrivateName(ident);
>             StringImpl* impl = str.releaseImpl().get();
>             ASSERT(impl->isSymbol());
>             return static_cast<UniquedStringImpl*>(impl);
>diff --git a/Source/JavaScriptCore/runtime/CommonIdentifiers.cpp b/Source/JavaScriptCore/runtime/CommonIdentifiers.cpp
>index 197370d110a75da60bdce44995bfe7c26a1893a2..deb3cfcc49a653cd9877af350bee24bb426735de 100644
>--- a/Source/JavaScriptCore/runtime/CommonIdentifiers.cpp
>+++ b/Source/JavaScriptCore/runtime/CommonIdentifiers.cpp
>@@ -41,6 +41,7 @@ CommonIdentifiers::CommonIdentifiers(VM* vm)
>     , useStrictIdentifier(Identifier::fromString(vm, "use strict"))
>     , timesIdentifier(Identifier::fromString(vm, "*"))
>     , m_builtinNames(new BuiltinNames(vm, this))
>+    JSC_PARSER_PRIVATE_NAMES(INITIALIZE_PRIVATE_NAME)
>     JSC_COMMON_IDENTIFIERS_EACH_KEYWORD(INITIALIZE_KEYWORD)
>     JSC_COMMON_IDENTIFIERS_EACH_PROPERTY_NAME(INITIALIZE_PROPERTY_NAME)
>     JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_WELL_KNOWN_SYMBOL(INITIALIZE_SYMBOL)
>@@ -51,16 +52,16 @@ CommonIdentifiers::~CommonIdentifiers()
> {
> }
> 
>-const Identifier* CommonIdentifiers::lookUpPrivateName(const Identifier& ident) const
>+SymbolImpl* CommonIdentifiers::lookUpPrivateName(const Identifier& ident) const
> {
>     return m_builtinNames->lookUpPrivateName(ident);
> }
>-    
>-Identifier CommonIdentifiers::lookUpPublicName(const Identifier& ident) const
>+
>+Identifier CommonIdentifiers::getPublicName(VM& vm, SymbolImpl* symbol) const
> {
>-    return m_builtinNames->lookUpPublicName(ident);
>+    return m_builtinNames->getPublicName(vm, symbol);
> }
>-
>+    
> void CommonIdentifiers::appendExternalName(const Identifier& publicName, const Identifier& privateName)
> {
>     m_builtinNames->appendExternalName(publicName, privateName);
>diff --git a/Source/JavaScriptCore/runtime/CommonIdentifiers.h b/Source/JavaScriptCore/runtime/CommonIdentifiers.h
>index 80227f77a827d72c4f04d22de9a2225c3f95e389..c06f7ec75c88bb99be64f498cce719917fca424a 100644
>--- a/Source/JavaScriptCore/runtime/CommonIdentifiers.h
>+++ b/Source/JavaScriptCore/runtime/CommonIdentifiers.h
>@@ -28,49 +28,31 @@
> #define JSC_COMMON_IDENTIFIERS_EACH_PROPERTY_NAME(macro) \
>     macro(Array) \
>     macro(ArrayBuffer) \
>-    macro(ArrayIterator) \
>     macro(BYTES_PER_ELEMENT) \
>     macro(BigInt) \
>     macro(Boolean) \
>     macro(Collator) \
>-    macro(Credential) \
>     macro(Date) \
>     macro(DateTimeFormat) \
>-    macro(DataTransferItem) \
>-    macro(DataTransferItemList) \
>     macro(Error) \
>     macro(EvalError) \
>     macro(Function) \
>-    macro(GeneratorFunction) \
>     macro(Infinity) \
>     macro(Intl) \
>-    macro(JSON) \
>     macro(Loader) \
>-    macro(Map)\
>-    macro(MapIterator)\
>-    macro(Math) \
>+    macro(Map) \
>     macro(NaN) \
>     macro(Number) \
>     macro(NumberFormat) \
>     macro(Object) \
>     macro(PluralRules) \
>     macro(Promise) \
>-    macro(Proxy) \
>-    macro(RangeError) \
>-    macro(ReferenceError) \
>     macro(Reflect) \
>     macro(RegExp) \
>-    macro(Set)\
>-    macro(SetIterator)\
>+    macro(Set) \
>     macro(SharedArrayBuffer) \
>     macro(String) \
>     macro(Symbol) \
>-    macro(SyntaxError) \
>-    macro(TypeError) \
>-    macro(URIError) \
>-    macro(UTC) \
>-    macro(WeakMap)\
>-    macro(WeakSet)\
>     macro(__defineGetter__) \
>     macro(__defineSetter__) \
>     macro(__lookupGetter__) \
>@@ -80,11 +62,9 @@
>     macro(anonymous) \
>     macro(arguments) \
>     macro(as) \
>-    macro(assign) \
>     macro(async) \
>     macro(back) \
>     macro(bind) \
>-    macro(buffer) \
>     macro(byteLength) \
>     macro(byteOffset) \
>     macro(bytecode) \
>@@ -121,7 +101,6 @@
>     macro(exec) \
>     macro(executionCount) \
>     macro(exitKind) \
>-    macro(fetch) \
>     macro(flags) \
>     macro(forEach) \
>     macro(formatMatcher) \
>@@ -140,7 +119,6 @@
>     macro(hour) \
>     macro(hourCycle) \
>     macro(hour12) \
>-    macro(href) \
>     macro(id) \
>     macro(ignoreCase) \
>     macro(ignorePunctuation) \
>@@ -184,7 +162,6 @@
>     macro(propertyIsEnumerable) \
>     macro(prototype) \
>     macro(raw) \
>-    macro(reload) \
>     macro(replace) \
>     macro(resolve) \
>     macro(second) \
>@@ -285,6 +262,16 @@
>     macro(toStringTag) \
>     macro(unscopables)
> 
>+#define JSC_PARSER_PRIVATE_NAMES(macro) \
>+    macro(generator) \
>+    macro(generatorState) \
>+    macro(generatorValue) \
>+    macro(generatorResumeMode) \
>+    macro(generatorFrame) \
>+    macro(meta) \
>+    macro(starDefault) \
>+    macro(undefined) \
>+
> namespace JSC {
>     
>     class BuiltinNames;
>@@ -308,6 +295,10 @@ namespace JSC {
>         std::unique_ptr<BuiltinNames> m_builtinNames;
> 
>     public:
>+
>+#define JSC_IDENTIFIER_DECLARE_PARSER_PRIVATE_NAME(name) const Identifier name##PrivateName;
>+        JSC_PARSER_PRIVATE_NAMES(JSC_IDENTIFIER_DECLARE_PARSER_PRIVATE_NAME)
>+#undef JSC_IDENTIFIER_DECLARE_PARSER_PRIVATE_NAME
>         
> #define JSC_IDENTIFIER_DECLARE_KEYWORD_NAME_GLOBAL(name) const Identifier name##Keyword;
>         JSC_COMMON_IDENTIFIERS_EACH_KEYWORD(JSC_IDENTIFIER_DECLARE_KEYWORD_NAME_GLOBAL)
>@@ -321,8 +312,8 @@ namespace JSC {
>         JSC_COMMON_PRIVATE_IDENTIFIERS_EACH_WELL_KNOWN_SYMBOL(JSC_IDENTIFIER_DECLARE_PRIVATE_WELL_KNOWN_SYMBOL_GLOBAL)
> #undef JSC_IDENTIFIER_DECLARE_PRIVATE_WELL_KNOWN_SYMBOL_GLOBAL
> 
>-        const Identifier* lookUpPrivateName(const Identifier&) const;
>-        Identifier lookUpPublicName(const Identifier&) const;
>+        SymbolImpl* lookUpPrivateName(const Identifier&) const;
>+        Identifier getPublicName(VM&, SymbolImpl*) const;
> 
>         // Callers of this method should make sure that identifiers given to this method 
>         // survive the lifetime of CommonIdentifiers and related VM.
>diff --git a/Source/JavaScriptCore/runtime/ExceptionHelpers.cpp b/Source/JavaScriptCore/runtime/ExceptionHelpers.cpp
>index 8a617b480f5bcf334eda8947f8eea1a4b64d1e5d..a37446e4d2762e4c24cbeab66dce191b54726491 100644
>--- a/Source/JavaScriptCore/runtime/ExceptionHelpers.cpp
>+++ b/Source/JavaScriptCore/runtime/ExceptionHelpers.cpp
>@@ -82,8 +82,7 @@ JSObject* createStackOverflowError(ExecState* exec, JSGlobalObject* globalObject
> JSObject* createUndefinedVariableError(ExecState* exec, const Identifier& ident)
> {
>     if (ident.isPrivateName()) {
>-        VM& vm = exec->vm();
>-        String message(makeString("Can't find private variable: @", vm.propertyNames->lookUpPublicName(ident).string()));
>+        String message(makeString("Can't find private variable: PrivateSymbol.", ident.string()));
>         return createReferenceError(exec, message);
>     }
>     String message(makeString("Can't find variable: ", ident.string()));
>diff --git a/Source/JavaScriptCore/runtime/Identifier.cpp b/Source/JavaScriptCore/runtime/Identifier.cpp
>index f84b69e4d3d3f5df8b66ff76881efe495fdb242c..46f79c86ed6ec10db1a7437df317451f9e938ebb 100644
>--- a/Source/JavaScriptCore/runtime/Identifier.cpp
>+++ b/Source/JavaScriptCore/runtime/Identifier.cpp
>@@ -96,9 +96,14 @@ Identifier Identifier::from(VM* vm, double value)
> 
> void Identifier::dump(PrintStream& out) const
> {
>-    if (impl())
>+    if (impl()) {
>+        if (impl()->isSymbol()) {
>+            auto* symbol = static_cast<SymbolImpl*>(impl());
>+            if (symbol->isPrivate())
>+                out.print("PrivateSymbol.");
>+        }
>         out.print(impl());
>-    else
>+    } else
>         out.print("<null identifier>");
> }
> 
>diff --git a/Source/JavaScriptCore/runtime/Identifier.h b/Source/JavaScriptCore/runtime/Identifier.h
>index 54e84631d76def3215b29058ad0cf3cbd2bebb14..fa8d3421c41a2ed2c33100c22b4af6eb9ad1d316 100644
>--- a/Source/JavaScriptCore/runtime/Identifier.h
>+++ b/Source/JavaScriptCore/runtime/Identifier.h
>@@ -126,6 +126,7 @@ class Identifier {
>     static Identifier fromUid(VM*, UniquedStringImpl* uid);
>     static Identifier fromUid(ExecState*, UniquedStringImpl* uid);
>     static Identifier fromUid(const PrivateName&);
>+    static Identifier fromUid(SymbolImpl&);
> 
>     static Identifier createLCharFromUChar(VM* vm, const UChar* s, int length) { return Identifier(vm, add8(vm, s, length)); }
> 
>diff --git a/Source/JavaScriptCore/runtime/IdentifierInlines.h b/Source/JavaScriptCore/runtime/IdentifierInlines.h
>index 9b226a215de2b5cc2e53738c7fa76784dbe19e37..9964cc59c80861d9c2380ebab3e7e2cf1cf8ae09 100644
>--- a/Source/JavaScriptCore/runtime/IdentifierInlines.h
>+++ b/Source/JavaScriptCore/runtime/IdentifierInlines.h
>@@ -88,6 +88,11 @@ inline Identifier Identifier::fromUid(const PrivateName& name)
>     return name.uid();
> }
> 
>+inline Identifier Identifier::fromUid(SymbolImpl& symbol)
>+{
>+    return symbol;
>+}
>+
> template<unsigned charactersCount>
> inline Identifier Identifier::fromString(VM* vm, const char (&characters)[charactersCount])
> {
>diff --git a/Source/JavaScriptCore/runtime/JSTypedArrayViewPrototype.cpp b/Source/JavaScriptCore/runtime/JSTypedArrayViewPrototype.cpp
>index 9dd874413e24a8cbb3bf27d55594294eca901f53..83902aff8b15bc052ddd8bc94baccae85ea64072 100644
>--- a/Source/JavaScriptCore/runtime/JSTypedArrayViewPrototype.cpp
>+++ b/Source/JavaScriptCore/runtime/JSTypedArrayViewPrototype.cpp
>@@ -297,7 +297,7 @@ void JSTypedArrayViewPrototype::finishCreation(VM& vm, JSGlobalObject* globalObj
> 
>     putDirectWithoutTransition(vm, vm.propertyNames->toString, globalObject->arrayProtoToStringFunction(), static_cast<unsigned>(PropertyAttribute::DontEnum));
> 
>-    JSC_NATIVE_GETTER(vm.propertyNames->buffer, typedArrayViewProtoGetterFuncBuffer, PropertyAttribute::DontEnum | PropertyAttribute::ReadOnly);
>+    JSC_NATIVE_GETTER("buffer", typedArrayViewProtoGetterFuncBuffer, PropertyAttribute::DontEnum | PropertyAttribute::ReadOnly);
>     JSC_NATIVE_INTRINSIC_GETTER(vm.propertyNames->byteLength, typedArrayViewProtoGetterFuncByteLength, PropertyAttribute::DontEnum | PropertyAttribute::ReadOnly, TypedArrayByteLengthIntrinsic);
>     JSC_NATIVE_INTRINSIC_GETTER(vm.propertyNames->byteOffset, typedArrayViewProtoGetterFuncByteOffset, PropertyAttribute::DontEnum | PropertyAttribute::ReadOnly, TypedArrayByteOffsetIntrinsic);
>     JSC_NATIVE_FUNCTION_WITHOUT_TRANSITION("copyWithin", typedArrayViewProtoFuncCopyWithin, static_cast<unsigned>(PropertyAttribute::DontEnum), 2);
>diff --git a/Source/JavaScriptCore/tools/JSDollarVM.cpp b/Source/JavaScriptCore/tools/JSDollarVM.cpp
>index 6cc62ea6f333eda452268f4ee8d07b2fd332a82a..5a5a3cb8f300de284bcaa1b2381bb0b5e3e59d71 100644
>--- a/Source/JavaScriptCore/tools/JSDollarVM.cpp
>+++ b/Source/JavaScriptCore/tools/JSDollarVM.cpp
>@@ -1854,11 +1854,11 @@ static EncodedJSValue JSC_HOST_CALL functionGetPrivateProperty(ExecState* exec)
> 
>     String str = asString(exec->argument(1))->value(exec);
> 
>-    const Identifier* ident = vm.propertyNames->lookUpPrivateName(Identifier::fromString(exec, str));
>-    if (!ident)
>+    SymbolImpl* symbol = vm.propertyNames->lookUpPrivateName(Identifier::fromString(exec, str));
>+    if (!symbol)
>         return throwVMError(exec, scope, "Unknown private name.");
> 
>-    RELEASE_AND_RETURN(scope, JSValue::encode(exec->argument(0).get(exec, *ident)));
>+    RELEASE_AND_RETURN(scope, JSValue::encode(exec->argument(0).get(exec, symbol)));
> }
> 
> static EncodedJSValue JSC_HOST_CALL functionCreateRoot(ExecState* exec)
>diff --git a/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp b/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp
>index d68e703efe77dfc0f8cfc1dcec393924f4d33516..4f72c57787166e5884c18e7532bcfc5d2c8cafcf 100644
>--- a/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp
>+++ b/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp
>@@ -322,15 +322,15 @@ void JSDOMWindow::heapSnapshot(JSCell* cell, HeapSnapshotBuilder& builder)
> template <CrossOriginObject objectType>
> static void addCrossOriginPropertyNames(VM& vm, PropertyNameArray& propertyNames)
> {
>+    auto& builtinNames = static_cast<JSVMClientData*>(vm.clientData)->builtinNames();
>     switch (objectType) {
>     case CrossOriginObject::Location: {
>-        static const Identifier* const properties[] = { &vm.propertyNames->href, &vm.propertyNames->replace };
>+        static const Identifier* const properties[] = { &builtinNames.hrefPublicName(), &vm.propertyNames->replace };
>         for (auto* property : properties)
>             propertyNames.add(*property);
>         break;
>     }
>     case CrossOriginObject::Window: {
>-        auto& builtinNames = static_cast<JSVMClientData*>(vm.clientData)->builtinNames();
>         static const Identifier* const properties[] = {
>             &builtinNames.blurPublicName(), &builtinNames.closePublicName(), &builtinNames.closedPublicName(),
>             &builtinNames.focusPublicName(), &builtinNames.framesPublicName(), &vm.propertyNames->length,
>diff --git a/Source/WebCore/bindings/js/JSLocationCustom.cpp b/Source/WebCore/bindings/js/JSLocationCustom.cpp
>index 11d22a71877ce373c1987fba2b926376c1d61184..b18f64e8ca546d15cd7c94dac38a85ccc44d3597 100644
>--- a/Source/WebCore/bindings/js/JSLocationCustom.cpp
>+++ b/Source/WebCore/bindings/js/JSLocationCustom.cpp
>@@ -28,6 +28,7 @@
> #include "JSDOMExceptionHandling.h"
> #include "JSDOMWindowCustom.h"
> #include "RuntimeApplicationChecks.h"
>+#include "WebCoreJSClientData.h"
> #include <JavaScriptCore/JSFunction.h>
> #include <JavaScriptCore/Lookup.h>
> 
>@@ -60,7 +61,7 @@ static bool getOwnPropertySlotCommon(JSLocation& thisObject, ExecState& state, P
> 
>     // Getting location.href cross origin needs to throw. However, getOwnPropertyDescriptor() needs to return
>     // a descriptor that has a setter but no getter.
>-    if (slot.internalMethodType() == PropertySlot::InternalMethodType::GetOwnProperty && propertyName == vm.propertyNames->href) {
>+    if (slot.internalMethodType() == PropertySlot::InternalMethodType::GetOwnProperty && propertyName == static_cast<JSVMClientData*>(vm.clientData)->builtinNames().hrefPublicName()) {
>         auto* entry = JSLocation::info()->staticPropHashTable->entry(propertyName);
>         CustomGetterSetter* customGetterSetter = CustomGetterSetter::create(vm, nullptr, entry->propertyPutter());
>         slot.setCustomGetterSetter(&thisObject, static_cast<unsigned>(JSC::PropertyAttribute::CustomAccessor | PropertyAttribute::DontEnum), customGetterSetter);
>@@ -105,7 +106,7 @@ static bool putCommon(JSLocation& thisObject, ExecState& state, PropertyName pro
>     // Always allow assigning to the whole location.
>     // However, alllowing assigning of pieces might inadvertently disclose parts of the original location.
>     // So fall through to the access check for those.
>-    if (propertyName == vm.propertyNames->href)
>+    if (propertyName == static_cast<JSVMClientData*>(vm.clientData)->builtinNames().hrefPublicName())
>         return false;
> 
>     // Block access and throw if there is a security error.
>diff --git a/Source/WebCore/bindings/js/WebCoreBuiltinNames.h b/Source/WebCore/bindings/js/WebCoreBuiltinNames.h
>index 183fe8a8858a38f4a8e80969b5bdec5041588bef..1fa6f143da226e4b9e1722df0c474bad02df577d 100644
>--- a/Source/WebCore/bindings/js/WebCoreBuiltinNames.h
>+++ b/Source/WebCore/bindings/js/WebCoreBuiltinNames.h
>@@ -279,6 +279,7 @@ namespace WebCore {
>     macro(getTracks) \
>     macro(getUserMedia) \
>     macro(header) \
>+    macro(href) \
>     macro(indexedDB) \
>     macro(initializeWith) \
>     macro(isDisturbed) \
>diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
>index 74908a7fa06567b5bd8e5dc2c62e2aca801455b4..1c7478908874fe0029f5983020ed763aff23c661 100644
>--- a/LayoutTests/ChangeLog
>+++ b/LayoutTests/ChangeLog
>@@ -1,3 +1,12 @@
>+2019-02-06  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [JSC] PrivateName to PublicName hash table is wasteful
>+        https://bugs.webkit.org/show_bug.cgi?id=194277
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        * streams/readable-byte-stream-controller-expected.txt:
>+
> 2019-02-05  Nikita Vasilyev  <nvasilyev@apple.com>
> 
>         Web Inspector: Fix modify-css-property-race.html test failures
>diff --git a/LayoutTests/streams/readable-byte-stream-controller-expected.txt b/LayoutTests/streams/readable-byte-stream-controller-expected.txt
>index 9003147251442588ae59d489e00b1ad6c5b3d2ed..e34d3126a7125922d5b60e560a1673fab15fb331 100644
>--- a/LayoutTests/streams/readable-byte-stream-controller-expected.txt
>+++ b/LayoutTests/streams/readable-byte-stream-controller-expected.txt
>@@ -14,7 +14,7 @@ PASS Calling close() after calling error() should throw a TypeError
> PASS Calling read() on a reader associated to a controller that has been errored should fail with provided error 
> PASS Calling read() on a reader associated to a controller that has been closed should not be rejected 
> PASS Pending reading promise should be rejected if controller is errored (case where autoAllocateChunkSize is undefined) 
>-FAIL Pending reading promise should be rejected if controller is errored (case where autoAllocateChunkSize is specified) Can't find private variable: @Uint8Array
>+FAIL Pending reading promise should be rejected if controller is errored (case where autoAllocateChunkSize is specified) Can't find private variable: PrivateSymbol.Uint8Array
> PASS Enqueuing a chunk, getting a reader and calling read should result in a promise resolved with said chunk 
> PASS Getting a reader, calling read and enqueuing a chunk should result in the read promise being resolved with said chunk 
> PASS Getting a reader, enqueuing a chunk and finally calling read should result in a promise resolved with said chunk 
>@@ -39,7 +39,7 @@ PASS Calling close() after calling error() should throw a TypeError
> PASS Calling read() on a reader associated to a controller that has been errored should fail with provided error 
> PASS Calling read() on a reader associated to a controller that has been closed should not be rejected 
> PASS Pending reading promise should be rejected if controller is errored (case where autoAllocateChunkSize is undefined) 
>-FAIL Pending reading promise should be rejected if controller is errored (case where autoAllocateChunkSize is specified) Can't find private variable: @Uint8Array
>+FAIL Pending reading promise should be rejected if controller is errored (case where autoAllocateChunkSize is specified) Can't find private variable: PrivateSymbol.Uint8Array
> PASS Enqueuing a chunk, getting a reader and calling read should result in a promise resolved with said chunk 
> PASS Getting a reader, calling read and enqueuing a chunk should result in the read promise being resolved with said chunk 
> PASS Getting a reader, enqueuing a chunk and finally calling read should result in a promise resolved with said chunk 
>diff --git a/JSTests/ChakraCore.yaml b/JSTests/ChakraCore.yaml
>index 6beb273519f9887ba274fb67b470d2bd20110574..cb0ca11801a6a79a275e71aed85a59e00179cea0 100644
>--- a/JSTests/ChakraCore.yaml
>+++ b/JSTests/ChakraCore.yaml
>@@ -1582,7 +1582,7 @@
>   # Appears to be bad test.  JSC early parse exception vs. Chakra runtime exception.
>   cmd: runChakra :skip, "NoException", "blockscope-functionbinding.baseline", []
> - path: ChakraCore/test/es6/letconst_global.js
>-  cmd: runChakra :baseline, "NoException", "letconst_global.baseline-jsc", []
>+  cmd: runChakra :skip, "NoException", "letconst_global.baseline-jsc", []
> - path: ChakraCore/test/es6/letconst_global_shadowing.js
>   cmd: runChakra :baseline, "NoException", "letconst_global_shadowing.baseline-jsc", []
> - path: ChakraCore/test/es6/letconst_global_shadow_builtins.js
>diff --git a/JSTests/ChangeLog b/JSTests/ChangeLog
>index d72732d6fbd832d802c0c62db7ee835a7cfd0de2..9c83c8c268340b588bd033459d5036318663eefb 100644
>--- a/JSTests/ChangeLog
>+++ b/JSTests/ChangeLog
>@@ -1,3 +1,14 @@
>+2019-02-06  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [JSC] PrivateName to PublicName hash table is wasteful
>+        https://bugs.webkit.org/show_bug.cgi?id=194277
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This test depends on the order of JSSegmentedVariableObjects' variables, which is not guaranteed in JSC. Skipped.
>+
>+        * ChakraCore.yaml:
>+
> 2019-02-05  Dominik Infuehr  <dinfuehr@igalia.com>
> 
>         [ARM] Test running out of executable memory

Flags:

msaboff: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 194277: 361158 | 361219 | 361238 | 361253 | 361261 | 361264 | 361266 | 361278 | 361282 | 361284