Attachment 360424 Details for Bug 193546 – Patch

[patch] Patch

bug-193546-20190128193511.patch (text/plain), 16.63 KB, created by Yusuke Suzuki on 2019-01-28 19:35:12 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Yusuke Suzuki

Created: 2019-01-28 19:35:12 PST

Size: 16.63 KB

patch

obsolete

>Subversion Revision: 240629
>diff --git a/Source/JavaScriptCore/ChangeLog b/Source/JavaScriptCore/ChangeLog
>index c2a70f5e1964408947d4fe00b2e7a83010690756..93bee91fd6345bef9c3f1aed50fb46541b33cb7f 100644
>--- a/Source/JavaScriptCore/ChangeLog
>+++ b/Source/JavaScriptCore/ChangeLog
>@@ -1,3 +1,46 @@
>+2019-01-28  Yusuke Suzuki  <ysuzuki@apple.com>
>+
>+        [JSC] Reduce size of memory used for ShadowChicken
>+        https://bugs.webkit.org/show_bug.cgi?id=193546
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        This patch lazily instantiate ShadowChicken. We do not need this until we start logging ShadowChicken packets.
>+        The removal of ShadowChicken saves 55KB memory.
>+
>+        * debugger/DebuggerCallFrame.cpp:
>+        (JSC::DebuggerCallFrame::create):
>+        * ftl/FTLLowerDFGToB3.cpp:
>+        (JSC::FTL::DFG::LowerDFGToB3::ensureShadowChickenPacket):
>+        * heap/Heap.cpp:
>+        (JSC::Heap::stopThePeriphery):
>+        (JSC::Heap::addCoreConstraints):
>+        * jit/CCallHelpers.cpp:
>+        (JSC::CCallHelpers::ensureShadowChickenPacket):
>+        * jit/JITExceptions.cpp:
>+        (JSC::genericUnwind):
>+        * jit/JITOpcodes.cpp:
>+        (JSC::JIT::emit_op_log_shadow_chicken_prologue):
>+        (JSC::JIT::emit_op_log_shadow_chicken_tail):
>+        * jit/JITOpcodes32_64.cpp:
>+        (JSC::JIT::emit_op_log_shadow_chicken_prologue):
>+        (JSC::JIT::emit_op_log_shadow_chicken_tail):
>+        * jit/JITOperations.cpp:
>+        * llint/LLIntSlowPaths.cpp:
>+        (JSC::LLInt::LLINT_SLOW_PATH_DECL):
>+        * runtime/JSGlobalObject.cpp:
>+        (JSC::JSGlobalObject::setDebugger):
>+        * runtime/JSGlobalObject.h:
>+        (JSC::JSGlobalObject::setDebugger): Deleted.
>+        * runtime/VM.cpp:
>+        (JSC::VM::VM):
>+        (JSC::VM::ensureShadowChicken):
>+        * runtime/VM.h:
>+        (JSC::VM::shadowChicken):
>+        * tools/JSDollarVM.cpp:
>+        (JSC::functionShadowChickenFunctionsOnStack):
>+        (JSC::changeDebuggerModeWhenIdle):
>+
> 2019-01-28  Mark Lam  <mark.lam@apple.com>
> 
>         ToString node actually does GC.
>diff --git a/Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp b/Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp
>index 630f8f0deac042f471de956bc748c0a19bcef67c..b30a8625d23ee069c6747072573c727f5eb2d22a 100644
>--- a/Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp
>+++ b/Source/JavaScriptCore/debugger/DebuggerCallFrame.cpp
>@@ -70,7 +70,8 @@ Ref<DebuggerCallFrame> DebuggerCallFrame::create(VM& vm, CallFrame* callFrame)
>     }
> 
>     Vector<ShadowChicken::Frame> frames;
>-    vm.shadowChicken().iterate(vm, callFrame, [&] (const ShadowChicken::Frame& frame) -> bool {
>+    vm.ensureShadowChicken();
>+    vm.shadowChicken()->iterate(vm, callFrame, [&] (const ShadowChicken::Frame& frame) -> bool {
>         frames.append(frame);
>         return true;
>     });
>diff --git a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>index d7618ddc05c83121d262f8c2ebfc109c94aeb5e6..4f625fe02a326b54f52f703193d1936a32709682 100644
>--- a/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>+++ b/Source/JavaScriptCore/ftl/FTLLowerDFGToB3.cpp
>@@ -13469,16 +13469,17 @@ class LowerDFGToB3 {
>     
>     LValue ensureShadowChickenPacket()
>     {
>+        RELEASE_ASSERT(vm().shadowChicken());
>         LBasicBlock slowCase = m_out.newBlock();
>         LBasicBlock continuation = m_out.newBlock();
>         
>-        TypedPointer addressOfLogCursor = m_out.absolute(vm().shadowChicken().addressOfLogCursor());
>+        TypedPointer addressOfLogCursor = m_out.absolute(vm().shadowChicken()->addressOfLogCursor());
>         LValue logCursor = m_out.loadPtr(addressOfLogCursor);
>         
>         ValueFromBlock fastResult = m_out.anchor(logCursor);
>         
>         m_out.branch(
>-            m_out.below(logCursor, m_out.constIntPtr(vm().shadowChicken().logEnd())),
>+            m_out.below(logCursor, m_out.constIntPtr(vm().shadowChicken()->logEnd())),
>             usually(continuation), rarely(slowCase));
>         
>         LBasicBlock lastNext = m_out.appendTo(slowCase, continuation);
>diff --git a/Source/JavaScriptCore/heap/Heap.cpp b/Source/JavaScriptCore/heap/Heap.cpp
>index 8da70dd04411a36242389b3f2a83a87bd94694de..06ea3b62e6436440323b3a9dd3c2debe8b002e7b 100644
>--- a/Source/JavaScriptCore/heap/Heap.cpp
>+++ b/Source/JavaScriptCore/heap/Heap.cpp
>@@ -1588,7 +1588,8 @@ void Heap::stopThePeriphery(GCConductor conn)
> #endif // ENABLE(JIT)
>     UNUSED_PARAM(conn);
>     
>-    vm()->shadowChicken().update(*vm(), vm()->topCallFrame);
>+    if (auto* shadowChicken = vm()->shadowChicken())
>+        shadowChicken->update(*vm(), vm()->topCallFrame);
>     
>     m_structureIDTable.flushOldTables();
>     m_objectSpace.stopAllocating();
>@@ -2699,7 +2700,8 @@ void Heap::addCoreConstraints()
>             if (m_vm->typeProfiler())
>                 m_vm->typeProfilerLog()->visit(slotVisitor);
>             
>-            m_vm->shadowChicken().visitChildren(slotVisitor);
>+            if (auto* shadowChicken = m_vm->shadowChicken())
>+                shadowChicken->visitChildren(slotVisitor);
>         },
>         ConstraintVolatility::GreyedByExecution);
>     
>diff --git a/Source/JavaScriptCore/jit/CCallHelpers.cpp b/Source/JavaScriptCore/jit/CCallHelpers.cpp
>index 74d740f62dd1d313fd32d76d8a4d019fcada0797..062171996b73bc92b7752236a7fa09b8cb6db641 100644
>--- a/Source/JavaScriptCore/jit/CCallHelpers.cpp
>+++ b/Source/JavaScriptCore/jit/CCallHelpers.cpp
>@@ -54,14 +54,15 @@ void CCallHelpers::logShadowChickenTailPacket(GPRReg shadowPacket, JSValueRegs t
> 
> void CCallHelpers::ensureShadowChickenPacket(VM& vm, GPRReg shadowPacket, GPRReg scratch1NonArgGPR, GPRReg scratch2)
> {
>+    RELEASE_ASSERT(vm.shadowChicken());
>     ASSERT(!RegisterSet::argumentGPRS().get(scratch1NonArgGPR));
>-    move(TrustedImmPtr(vm.shadowChicken().addressOfLogCursor()), scratch1NonArgGPR);
>+    move(TrustedImmPtr(vm.shadowChicken()->addressOfLogCursor()), scratch1NonArgGPR);
>     loadPtr(Address(scratch1NonArgGPR), shadowPacket);
>-    Jump ok = branchPtr(Below, shadowPacket, TrustedImmPtr(vm.shadowChicken().logEnd()));
>+    Jump ok = branchPtr(Below, shadowPacket, TrustedImmPtr(vm.shadowChicken()->logEnd()));
>     setupArguments<decltype(operationProcessShadowChickenLog)>();
>     move(TrustedImmPtr(tagCFunctionPtr<OperationPtrTag>(operationProcessShadowChickenLog)), scratch1NonArgGPR);
>     call(scratch1NonArgGPR, OperationPtrTag);
>-    move(TrustedImmPtr(vm.shadowChicken().addressOfLogCursor()), scratch1NonArgGPR);
>+    move(TrustedImmPtr(vm.shadowChicken()->addressOfLogCursor()), scratch1NonArgGPR);
>     loadPtr(Address(scratch1NonArgGPR), shadowPacket);
>     ok.link(this);
>     addPtr(TrustedImm32(sizeof(ShadowChicken::Packet)), shadowPacket, scratch2);
>diff --git a/Source/JavaScriptCore/jit/JITExceptions.cpp b/Source/JavaScriptCore/jit/JITExceptions.cpp
>index 3aaa87fc1a756da6bf86296fc8c2d3b3ae3113ee..7fb225b17199d37b35991a7affb9e92cb2e91e72 100644
>--- a/Source/JavaScriptCore/jit/JITExceptions.cpp
>+++ b/Source/JavaScriptCore/jit/JITExceptions.cpp
>@@ -53,7 +53,8 @@ void genericUnwind(VM* vm, ExecState* callFrame)
>         CRASH();
>     }
>     
>-    vm->shadowChicken().log(*vm, topJSCallFrame, ShadowChicken::Packet::throwPacket());
>+    if (auto* shadowChicken = vm->shadowChicken())
>+        shadowChicken->log(*vm, topJSCallFrame, ShadowChicken::Packet::throwPacket());
> 
>     Exception* exception = scope.exception();
>     RELEASE_ASSERT(exception);
>diff --git a/Source/JavaScriptCore/jit/JITOpcodes.cpp b/Source/JavaScriptCore/jit/JITOpcodes.cpp
>index c4dced921dc0986da200a216a081ad74ab8bc32a..378dc505f7007b5207a2115a856002d54cf11cb7 100644
>--- a/Source/JavaScriptCore/jit/JITOpcodes.cpp
>+++ b/Source/JavaScriptCore/jit/JITOpcodes.cpp
>@@ -1460,6 +1460,7 @@ void JIT::emit_op_profile_type(const Instruction* currentInstruction)
> 
> void JIT::emit_op_log_shadow_chicken_prologue(const Instruction* currentInstruction)
> {
>+    RELEASE_ASSERT(vm()->shadowChicken());
>     updateTopCallFrame();
>     static_assert(nonArgGPR0 != regT0 && nonArgGPR0 != regT2, "we will have problems if this is true.");
>     auto bytecode = currentInstruction->as<OpLogShadowChickenPrologue>();
>@@ -1473,6 +1474,7 @@ void JIT::emit_op_log_shadow_chicken_prologue(const Instruction* currentInstruct
> 
> void JIT::emit_op_log_shadow_chicken_tail(const Instruction* currentInstruction)
> {
>+    RELEASE_ASSERT(vm()->shadowChicken());
>     updateTopCallFrame();
>     static_assert(nonArgGPR0 != regT0 && nonArgGPR0 != regT2, "we will have problems if this is true.");
>     auto bytecode = currentInstruction->as<OpLogShadowChickenTail>();
>diff --git a/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp b/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp
>index 6462f6d928ac5c41aa970d974bc491456eb2a8d5..f36cd4339dee03f75ed10df196948e2317570f69 100644
>--- a/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp
>+++ b/Source/JavaScriptCore/jit/JITOpcodes32_64.cpp
>@@ -1335,6 +1335,7 @@ void JIT::emit_op_profile_type(const Instruction* currentInstruction)
> 
> void JIT::emit_op_log_shadow_chicken_prologue(const Instruction* currentInstruction)
> {
>+    RELEASE_ASSERT(vm()->shadowChicken());
>     updateTopCallFrame();
>     static_assert(nonArgGPR0 != regT0 && nonArgGPR0 != regT2, "we will have problems if this is true.");
>     auto bytecode = currentInstruction->as<OpLogShadowChickenPrologue>();
>@@ -1350,6 +1351,7 @@ void JIT::emit_op_log_shadow_chicken_prologue(const Instruction* currentInstruct
> 
> void JIT::emit_op_log_shadow_chicken_tail(const Instruction* currentInstruction)
> {
>+    RELEASE_ASSERT(vm()->shadowChicken());
>     updateTopCallFrame();
>     static_assert(nonArgGPR0 != regT0 && nonArgGPR0 != regT2, "we will have problems if this is true.");
>     auto bytecode = currentInstruction->as<OpLogShadowChickenTail>();
>diff --git a/Source/JavaScriptCore/jit/JITOperations.cpp b/Source/JavaScriptCore/jit/JITOperations.cpp
>index 95bfb81c6584b17927675c472445727f760200db..6292db20c73b420bb2b6bebfa6b082ab0b34454d 100644
>--- a/Source/JavaScriptCore/jit/JITOperations.cpp
>+++ b/Source/JavaScriptCore/jit/JITOperations.cpp
>@@ -2882,7 +2882,8 @@ void JIT_OPERATION operationProcessShadowChickenLog(ExecState* exec)
> {
>     VM& vm = exec->vm();
>     NativeCallFrameTracer tracer(&vm, exec);
>-    vm.shadowChicken().update(vm, exec);
>+    RELEASE_ASSERT(vm.shadowChicken());
>+    vm.shadowChicken()->update(vm, exec);
> }
> 
> int32_t JIT_OPERATION operationCheckIfExceptionIsUncatchableAndNotifyProfiler(ExecState* exec)
>diff --git a/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp b/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
>index 0facee6cf163240c09fb41c43d54d2e3d123a050..5c7de1d5e466afea3cec578d7c28d04bd251bd75 100644
>--- a/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
>+++ b/Source/JavaScriptCore/llint/LLIntSlowPaths.cpp
>@@ -1891,7 +1891,8 @@ LLINT_SLOW_PATH_DECL(slow_path_log_shadow_chicken_prologue)
>     
>     auto bytecode = pc->as<OpLogShadowChickenPrologue>();
>     JSScope* scope = exec->uncheckedR(bytecode.m_scope).Register::scope();
>-    vm.shadowChicken().log(vm, exec, ShadowChicken::Packet::prologue(exec->jsCallee(), exec, exec->callerFrame(), scope));
>+    RELEASE_ASSERT(vm.shadowChicken());
>+    vm.shadowChicken()->log(vm, exec, ShadowChicken::Packet::prologue(exec->jsCallee(), exec, exec->callerFrame(), scope));
>     
>     LLINT_END();
> }
>@@ -1909,7 +1910,8 @@ LLINT_SLOW_PATH_DECL(slow_path_log_shadow_chicken_tail)
> #else
>     CallSiteIndex callSiteIndex(pc);
> #endif
>-    vm.shadowChicken().log(vm, exec, ShadowChicken::Packet::tail(exec, thisValue, scope, exec->codeBlock(), callSiteIndex));
>+    RELEASE_ASSERT(vm.shadowChicken());
>+    vm.shadowChicken()->log(vm, exec, ShadowChicken::Packet::tail(exec, thisValue, scope, exec->codeBlock(), callSiteIndex));
>     
>     LLINT_END();
> }
>diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
>index 676ec0ee4a3199dd9b6711eb9abc5dbfeab22808..019bf9b0128281cd1a5a42ce072637aef242807b 100644
>--- a/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
>+++ b/Source/JavaScriptCore/runtime/JSGlobalObject.cpp
>@@ -1897,6 +1897,13 @@ void JSGlobalObject::queueMicrotask(Ref<Microtask>&& task)
>     vm().queueMicrotask(*this, WTFMove(task));
> }
> 
>+void JSGlobalObject::setDebugger(Debugger* debugger)
>+{
>+    m_debugger = debugger;
>+    if (debugger)
>+        vm().ensureShadowChicken();
>+}
>+
> bool JSGlobalObject::hasDebugger() const
> { 
>     return m_debugger;
>diff --git a/Source/JavaScriptCore/runtime/JSGlobalObject.h b/Source/JavaScriptCore/runtime/JSGlobalObject.h
>index 60c24262d181091f5de24957804c7f138ef2c9a7..c3ab151fc701e27bcec5de872c1a2bf52a51f165 100644
>--- a/Source/JavaScriptCore/runtime/JSGlobalObject.h
>+++ b/Source/JavaScriptCore/runtime/JSGlobalObject.h
>@@ -914,7 +914,7 @@ class JSGlobalObject : public JSSegmentedVariableObject {
>     }
> 
>     Debugger* debugger() const { return m_debugger; }
>-    void setDebugger(Debugger* debugger) { m_debugger = debugger; }
>+    void setDebugger(Debugger*);
> 
>     const GlobalObjectMethodTable* globalObjectMethodTable() const { return m_globalObjectMethodTable; }
> 
>diff --git a/Source/JavaScriptCore/runtime/VM.cpp b/Source/JavaScriptCore/runtime/VM.cpp
>index 0a640b2ef41d855eb7277b18e4fca854150face6..5f4aa78c67212737adca12ac0b0d21ded8ce37e1 100644
>--- a/Source/JavaScriptCore/runtime/VM.cpp
>+++ b/Source/JavaScriptCore/runtime/VM.cpp
>@@ -360,7 +360,6 @@ VM::VM(VMType vmType, HeapType heapType)
>     , m_typeProfilerEnabledCount(0)
>     , m_primitiveGigacageEnabled(IsWatched)
>     , m_controlFlowProfilerEnabledCount(0)
>-    , m_shadowChicken(std::make_unique<ShadowChicken>())
> {
>     interpreter = new Interpreter(*this);
>     StackBounds stack = Thread::current().stack();
>@@ -509,6 +508,9 @@ VM::VM(VMType vmType, HeapType heapType)
>     if (!canUseJIT())
>         noJITValueProfileSingleton = std::make_unique<ValueProfile>(0);
> 
>+    if (Options::forceDebuggerBytecodeGeneration() || Options::alwaysUseShadowChicken())
>+        ensureShadowChicken();
>+
>     VMInspector::instance().add(this);
> }
> 
>@@ -1239,6 +1241,13 @@ void VM::clearScratchBuffers()
>         scratchBuffer->setActiveLength(0);
> }
> 
>+void VM::ensureShadowChicken()
>+{
>+    if (m_shadowChicken)
>+        return;
>+    m_shadowChicken = std::make_unique<ShadowChicken>();
>+}
>+
> JSGlobalObject* VM::vmEntryGlobalObject(const CallFrame* callFrame) const
> {
>     if (callFrame && callFrame->isGlobalExec()) {
>diff --git a/Source/JavaScriptCore/runtime/VM.h b/Source/JavaScriptCore/runtime/VM.h
>index df5886d7240196421174349aef0a3d9877492fcd..5839e207fe375d24d9c526090dc3bd6ba2055c93 100644
>--- a/Source/JavaScriptCore/runtime/VM.h
>+++ b/Source/JavaScriptCore/runtime/VM.h
>@@ -857,7 +857,8 @@ class VM : public ThreadSafeRefCounted<VM>, public DoublyLinkedListNode<VM> {
> 
>     BytecodeIntrinsicRegistry& bytecodeIntrinsicRegistry() { return *m_bytecodeIntrinsicRegistry; }
>     
>-    ShadowChicken& shadowChicken() { return *m_shadowChicken; }
>+    ShadowChicken* shadowChicken() { return m_shadowChicken.get(); }
>+    void ensureShadowChicken();
>     
>     template<typename Func>
>     void logEvent(CodeBlock*, const char* summary, const Func& func);
>diff --git a/Source/JavaScriptCore/tools/JSDollarVM.cpp b/Source/JavaScriptCore/tools/JSDollarVM.cpp
>index 53c5c46fee0f47fabeadd859eed2d131ca31fd5e..67782eb2577816dd54d3dbd4528948b7ab2bbae1 100644
>--- a/Source/JavaScriptCore/tools/JSDollarVM.cpp
>+++ b/Source/JavaScriptCore/tools/JSDollarVM.cpp
>@@ -1932,7 +1932,22 @@ static EncodedJSValue JSC_HOST_CALL functionSetHiddenValue(ExecState* exec)
> static EncodedJSValue JSC_HOST_CALL functionShadowChickenFunctionsOnStack(ExecState* exec)
> {
>     VM& vm = exec->vm();
>-    return JSValue::encode(vm.shadowChicken().functionsOnStack(exec));
>+    auto scope = DECLARE_THROW_SCOPE(vm);
>+    if (auto* shadowChicken = vm.shadowChicken())
>+        return JSValue::encode(shadowChicken->functionsOnStack(exec));
>+
>+    JSArray* result = constructEmptyArray(exec, 0);
>+    RETURN_IF_EXCEPTION(scope, { });
>+    StackVisitor::visit(exec, &vm, [&] (StackVisitor& visitor) -> StackVisitor::Status {
>+        if (visitor->isInlinedFrame())
>+            return StackVisitor::Continue;
>+        if (visitor->isWasmFrame())
>+            return StackVisitor::Continue;
>+        result->push(exec, jsCast<JSObject*>(visitor->callee().asCell()));
>+        scope.releaseAssertNoException(); // This function is only called from tests.
>+        return StackVisitor::Continue;
>+    });
>+    return JSValue::encode(result);
> }
> 
> static EncodedJSValue JSC_HOST_CALL functionSetGlobalConstRedeclarationShouldNotThrow(ExecState* exec)
>@@ -2047,6 +2062,7 @@ static EncodedJSValue changeDebuggerModeWhenIdle(ExecState* exec, DebuggerMode m
>     vm->whenIdle([=] () {
>         Options::forceDebuggerBytecodeGeneration() = newDebuggerMode;
>         vm->deleteAllCode(PreventCollectionAndDeleteAllCode);
>+        vm->ensureShadowChicken();
>     });
>     return JSValue::encode(jsUndefined());
> }

Actions: View | Formatted Diff | Diff

Attachments on bug 193546: 360424 | 361495