Title: [241104] trunk
Revision
241104
Author
ysuz...@apple.com
Date
2019-02-06 14:58:00 -0800 (Wed, 06 Feb 2019)

Log Message

[JSC] PrivateName to PublicName hash table is wasteful
https://bugs.webkit.org/show_bug.cgi?id=194277

Reviewed by Michael Saboff.

JSTests:

This test depends on the order of JSSegmentedVariableObjects' variables, which is not guaranteed in JSC. Skipped.

* ChakraCore.yaml:

Source/_javascript_Core:

PrivateNames account for a lot of memory in the initial JSC footprint. BuiltinNames have Identifier fields corresponding to these PrivateNames
which makes the sizeof(BuiltinNames) 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):

Source/WebCore:

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:

LayoutTests:

* streams/readable-byte-stream-controller-expected.txt:

Modified Paths

Diff

Modified: trunk/JSTests/ChakraCore.yaml (241103 => 241104)


--- trunk/JSTests/ChakraCore.yaml	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/JSTests/ChakraCore.yaml	2019-02-06 22:58:00 UTC (rev 241104)
@@ -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

Modified: trunk/JSTests/ChangeLog (241103 => 241104)


--- trunk/JSTests/ChangeLog	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/JSTests/ChangeLog	2019-02-06 22:58:00 UTC (rev 241104)
@@ -1,3 +1,14 @@
+2019-02-06  Yusuke Suzuki  <ysuz...@apple.com>
+
+        [JSC] PrivateName to PublicName hash table is wasteful
+        https://bugs.webkit.org/show_bug.cgi?id=194277
+
+        Reviewed by Michael Saboff.
+
+        This test depends on the order of JSSegmentedVariableObjects' variables, which is not guaranteed in JSC. Skipped.
+
+        * ChakraCore.yaml:
+
 2019-02-05  Dominik Infuehr  <dinfu...@igalia.com>
 
         [ARM] Test running out of executable memory

Modified: trunk/LayoutTests/ChangeLog (241103 => 241104)


--- trunk/LayoutTests/ChangeLog	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/LayoutTests/ChangeLog	2019-02-06 22:58:00 UTC (rev 241104)
@@ -1,3 +1,12 @@
+2019-02-06  Yusuke Suzuki  <ysuz...@apple.com>
+
+        [JSC] PrivateName to PublicName hash table is wasteful
+        https://bugs.webkit.org/show_bug.cgi?id=194277
+
+        Reviewed by Michael Saboff.
+
+        * streams/readable-byte-stream-controller-expected.txt:
+
 2019-02-06  Justin Fan  <justin_...@apple.com>
 
         [Web GPU] Implement supporting dictionaries for GPUTexture

Modified: trunk/LayoutTests/streams/readable-byte-stream-controller-expected.txt (241103 => 241104)


--- trunk/LayoutTests/streams/readable-byte-stream-controller-expected.txt	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/LayoutTests/streams/readable-byte-stream-controller-expected.txt	2019-02-06 22:58:00 UTC (rev 241104)
@@ -14,7 +14,7 @@
 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 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 

Modified: trunk/Source/_javascript_Core/ChangeLog (241103 => 241104)


--- trunk/Source/_javascript_Core/ChangeLog	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/Source/_javascript_Core/ChangeLog	2019-02-06 22:58:00 UTC (rev 241104)
@@ -1,3 +1,74 @@
+2019-02-06  Yusuke Suzuki  <ysuz...@apple.com>
+
+        [JSC] PrivateName to PublicName hash table is wasteful
+        https://bugs.webkit.org/show_bug.cgi?id=194277
+
+        Reviewed by Michael Saboff.
+
+        PrivateNames account for a lot of memory in the initial JSC footprint. BuiltinNames have Identifier fields corresponding to these PrivateNames
+        which makes the sizeof(BuiltinNames) 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-06  Keith Rollin  <krol...@apple.com>
 
         Really enable the automatic checking and regenerations of .xcfilelists during builds

Modified: trunk/Source/_javascript_Core/builtins/BuiltinNames.cpp (241103 => 241104)


--- trunk/Source/_javascript_Core/builtins/BuiltinNames.cpp	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/Source/_javascript_Core/builtins/BuiltinNames.cpp	2019-02-06 22:58:00 UTC (rev 241104)
@@ -38,16 +38,36 @@
 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)
@@ -54,21 +74,22 @@
     : 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)

Modified: trunk/Source/_javascript_Core/builtins/BuiltinNames.h (241103 => 241104)


