Changeset 242722 in webkit

Timestamp:

Mar 11, 2019, 11:59:23 AM (7 years ago)

Author:

ysuzuki@apple.com

Message:

[JSC] BuiltinExecutables should behave like a WeakSet instead of generic WeakHandleOwner for memory footprint
​https://bugs.webkit.org/show_bug.cgi?id=195508

Reviewed by Darin Adler.

Weak<> is not cheap in terms of memory footprint. We allocate WeakBlock (256 bytes) for book-keeping Weak<>.
Currently BuiltinExecutables has 203 Weak<> members and many WeakBlocks are actually allocated because
many UnlinkedFunctionExecutables in BuiltinExecutables are allocated during JSGlobalObject initialization process.

This patch changes two things in BuiltinExecutables.

1. Previously we have m_xxxSourceCode fields too. But we do not need to keep it since we know how to produce it when it is required. We generate SourceCode in xxxSourceCode() method instead of just returning m_xxxSourceCode. This reduces sizeof(BuiltinExecutables) 24 x 203 = 4KB.

2. Instead of using Weak<>, BuiltinExecutables holds raw array of UnlinkedFunctionExecutable*. And Heap::finalizeUnconditionalFinalizers() correctly clears dead executables. This is similar to JSWeakSet implementation. And it saves WeakBlock allocations.

• builtins/BuiltinExecutables.cpp:

(JSC::BuiltinExecutables::BuiltinExecutables):
(JSC::BuiltinExecutables::finalizeUnconditionally):
(JSC::JSC_FOREACH_BUILTIN_CODE): Deleted.
(JSC::BuiltinExecutables::finalize): Deleted.

• builtins/BuiltinExecutables.h:

(JSC::BuiltinExecutables::static_cast<unsigned>):
(): Deleted.

• heap/Heap.cpp:

(JSC::Heap::finalizeUnconditionalFinalizers):

Location:

trunk/Source/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• builtins/BuiltinExecutables.cpp (modified) (2 diffs)
• builtins/BuiltinExecutables.h (modified) (4 diffs)
• heap/Heap.cpp (modified) (2 diffs)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2019-03-08  Yusuke Suzuki  <ysuzuki@apple.com>
+
+        [JSC] BuiltinExecutables should behave like a WeakSet instead of generic WeakHandleOwner for memory footprint
+        https://bugs.webkit.org/show_bug.cgi?id=195508
+
+        Reviewed by Darin Adler.
+
+        Weak<> is not cheap in terms of memory footprint. We allocate WeakBlock (256 bytes) for book-keeping Weak<>.
+        Currently BuiltinExecutables has 203 Weak<> members and many WeakBlocks are actually allocated because
+        many UnlinkedFunctionExecutables in BuiltinExecutables are allocated during JSGlobalObject initialization process.
+
+        This patch changes two things in BuiltinExecutables.
+
+        1. Previously we have m_xxxSourceCode fields too. But we do not need to keep it since we know how to produce it when it is required.
+           We generate SourceCode in xxxSourceCode() method instead of just returning m_xxxSourceCode. This reduces sizeof(BuiltinExecutables) 24 x 203 = 4KB.
+
+        2. Instead of using Weak<>, BuiltinExecutables holds raw array of UnlinkedFunctionExecutable*. And Heap::finalizeUnconditionalFinalizers() correctly clears dead executables.
+           This is similar to JSWeakSet implementation. And it saves WeakBlock allocations.
+
+        * builtins/BuiltinExecutables.cpp:
+        (JSC::BuiltinExecutables::BuiltinExecutables):
+        (JSC::BuiltinExecutables::finalizeUnconditionally):
+        (JSC::JSC_FOREACH_BUILTIN_CODE): Deleted.
+        (JSC::BuiltinExecutables::finalize): Deleted.
+        * builtins/BuiltinExecutables.h:
+        (JSC::BuiltinExecutables::static_cast<unsigned>):
+        (): Deleted.
+        * heap/Heap.cpp:
+        (JSC::Heap::finalizeUnconditionalFinalizers):
+
 2019-03-11  Robin Morisset  <rmorisset@apple.com>
 

trunk/Source/JavaScriptCore/builtins/BuiltinExecutables.cpp

@@ -38 +38 @@
     : m_vm(vm)
     , m_combinedSourceProvider(StringSourceProvider::create(StringImpl::createFromLiteral(s_JSCCombinedCode, s_JSCCombinedCodeLength), { }, URL()))
-#define INITIALIZE_BUILTIN_SOURCE_MEMBERS(name, functionName, overrideName, length) , m_##name##Source(m_combinedSourceProvider.copyRef(), s_##name - s_JSCCombinedCode, (s_##name - s_JSCCombinedCode) + length, 1, 1)
-    JSC_FOREACH_BUILTIN_CODE(INITIALIZE_BUILTIN_SOURCE_MEMBERS)
-#undef INITIALIZE_BUILTIN_SOURCE_MEMBERS
 {
 }
