Attachment 360881 Details for Bug 194167 – Patch

[patch] Patch

bug-194167-20190201115009.patch (text/plain), 16.68 KB, created by Yusuke Suzuki on 2019-02-01 11:50:09 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2019-02-01 11:50:09 PST

Size: 16.68 KB

patch

obsolete

>Subversion Revision: 240860
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index a9513e25ad9ff1c9a4aa4f6a219f3e766d57fdda..ce88f9d45bfffffe6d92e42900deb8a63c6db6bb 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,45 @@
>+2019-02-01  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [JSC] Unify CodeBlock IsoSubspaces
>+        https://bugs.webkit.org/show_bug.cgi?id=194167
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        When we move CodeBlock into its IsoSubspace, we create IsoSubspaces for each subclass of CodeBlock.
>+        But this is not necessary since,
>+
>+        1. They do not override the classInfo methods.
>+        2. sizeof(ProgramCodeBlock etc.) == sizeof(CodeBlock) since subclasses adds no additional fields.
>+
>+        Creating IsoSubspace for each subclass is costly in terms of memory. Especially, IsoSubspace for
>+        ProgramCodeBlock is. We typically create only one ProgramCodeBlock, and it means the rest of the
>+        MarkedBlock (16KB - sizeof(footer) - sizeof(ProgramCodeBlock)) is just wasted.
>+
>+        This patch unifies these IsoSubspaces into one.
>+
>+        * bytecode/CodeBlock.cpp:
>+        (JSC::CodeBlock::destroy):
>+        * bytecode/CodeBlock.h:
>+        * bytecode/EvalCodeBlock.cpp:
>+        (JSC::EvalCodeBlock::destroy): Deleted.
>+        * bytecode/EvalCodeBlock.h: We drop some utility functions in EvalCodeBlock and use UnlinkedEvalCodeBlock's one directly.
>+        * bytecode/FunctionCodeBlock.cpp:
>+        (JSC::FunctionCodeBlock::destroy): Deleted.
>+        * bytecode/FunctionCodeBlock.h:
>+        * bytecode/GlobalCodeBlock.h:
>+        * bytecode/ModuleProgramCodeBlock.cpp:
>+        (JSC::ModuleProgramCodeBlock::destroy): Deleted.
>+        * bytecode/ModuleProgramCodeBlock.h:
>+        * bytecode/ProgramCodeBlock.cpp:
>+        (JSC::ProgramCodeBlock::destroy): Deleted.
>+        * bytecode/ProgramCodeBlock.h:
>+        * interpreter/Interpreter.cpp:
>+        (JSC::Interpreter::execute):
>+        * runtime/VM.cpp:
>+        (JSC::VM::VM):
>+        * runtime/VM.h:
>+        (JSC::VM::forEachCodeBlockSpace):
>+
> 2019-02-01  Yusuke Suzuki  <ysuzuki@apple.com>
> 
>         Unreviewed, follow-up after r240859
>diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.cpp b/Source/JavaScriptCore/bytecode/CodeBlock.cpp
>index 228ec54221394b8eac0745f1ed909cebdb7cb3c5..cd10a4583a7a50f2ef4853a09da3faf63d59deac 100644
>--- a/Source/JavaScriptCore/bytecode/CodeBlock.cpp
>+++ b/Source/JavaScriptCore/bytecode/CodeBlock.cpp
>@@ -1385,6 +1385,11 @@ void CodeBlock::finalizeUnconditionally(VM&)
>     VM::SpaceAndFinalizerSet::finalizerSetFor(*subspace()).remove(this);
> }
> 
>+void CodeBlock::destroy(JSCell* cell)
>+{
>+    static_cast<CodeBlock*>(cell)->~CodeBlock();
>+}
>+
> void CodeBlock::getICStatusMap(const ConcurrentJSLocker&, ICStatusMap& result)
> {
> #if ENABLE(JIT)
>diff --git a/Source/JavaScriptCore/bytecode/CodeBlock.h b/Source/JavaScriptCore/bytecode/CodeBlock.h
>index 5fd5f07d1fb66e56815112ef0043e63cccd64312..8d171e450d12c0cb3ed86cf0e6d49bdc897a6f8a 100644
>--- a/Source/JavaScriptCore/bytecode/CodeBlock.h
>+++ b/Source/JavaScriptCore/bytecode/CodeBlock.h
>@@ -191,6 +191,7 @@ class CodeBlock : public JSCell {
> 
>     static size_t estimatedSize(JSCell*, VM&);
>     static void visitChildren(JSCell*, SlotVisitor&);
>+    static void destroy(JSCell*);
>     void visitChildren(SlotVisitor&);
>     void finalizeUnconditionally(VM&);
> 
>diff --git a/Source/JavaScriptCore/bytecode/EvalCodeBlock.cpp b/Source/JavaScriptCore/bytecode/EvalCodeBlock.cpp
>index 2e91fc5c34299cdaeb38f7fcd091199df537b4d5..cd837be926f1be61bcbaea67f9572566a1170574 100644
>--- a/Source/JavaScriptCore/bytecode/EvalCodeBlock.cpp
>+++ b/Source/JavaScriptCore/bytecode/EvalCodeBlock.cpp
>@@ -37,9 +37,4 @@ const ClassInfo EvalCodeBlock::s_info = {
>     CREATE_METHOD_TABLE(EvalCodeBlock)
> };
> 
>-void EvalCodeBlock::destroy(JSCell* cell)
>-{
>-    static_cast<EvalCodeBlock*>(cell)->~EvalCodeBlock();
>-}
>-
> } // namespace JSC
>diff --git a/Source/JavaScriptCore/bytecode/EvalCodeBlock.h b/Source/JavaScriptCore/bytecode/EvalCodeBlock.h
>index 6775c362b5d805149dd51ac3210e01d764fc93c5..40bab25139338176bdf3b15cb1a1e49783be025a 100644
>--- a/Source/JavaScriptCore/bytecode/EvalCodeBlock.h
>+++ b/Source/JavaScriptCore/bytecode/EvalCodeBlock.h
>@@ -41,7 +41,7 @@ class EvalCodeBlock final : public GlobalCodeBlock {
>     template<typename>
>     static IsoSubspace* subspaceFor(VM& vm)
>     {
>-        return &vm.evalCodeBlockSpace.space;
>+        return &vm.codeBlockSpace.space;
>     }
> 
>     static EvalCodeBlock* create(VM* vm, CopyParsedBlockTag, EvalCodeBlock& other)
>@@ -67,11 +67,6 @@ class EvalCodeBlock final : public GlobalCodeBlock {
>         return Structure::create(vm, globalObject, prototype, TypeInfo(CodeBlockType, StructureFlags), info());
>     }
> 
>-    const Identifier& variable(unsigned index) { return unlinkedEvalCodeBlock()->variable(index); }
>-    unsigned numVariables() { return unlinkedEvalCodeBlock()->numVariables(); }
>-    const Identifier& functionHoistingCandidate(unsigned index) { return unlinkedEvalCodeBlock()->functionHoistingCandidate(index); }
>-    unsigned numFunctionHoistingCandidates() { return unlinkedEvalCodeBlock()->numFunctionHoistingCandidates(); }
>-    
> private:
>     EvalCodeBlock(VM* vm, Structure* structure, CopyParsedBlockTag, EvalCodeBlock& other)
>         : GlobalCodeBlock(vm, structure, CopyParsedBlock, other)
>@@ -83,11 +78,7 @@ class EvalCodeBlock final : public GlobalCodeBlock {
>         : GlobalCodeBlock(vm, structure, ownerExecutable, unlinkedCodeBlock, scope, WTFMove(sourceProvider), 0, 1)
>     {
>     }
>-    
>-    static void destroy(JSCell*);
>-
>-private:
>-    UnlinkedEvalCodeBlock* unlinkedEvalCodeBlock() const { return jsCast<UnlinkedEvalCodeBlock*>(unlinkedCodeBlock()); }
> };
>+static_assert(sizeof(EvalCodeBlock) == sizeof(CodeBlock), "Subclasses of CodeBlock should be the same size to share IsoSubspace");
> 
> } // namespace JSC
>diff --git a/Source/JavaScriptCore/bytecode/FunctionCodeBlock.cpp b/Source/JavaScriptCore/bytecode/FunctionCodeBlock.cpp
>index e87f50425667904e0cd9d501368fac79d730d1d4..a1dd540887fe86374673654e76eda25019539983 100644
>--- a/Source/JavaScriptCore/bytecode/FunctionCodeBlock.cpp
>+++ b/Source/JavaScriptCore/bytecode/FunctionCodeBlock.cpp
>@@ -37,9 +37,4 @@ const ClassInfo FunctionCodeBlock::s_info = {
>     CREATE_METHOD_TABLE(FunctionCodeBlock)
> };
> 
>-void FunctionCodeBlock::destroy(JSCell* cell)
>-{
>-    static_cast<FunctionCodeBlock*>(cell)->~FunctionCodeBlock();
>-}
>-
> } // namespace JSC
>diff --git a/Source/JavaScriptCore/bytecode/FunctionCodeBlock.h b/Source/JavaScriptCore/bytecode/FunctionCodeBlock.h
>index 6f50422882ffaf55360bd12f325a2d23bfda4827..55540bd0b199ae229dcfbc8a1a14f89f90ceff61 100644
>--- a/Source/JavaScriptCore/bytecode/FunctionCodeBlock.h
>+++ b/Source/JavaScriptCore/bytecode/FunctionCodeBlock.h
>@@ -42,7 +42,7 @@ class FunctionCodeBlock final : public CodeBlock {
>     template<typename>
>     static IsoSubspace* subspaceFor(VM& vm)
>     {
>-        return &vm.functionCodeBlockSpace.space;
>+        return &vm.codeBlockSpace.space;
>     }
> 
>     static FunctionCodeBlock* create(VM* vm, CopyParsedBlockTag, FunctionCodeBlock& other)
>@@ -79,8 +79,7 @@ class FunctionCodeBlock final : public CodeBlock {
>         : CodeBlock(vm, structure, ownerExecutable, unlinkedCodeBlock, scope, WTFMove(sourceProvider), sourceOffset, firstLineColumnOffset)
>     {
>     }
>-    
>-    static void destroy(JSCell*);
> };
>+static_assert(sizeof(FunctionCodeBlock) == sizeof(CodeBlock), "Subclasses of CodeBlock should be the same size to share IsoSubspace");
> 
> } // namespace JSC
>diff --git a/Source/JavaScriptCore/bytecode/GlobalCodeBlock.h b/Source/JavaScriptCore/bytecode/GlobalCodeBlock.h
>index aa29cca33243573c2700e287866e19b9584a2bbd..a7a3067b867c77394c6da5f7d7c2c407bd88f018 100644
>--- a/Source/JavaScriptCore/bytecode/GlobalCodeBlock.h
>+++ b/Source/JavaScriptCore/bytecode/GlobalCodeBlock.h
>@@ -50,5 +50,6 @@ class GlobalCodeBlock : public CodeBlock {
>     {
>     }
> };
>+static_assert(sizeof(GlobalCodeBlock) == sizeof(CodeBlock), "Subclasses of CodeBlock should be the same size to share IsoSubspace");
> 
> } // namespace JSC
>diff --git a/Source/JavaScriptCore/bytecode/ModuleProgramCodeBlock.cpp b/Source/JavaScriptCore/bytecode/ModuleProgramCodeBlock.cpp
>index a9f76a2f6f58e1ce90d995c8236ad679befbe803..0eea45c828ea6ba17d7dd4289e9d4866545f7e48 100644
>--- a/Source/JavaScriptCore/bytecode/ModuleProgramCodeBlock.cpp
>+++ b/Source/JavaScriptCore/bytecode/ModuleProgramCodeBlock.cpp
>@@ -37,9 +37,4 @@ const ClassInfo ModuleProgramCodeBlock::s_info = {
>     CREATE_METHOD_TABLE(ModuleProgramCodeBlock)
> };
> 
>-void ModuleProgramCodeBlock::destroy(JSCell* cell)
>-{
>-    static_cast<ModuleProgramCodeBlock*>(cell)->~ModuleProgramCodeBlock();
>-}
>-
> } // namespace JSC
>diff --git a/Source/JavaScriptCore/bytecode/ModuleProgramCodeBlock.h b/Source/JavaScriptCore/bytecode/ModuleProgramCodeBlock.h
>index ae99f77d5b43682003ce4b057d948c43ae156350..f74784ebdfd34eb41ad99e5a378086eb18bb6899 100644
>--- a/Source/JavaScriptCore/bytecode/ModuleProgramCodeBlock.h
>+++ b/Source/JavaScriptCore/bytecode/ModuleProgramCodeBlock.h
>@@ -42,7 +42,7 @@ class ModuleProgramCodeBlock final : public GlobalCodeBlock {
>     template<typename>
>     static IsoSubspace* subspaceFor(VM& vm)
>     {
>-        return &vm.moduleProgramCodeBlockSpace.space;
>+        return &vm.codeBlockSpace.space;
>     }
> 
>     static ModuleProgramCodeBlock* create(VM* vm, CopyParsedBlockTag, ModuleProgramCodeBlock& other)
>@@ -79,8 +79,7 @@ class ModuleProgramCodeBlock final : public GlobalCodeBlock {
>         : GlobalCodeBlock(vm, structure, ownerExecutable, unlinkedCodeBlock, scope, WTFMove(sourceProvider), 0, firstLineColumnOffset)
>     {
>     }
>-
>-    static void destroy(JSCell*);
> };
>+static_assert(sizeof(ModuleProgramCodeBlock) == sizeof(CodeBlock), "Subclasses of CodeBlock should be the same size to share IsoSubspace");
> 
> } // namespace JSC
>diff --git a/Source/JavaScriptCore/bytecode/ProgramCodeBlock.cpp b/Source/JavaScriptCore/bytecode/ProgramCodeBlock.cpp
>index 748554e0c9e1904c8825b28fb5fdffcf52ac5eee..602a86114648ef5fd30eff3e43b6dc5a9fd16d28 100644
>--- a/Source/JavaScriptCore/bytecode/ProgramCodeBlock.cpp
>+++ b/Source/JavaScriptCore/bytecode/ProgramCodeBlock.cpp
>@@ -37,9 +37,4 @@ const ClassInfo ProgramCodeBlock::s_info = {
>     CREATE_METHOD_TABLE(ProgramCodeBlock)
> };
> 
>-void ProgramCodeBlock::destroy(JSCell* cell)
>-{
>-    static_cast<ProgramCodeBlock*>(cell)->~ProgramCodeBlock();
>-}
>-
> } // namespace JSC
>diff --git a/Source/JavaScriptCore/bytecode/ProgramCodeBlock.h b/Source/JavaScriptCore/bytecode/ProgramCodeBlock.h
>index 8ea9b4caa04439831ec17b1acdbf8c8d1f84e8ae..be89b6df495b7f3e25ed3457450703b073b0fa80 100644
>--- a/Source/JavaScriptCore/bytecode/ProgramCodeBlock.h
>+++ b/Source/JavaScriptCore/bytecode/ProgramCodeBlock.h
>@@ -42,7 +42,7 @@ class ProgramCodeBlock final : public GlobalCodeBlock {
>     template<typename>
>     static IsoSubspace* subspaceFor(VM& vm)
>     {
>-        return &vm.programCodeBlockSpace.space;
>+        return &vm.codeBlockSpace.space;
>     }
> 
>     static ProgramCodeBlock* create(VM* vm, CopyParsedBlockTag, ProgramCodeBlock& other)
>@@ -79,8 +79,7 @@ class ProgramCodeBlock final : public GlobalCodeBlock {
>         : GlobalCodeBlock(vm, structure, ownerExecutable, unlinkedCodeBlock, scope, WTFMove(sourceProvider), 0, firstLineColumnOffset)
>     {
>     }
>-
>-    static void destroy(JSCell*);
> };
>+static_assert(sizeof(ProgramCodeBlock) == sizeof(CodeBlock), "Subclasses of CodeBlock should be the same size to share IsoSubspace");
> 
> } // namespace JSC
>diff --git a/Source/JavaScriptCore/interpreter/Interpreter.cpp b/Source/JavaScriptCore/interpreter/Interpreter.cpp
>index 8e8256028b1037db2b307acd17d05b7f358f47d0..0cabe8446aae62059b145a24d85af5d9121d1aba 100644
>--- a/Source/JavaScriptCore/interpreter/Interpreter.cpp
>+++ b/Source/JavaScriptCore/interpreter/Interpreter.cpp
>@@ -1044,12 +1044,14 @@ JSValue Interpreter::execute(EvalExecutable* eval, CallFrame* callFrame, JSValue
>             return checkedReturn(compileError);
>         codeBlock = jsCast<EvalCodeBlock*>(tempCodeBlock);
>     }
>+    UnlinkedEvalCodeBlock* unlinkedCodeBlock = jsDynamicCast<UnlinkedEvalCodeBlock*>(vm, codeBlock->unlinkedCodeBlock());
>+    RELEASE_ASSERT(unlinkedCodeBlock);
> 
>     // We can't declare a "var"/"function" that overwrites a global "let"/"const"/"class" in a sloppy-mode eval.
>     if (variableObject->isGlobalObject() && !eval->isStrictMode() && (numVariables || numTopLevelFunctionDecls)) {
>         JSGlobalLexicalEnvironment* globalLexicalEnvironment = jsCast<JSGlobalObject*>(variableObject)->globalLexicalEnvironment();
>         for (unsigned i = 0; i < numVariables; ++i) {
>-            const Identifier& ident = codeBlock->variable(i);
>+            const Identifier& ident = unlinkedCodeBlock->variable(i);
>             PropertySlot slot(globalLexicalEnvironment, PropertySlot::InternalMethodType::VMInquiry);
>             if (JSGlobalLexicalEnvironment::getOwnPropertySlot(globalLexicalEnvironment, callFrame, ident, slot)) {
>                 return checkedReturn(throwTypeError(callFrame, throwScope, makeString("Can't create duplicate global variable in eval: '", String(ident.impl()), "'")));
>@@ -1074,7 +1076,7 @@ JSValue Interpreter::execute(EvalExecutable* eval, CallFrame* callFrame, JSValue
>             variableObject->globalObject(vm)->varInjectionWatchpoint()->fireAll(vm, "Executed eval, fired VarInjection watchpoint");
> 
>         for (unsigned i = 0; i < numVariables; ++i) {
>-            const Identifier& ident = codeBlock->variable(i);
>+            const Identifier& ident = unlinkedCodeBlock->variable(i);
>             bool hasProperty = variableObject->hasProperty(callFrame, ident);
>             RETURN_IF_EXCEPTION(throwScope, checkedReturn(throwScope.exception()));
>             if (!hasProperty) {
>@@ -1108,7 +1110,7 @@ JSValue Interpreter::execute(EvalExecutable* eval, CallFrame* callFrame, JSValue
>             }
> 
>             for (unsigned i = 0; i < numFunctionHoistingCandidates; ++i) {
>-                const Identifier& ident = codeBlock->functionHoistingCandidate(i);
>+                const Identifier& ident = unlinkedCodeBlock->functionHoistingCandidate(i);
>                 JSValue resolvedScope = JSScope::resolveScopeForHoistingFuncDeclInEval(callFrame, scope, ident);
>                 RETURN_IF_EXCEPTION(throwScope, checkedReturn(throwScope.exception()));
>                 if (!resolvedScope.isUndefined()) {
>diff --git a/Source/JavaScriptCore/runtime/VM.cpp b/Source/JavaScriptCore/runtime/VM.cpp
>index cb400a17f65301aae0a56d4fafc73f23046cfd80..fa6a3fbd79861a4aec01a59fa6ca9db8e91a265d 100644
>--- a/Source/JavaScriptCore/runtime/VM.cpp
>+++ b/Source/JavaScriptCore/runtime/VM.cpp
>@@ -317,10 +317,7 @@ VM::VM(VMType vmType, HeapType heapType)
>     , executableToCodeBlockEdgesWithConstraints(executableToCodeBlockEdgeSpace)
>     , executableToCodeBlockEdgesWithFinalizers(executableToCodeBlockEdgeSpace)
>     , inferredValuesWithFinalizers(inferredValueSpace)
>-    , evalCodeBlockSpace ISO_SUBSPACE_INIT(heap, destructibleCellHeapCellType.get(), EvalCodeBlock)
>-    , functionCodeBlockSpace ISO_SUBSPACE_INIT(heap, destructibleCellHeapCellType.get(), FunctionCodeBlock)
>-    , moduleProgramCodeBlockSpace ISO_SUBSPACE_INIT(heap, destructibleCellHeapCellType.get(), ModuleProgramCodeBlock)
>-    , programCodeBlockSpace ISO_SUBSPACE_INIT(heap, destructibleCellHeapCellType.get(), ProgramCodeBlock)
>+    , codeBlockSpace ISO_SUBSPACE_INIT(heap, destructibleCellHeapCellType.get(), CodeBlock)
>     , directEvalExecutableSpace ISO_SUBSPACE_INIT(heap, destructibleCellHeapCellType.get(), DirectEvalExecutable)
>     , functionExecutableSpace ISO_SUBSPACE_INIT(heap, destructibleCellHeapCellType.get(), FunctionExecutable)
>     , indirectEvalExecutableSpace ISO_SUBSPACE_INIT(heap, destructibleCellHeapCellType.get(), IndirectEvalExecutable)
>diff --git a/Source/JavaScriptCore/runtime/VM.h b/Source/JavaScriptCore/runtime/VM.h
>index b435e028d43f955092d83b47b82bf8afa0ad118c..4d6f2c615c2662d642350d463d09ee638799de25 100644
>--- a/Source/JavaScriptCore/runtime/VM.h
>+++ b/Source/JavaScriptCore/runtime/VM.h
>@@ -418,20 +418,14 @@ class VM : public ThreadSafeRefCounted<VM>, public DoublyLinkedListNode<VM> {
>         }
>     };
>     
>-    SpaceAndFinalizerSet evalCodeBlockSpace;
>-    SpaceAndFinalizerSet functionCodeBlockSpace;
>-    SpaceAndFinalizerSet moduleProgramCodeBlockSpace;
>-    SpaceAndFinalizerSet programCodeBlockSpace;
>+    SpaceAndFinalizerSet codeBlockSpace;
> 
>     template<typename Func>
>     void forEachCodeBlockSpace(const Func& func)
>     {
>         // This should not include webAssemblyCodeBlockSpace because this is about subsclasses of
>         // JSC::CodeBlock.
>-        func(evalCodeBlockSpace);
>-        func(functionCodeBlockSpace);
>-        func(moduleProgramCodeBlockSpace);
>-        func(programCodeBlockSpace);
>+        func(codeBlockSpace);
>     }
> 
>     struct ScriptExecutableSpaceAndSet {

Flags:

saam: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 194167: 360881