--- trunk/Source/_javascript_Core/builtins/BuiltinNames.h	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/Source/_javascript_Core/builtins/BuiltinNames.h	2019-02-06 22:58:00 UTC (rev 241104)
@@ -32,12 +32,15 @@
 
 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) \
     JSC_COMMON_BYTECODE_INTRINSIC_CONSTANTS_EACH_NAME(macro) \
@@ -192,28 +195,19 @@
     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;
     
@@ -220,13 +214,13 @@
 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; }
@@ -233,43 +227,63 @@
     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

Modified: trunk/Source/_javascript_Core/builtins/BuiltinUtils.h (241103 => 241104)


--- trunk/Source/_javascript_Core/builtins/BuiltinUtils.h	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/Source/_javascript_Core/builtins/BuiltinUtils.h	2019-02-06 22:58:00 UTC (rev 241104)
@@ -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; } \

Modified: trunk/Source/_javascript_Core/bytecode/BytecodeDumper.cpp (241103 => 241104)


--- trunk/Source/_javascript_Core/bytecode/BytecodeDumper.cpp	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/Source/_javascript_Core/bytecode/BytecodeDumper.cpp	2019-02-06 22:58:00 UTC (rev 241104)
@@ -104,7 +104,7 @@
         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);
     }

Modified: trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp (241103 => 241104)


--- trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/Source/_javascript_Core/bytecompiler/NodesCodegen.cpp	2019-02-06 22:58:00 UTC (rev 241104)
@@ -967,10 +967,10 @@
     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 @@
     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)

Modified: trunk/Source/_javascript_Core/parser/Lexer.cpp (241103 => 241104)


--- trunk/Source/_javascript_Core/parser/Lexer.cpp	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/Source/_javascript_Core/parser/Lexer.cpp	2019-02-06 22:58:00 UTC (rev 241104)
@@ -975,9 +975,9 @@
                 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 @@
                 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;
         }

Modified: trunk/Source/_javascript_Core/parser/Parser.cpp (241103 => 241104)


--- trunk/Source/_javascript_Core/parser/Parser.cpp	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/Source/_javascript_Core/parser/Parser.cpp	2019-02-06 22:58:00 UTC (rev 241104)
@@ -2284,15 +2284,15 @@
     };
 
     // @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 @@
         //
         // 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 @@
         //
         // 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 @@
         //
         // 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 @@
         }
 
         if (!localName)
-            localName = &m_vm->propertyNames->builtinNames().starDefaultPrivateName();
+            localName = &m_vm->propertyNames->starDefaultPrivateName;
 
         if (isFunctionOrClassDeclaration) {
             if (startsWithFunction) {
@@ -3439,11 +3439,11 @@
             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 @@
                 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");

Modified: trunk/Source/_javascript_Core/parser/ParserArena.h (241103 => 241104)


--- trunk/Source/_javascript_Core/parser/ParserArena.h	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/Source/_javascript_Core/parser/ParserArena.h	2019-02-06 22:58:00 UTC (rev 241104)
@@ -47,6 +47,7 @@
         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 @@
         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;

Modified: trunk/Source/_javascript_Core/runtime/CachedTypes.cpp (241103 => 241104)


--- trunk/Source/_javascript_Core/runtime/CachedTypes.cpp	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/Source/_javascript_Core/runtime/CachedTypes.cpp	2019-02-06 22:58:00 UTC (rev 241104)
@@ -534,12 +534,14 @@
     {
         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 @@
                 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);

Modified: trunk/Source/_javascript_Core/runtime/CommonIdentifiers.cpp (241103 => 241104)


--- trunk/Source/_javascript_Core/runtime/CommonIdentifiers.cpp	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/Source/_javascript_Core/runtime/CommonIdentifiers.cpp	2019-02-06 22:58:00 UTC (rev 241104)
@@ -41,6 +41,7 @@
     , 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,14 +52,14 @@
 {
 }
 
-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)

Modified: trunk/Source/_javascript_Core/runtime/CommonIdentifiers.h (241103 => 241104)


--- trunk/Source/_javascript_Core/runtime/CommonIdentifiers.h	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/Source/_javascript_Core/runtime/CommonIdentifiers.h	2019-02-06 22:58:00 UTC (rev 241104)
@@ -28,27 +28,19 @@
 #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) \
@@ -55,22 +47,12 @@
     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 @@
         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 @@
         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.

Modified: trunk/Source/_javascript_Core/runtime/ExceptionHelpers.cpp (241103 => 241104)


--- trunk/Source/_javascript_Core/runtime/ExceptionHelpers.cpp	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/Source/_javascript_Core/runtime/ExceptionHelpers.cpp	2019-02-06 22:58:00 UTC (rev 241104)
@@ -82,8 +82,7 @@
 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()));