@@ -263 +260 @@
 }
 
-void BuiltinExecutables::finalize(Handle<Unknown>, void* context)
-{
-    static_cast<Weak<UnlinkedFunctionExecutable>*>(context)->clear();
+void BuiltinExecutables::finalizeUnconditionally()
+{
+    for (auto*& unlinkedExecutable : m_unlinkedExecutables) {
+        if (unlinkedExecutable && !Heap::isMarked(unlinkedExecutable))
+            unlinkedExecutable = nullptr;
+    }
 }
 
 #define DEFINE_BUILTIN_EXECUTABLES(name, functionName, overrideName, length) \
+SourceCode BuiltinExecutables::name##Source() \
+{\
+    return SourceCode { m_combinedSourceProvider.copyRef(), static_cast<int>(s_##name - s_JSCCombinedCode), static_cast<int>((s_##name - s_JSCCombinedCode) + length), 1, 1 };\
+}\
+\
 UnlinkedFunctionExecutable* BuiltinExecutables::name##Executable() \
 {\
-    if (!m_##name##Executable) {\
+    unsigned index = static_cast<unsigned>(BuiltinCodeIndex::name);\
+    if (!m_unlinkedExecutables[index]) {\
         Identifier executableName = m_vm.propertyNames->builtinNames().functionName##PublicName();\
         if (overrideName)\
             executableName = Identifier::fromString(&m_vm, overrideName);\
-        m_##name##Executable = Weak<UnlinkedFunctionExecutable>(createBuiltinExecutable(m_##name##Source, executableName, s_##name##ConstructAbility), this, &m_##name##Executable);\
+        m_unlinkedExecutables[index] = createBuiltinExecutable(name##Source(), executableName, s_##name##ConstructAbility);\
     }\
-    return m_##name##Executable.get();\
+    return m_unlinkedExecutables[index];\
 }
 JSC_FOREACH_BUILTIN_CODE(DEFINE_BUILTIN_EXECUTABLES)
-#undef EXPOSE_BUILTIN_SOURCES
-
-}
+#undef DEFINE_BUILTIN_EXECUTABLES
+
+}

trunk/Source/JavaScriptCore/builtins/BuiltinExecutables.h

@@ -38 +38 @@
 class VM;
 
-class BuiltinExecutables final: private WeakHandleOwner {
+#define BUILTIN_NAME_ONLY(name, functionName, overriddenName, length) name,
+enum class BuiltinCodeIndex {
+    JSC_FOREACH_BUILTIN_CODE(BUILTIN_NAME_ONLY)
+    NumberOfBuiltinCodes
+};
+#undef BUILTIN_NAME_ONLY
+
+class BuiltinExecutables {
     WTF_MAKE_FAST_ALLOCATED;
 public:
@@ -45 +52 @@
 #define EXPOSE_BUILTIN_EXECUTABLES(name, functionName, overriddenName, length) \
 UnlinkedFunctionExecutable* name##Executable(); \
-const SourceCode& name##Source() { return m_##name##Source; }
+SourceCode name##Source();
     
     JSC_FOREACH_BUILTIN_CODE(EXPOSE_BUILTIN_EXECUTABLES)
@@ -54 +61 @@
 
     static UnlinkedFunctionExecutable* createExecutable(VM&, const SourceCode&, const Identifier&, ConstructorKind, ConstructAbility);
+
+    void finalizeUnconditionally();
+
 private:
-    void finalize(Handle<Unknown>, void* context) override;
-
     VM& m_vm;
 
@@ -62 +70 @@
 
     Ref<StringSourceProvider> m_combinedSourceProvider;
-#define DECLARE_BUILTIN_SOURCE_MEMBERS(name, functionName, overriddenName, length)\
-    SourceCode m_##name##Source; \
-    Weak<UnlinkedFunctionExecutable> m_##name##Executable;
-    JSC_FOREACH_BUILTIN_CODE(DECLARE_BUILTIN_SOURCE_MEMBERS)
-#undef DECLARE_BUILTIN_SOURCE_MEMBERS
+    UnlinkedFunctionExecutable* m_unlinkedExecutables[static_cast<unsigned>(BuiltinCodeIndex::NumberOfBuiltinCodes)] { };
 };
 

trunk/Source/JavaScriptCore/heap/Heap.cpp

@@ -23 +23 @@
 
 #include "BlockDirectoryInlines.h"
+#include "BuiltinExecutables.h"
 #include "CodeBlock.h"
 #include "CodeBlockSetInlines.h"
@@ -560 +561 @@
 void Heap::finalizeUnconditionalFinalizers()
 {
+    vm()->builtinExecutables()->finalizeUnconditionally();
     if (vm()->m_inferredValueSpace)
         finalizeMarkedUnconditionalFinalizers<InferredValue>(vm()->m_inferredValueSpace->space);