[JSC] BuiltinExecutables should behave like a WeakSet instead of generic WeakHandleOw...
authorysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 11 Mar 2019 18:59:23 +0000 (18:59 +0000)
committerysuzuki@apple.com <ysuzuki@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 11 Mar 2019 18:59:23 +0000 (18:59 +0000)
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):

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

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/builtins/BuiltinExecutables.cpp
Source/JavaScriptCore/builtins/BuiltinExecutables.h
Source/JavaScriptCore/heap/Heap.cpp

index 864763a..e16e450 100644 (file)
@@ -1,3 +1,33 @@
+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>
 
         IntlDateTimeFormat can be shrunk by 32 bytes
index 2d6fe0e..84baccc 100644 (file)
@@ -37,9 +37,6 @@ namespace JSC {
 BuiltinExecutables::BuiltinExecutables(VM& vm)
     : 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
 {
 }
 
@@ -262,23 +259,32 @@ UnlinkedFunctionExecutable* BuiltinExecutables::createExecutable(VM& vm, const S
     return functionExecutable;
 }
 
-void BuiltinExecutables::finalize(Handle<Unknown>, void* context)
+void BuiltinExecutables::finalizeUnconditionally()
 {
-    static_cast<Weak<UnlinkedFunctionExecutable>*>(context)->clear();
+    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
 
 }
index 6dcbedd..aa4433c 100644 (file)
@@ -37,14 +37,21 @@ class UnlinkedFunctionExecutable;
 class Identifier;
 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:
     explicit BuiltinExecutables(VM&);
 
 #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)
 #undef EXPOSE_BUILTIN_EXECUTABLES
@@ -53,19 +60,16 @@ const SourceCode& name##Source() { return m_##name##Source; }
     UnlinkedFunctionExecutable* createDefaultConstructor(ConstructorKind, const Identifier& name);
 
     static UnlinkedFunctionExecutable* createExecutable(VM&, const SourceCode&, const Identifier&, ConstructorKind, ConstructAbility);
-private:
-    void finalize(Handle<Unknown>, void* context) override;
 
+    void finalizeUnconditionally();
+
+private:
     VM& m_vm;
 
     UnlinkedFunctionExecutable* createBuiltinExecutable(const SourceCode&, const Identifier&, ConstructAbility);
 
     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)] { };
 };
 
 }
index 094a785..230711e 100644 (file)
@@ -22,6 +22,7 @@
 #include "Heap.h"
 
 #include "BlockDirectoryInlines.h"
+#include "BuiltinExecutables.h"
 #include "CodeBlock.h"
 #include "CodeBlockSetInlines.h"
 #include "CollectingScope.h"
@@ -559,6 +560,7 @@ void Heap::finalizeMarkedUnconditionalFinalizers(CellSet& cellSet)
 
 void Heap::finalizeUnconditionalFinalizers()
 {
+    vm()->builtinExecutables()->finalizeUnconditionally();
     if (vm()->m_inferredValueSpace)
         finalizeMarkedUnconditionalFinalizers<InferredValue>(vm()->m_inferredValueSpace->space);
     vm()->forEachCodeBlockSpace(