Modified: trunk/Source/_javascript_Core/runtime/Identifier.cpp (241103 => 241104)


--- trunk/Source/_javascript_Core/runtime/Identifier.cpp	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/Source/_javascript_Core/runtime/Identifier.cpp	2019-02-06 22:58:00 UTC (rev 241104)
@@ -96,9 +96,14 @@
 
 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>");
 }
 

Modified: trunk/Source/_javascript_Core/runtime/Identifier.h (241103 => 241104)


--- trunk/Source/_javascript_Core/runtime/Identifier.h	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/Source/_javascript_Core/runtime/Identifier.h	2019-02-06 22:58:00 UTC (rev 241104)
@@ -126,6 +126,7 @@
     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)); }
 

Modified: trunk/Source/_javascript_Core/runtime/IdentifierInlines.h (241103 => 241104)


--- trunk/Source/_javascript_Core/runtime/IdentifierInlines.h	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/Source/_javascript_Core/runtime/IdentifierInlines.h	2019-02-06 22:58:00 UTC (rev 241104)
@@ -88,6 +88,11 @@
     return name.uid();
 }
 
+inline Identifier Identifier::fromUid(SymbolImpl& symbol)
+{
+    return symbol;
+}
+
 template<unsigned charactersCount>
 inline Identifier Identifier::fromString(VM* vm, const char (&characters)[charactersCount])
 {

Modified: trunk/Source/_javascript_Core/runtime/JSTypedArrayViewPrototype.cpp (241103 => 241104)


--- trunk/Source/_javascript_Core/runtime/JSTypedArrayViewPrototype.cpp	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/Source/_javascript_Core/runtime/JSTypedArrayViewPrototype.cpp	2019-02-06 22:58:00 UTC (rev 241104)
@@ -297,7 +297,7 @@
 
     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);

Modified: trunk/Source/_javascript_Core/tools/JSDollarVM.cpp (241103 => 241104)


--- trunk/Source/_javascript_Core/tools/JSDollarVM.cpp	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/Source/_javascript_Core/tools/JSDollarVM.cpp	2019-02-06 22:58:00 UTC (rev 241104)
@@ -1854,11 +1854,11 @@
 
     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)

Modified: trunk/Source/WebCore/ChangeLog (241103 => 241104)


--- trunk/Source/WebCore/ChangeLog	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/Source/WebCore/ChangeLog	2019-02-06 22:58:00 UTC (rev 241104)
@@ -1,3 +1,19 @@
+2019-02-06  Yusuke Suzuki  <ysuz...@apple.com>
+
+        [JSC] PrivateName to PublicName hash table is wasteful
+        https://bugs.webkit.org/show_bug.cgi?id=194277
+
+        Reviewed by Michael Saboff.
+
+        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-06  Keith Rollin  <krol...@apple.com>
 
         Really enable the automatic checking and regenerations of .xcfilelists during builds

Modified: trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp (241103 => 241104)


--- trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/Source/WebCore/bindings/js/JSDOMWindowCustom.cpp	2019-02-06 22:58:00 UTC (rev 241104)
@@ -322,15 +322,15 @@
 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,

Modified: trunk/Source/WebCore/bindings/js/JSLocationCustom.cpp (241103 => 241104)


--- trunk/Source/WebCore/bindings/js/JSLocationCustom.cpp	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/Source/WebCore/bindings/js/JSLocationCustom.cpp	2019-02-06 22:58:00 UTC (rev 241104)
@@ -28,6 +28,7 @@
 #include "JSDOMExceptionHandling.h"
 #include "JSDOMWindowCustom.h"
 #include "RuntimeApplicationChecks.h"
+#include "WebCoreJSClientData.h"
 #include <_javascript_Core/JSFunction.h>
 #include <_javascript_Core/Lookup.h>
 
@@ -60,7 +61,7 @@
 
     // 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 @@
     // 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.

Modified: trunk/Source/WebCore/bindings/js/WebCoreBuiltinNames.h (241103 => 241104)


--- trunk/Source/WebCore/bindings/js/WebCoreBuiltinNames.h	2019-02-06 22:54:54 UTC (rev 241103)
+++ trunk/Source/WebCore/bindings/js/WebCoreBuiltinNames.h	2019-02-06 22:58:00 UTC (rev 241104)
@@ -280,6 +280,7 @@
     macro(getTracks) \
     macro(getUserMedia) \
     macro(header) \
+    macro(href) \
     macro(indexedDB) \
     macro(initializeWith) \
     macro(isDisturbed) \
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to