Changeset 241104 in webkit for trunk/Source/JavaScriptCore/builtins/BuiltinNames.cpp

Timestamp:

Feb 6, 2019, 2:58:00 PM (7 years ago)

Author:

[email protected]

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/JavaScriptCore:

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:

File:

• trunk/Source/JavaScriptCore/builtins/BuiltinNames.cpp (modified) (2 diffs)

trunk/Source/JavaScriptCore/builtins/BuiltinNames.cpp

@@ -39 +39 @@
 #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
@@ -55 +75 @@
     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