Attachment 340164 Details for Bug 185525 – patch for landing

[patch] patch for landing

b-backup.diff (text/plain), 8.45 KB, created by Saam Barati on 2018-05-10 20:42:54 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Saam Barati

Created: 2018-05-10 20:42:54 PDT

Size: 8.45 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 231675)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,36 @@
>+2018-05-10  Saam Barati  <sbarati@apple.com>
>+
>+        Don't allocate value profiles when the JIT is disabled
>+        https://bugs.webkit.org/show_bug.cgi?id=185525
>+
>+        Reviewed by Michael Saboff.
>+
>+        There are many JSC API clients that run with the JIT disabled. We were
>+        still allocating a ton of value profiles in this use case even though
>+        these clients get no benefit from doing value profiling. This patch makes
>+        it so that we don't allocate value profiles or argument value profiles
>+        when we're not using the JIT. We now just make all value profiles in
>+        the instruction stream point to a global value profile that the VM owns.
>+        And we make the argument value profile array have zero length and teach
>+        the LLInt how to handle that. Heap clears the global value profile on each GC.
>+
>+        In an app that I'm testing this against, this saves ~1MB of memory.
>+
>+        * bytecode/CodeBlock.cpp:
>+        (JSC::CodeBlock::finishCreation):
>+        (JSC::CodeBlock::setNumParameters):
>+        * bytecode/CodeBlock.h:
>+        (JSC::CodeBlock::numberOfArgumentValueProfiles):
>+        (JSC::CodeBlock::valueProfileForArgument):
>+        * bytecompiler/BytecodeGenerator.cpp:
>+        (JSC::BytecodeGenerator::emitProfiledOpcode):
>+        * heap/Heap.cpp:
>+        (JSC::Heap::runEndPhase):
>+        * llint/LowLevelInterpreter.asm:
>+        * runtime/VM.cpp:
>+        (JSC::VM::VM):
>+        * runtime/VM.h:
>+
> 2018-05-10  Filip Pizlo  <fpizlo@apple.com>
> 
>         DFG CFA should pick the right time to inject OSR entry data
>Index: Source/JavaScriptCore/bytecode/CodeBlock.cpp
>===================================================================
>--- Source/JavaScriptCore/bytecode/CodeBlock.cpp	(revision 231666)
>+++ Source/JavaScriptCore/bytecode/CodeBlock.cpp	(working copy)
>@@ -506,6 +506,8 @@ bool CodeBlock::finishCreation(VM& vm, S
>         m_arrayAllocationProfiles = RefCountedArray<ArrayAllocationProfile>(size);
>     if (size_t size = unlinkedCodeBlock->numberOfValueProfiles())
>         m_valueProfiles = RefCountedArray<ValueProfile>(size);
>+    if (!vm.canUseJIT())
>+        RELEASE_ASSERT(!m_valueProfiles.size());
>     if (size_t size = unlinkedCodeBlock->numberOfObjectAllocationProfiles())
>         m_objectAllocationProfiles = RefCountedArray<ObjectAllocationProfile>(size);
> 
>@@ -524,6 +526,12 @@ bool CodeBlock::finishCreation(VM& vm, S
> 
>     unsigned valueProfileCount = 0;
>     auto linkValueProfile = [&](unsigned bytecodeOffset, unsigned opLength) {
>+        if (!vm.canUseJIT()) {
>+            ASSERT(vm.noJITValueProfileSingleton);
>+            instructions[bytecodeOffset + opLength - 1] = vm.noJITValueProfileSingleton.get();
>+            return;
>+        }
>+
>         unsigned valueProfileIndex = valueProfileCount++;
>         ValueProfile* profile = &m_valueProfiles[valueProfileIndex];
>         ASSERT(profile->m_bytecodeOffset == -1);
>@@ -967,7 +975,7 @@ void CodeBlock::setNumParameters(int new
> {
>     m_numParameters = newValue;
> 
>-    m_argumentValueProfiles = RefCountedArray<ValueProfile>(newValue);
>+    m_argumentValueProfiles = RefCountedArray<ValueProfile>(vm()->canUseJIT() ? newValue : 0);
> }
> 
> CodeBlock* CodeBlock::specialOSREntryBlockOrNull()
>Index: Source/JavaScriptCore/bytecode/CodeBlock.h
>===================================================================
>--- Source/JavaScriptCore/bytecode/CodeBlock.h	(revision 231666)
>+++ Source/JavaScriptCore/bytecode/CodeBlock.h	(working copy)
>@@ -417,11 +417,12 @@ public:
>     unsigned numberOfArgumentValueProfiles()
>     {
>         ASSERT(m_numParameters >= 0);
>-        ASSERT(m_argumentValueProfiles.size() == static_cast<unsigned>(m_numParameters));
>+        ASSERT(m_argumentValueProfiles.size() == static_cast<unsigned>(m_numParameters) || !vm()->canUseJIT());
>         return m_argumentValueProfiles.size();
>     }
>     ValueProfile& valueProfileForArgument(unsigned argumentIndex)
>     {
>+        ASSERT(vm()->canUseJIT()); // This is only called from the various JIT compilers or places that first check numberOfArgumentValueProfiles before calling this.
>         ValueProfile& result = m_argumentValueProfiles[argumentIndex];
>         ASSERT(result.m_bytecodeOffset == -1);
>         return result;
>Index: Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp
>===================================================================
>--- Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp	(revision 231666)
>+++ Source/JavaScriptCore/bytecompiler/BytecodeGenerator.cpp	(working copy)
>@@ -1294,8 +1294,10 @@ UnlinkedObjectAllocationProfile Bytecode
> 
> UnlinkedValueProfile BytecodeGenerator::emitProfiledOpcode(OpcodeID opcodeID)
> {
>-    UnlinkedValueProfile result = m_codeBlock->addValueProfile();
>     emitOpcode(opcodeID);
>+    if (!m_vm->canUseJIT())
>+        return static_cast<UnlinkedValueProfile>(-1);
>+    UnlinkedValueProfile result = m_codeBlock->addValueProfile();
>     return result;
> }
> 
>Index: Source/JavaScriptCore/heap/Heap.cpp
>===================================================================
>--- Source/JavaScriptCore/heap/Heap.cpp	(revision 231666)
>+++ Source/JavaScriptCore/heap/Heap.cpp	(working copy)
>@@ -1457,6 +1457,9 @@ NEVER_INLINE bool Heap::runEndPhase(GCCo
>         
>     if (vm()->typeProfiler())
>         vm()->typeProfiler()->invalidateTypeSetCache();
>+
>+    if (ValueProfile* profile = vm()->noJITValueProfileSingleton.get())
>+        *profile = ValueProfile(0);
>         
>     reapWeakHandles();
>     pruneStaleEntriesFromWeakGCMaps();
>Index: Source/JavaScriptCore/llint/LowLevelInterpreter.asm
>===================================================================
>--- Source/JavaScriptCore/llint/LowLevelInterpreter.asm	(revision 231666)
>+++ Source/JavaScriptCore/llint/LowLevelInterpreter.asm	(working copy)
>@@ -1119,6 +1119,7 @@ macro functionInitialization(profileArgS
>     assert(macro (ok) bpgteq t0, 0, ok end)
>     btpz t0, .argumentProfileDone
>     loadp CodeBlock::m_argumentValueProfiles + VectorBufferOffset[t1], t3
>+    bpeq 0, t3, .argumentProfileDone # When we can't JIT, we don't allocate any argument value profiles.
>     mulp sizeof ValueProfile, t0, t2 # Aaaaahhhh! Need strength reduction!
>     lshiftp 3, t0
>     addp t2, t3
>Index: Source/JavaScriptCore/runtime/VM.cpp
>===================================================================
>--- Source/JavaScriptCore/runtime/VM.cpp	(revision 231666)
>+++ Source/JavaScriptCore/runtime/VM.cpp	(working copy)
>@@ -490,6 +490,9 @@ VM::VM(VMType vmType, HeapType heapType)
>     }
> #endif
> 
>+    if (!canUseJIT())
>+        noJITValueProfileSingleton = std::make_unique<ValueProfile>(0);
>+
>     VMInspector::instance().add(this);
> }
> 
>Index: Source/JavaScriptCore/runtime/VM.h
>===================================================================
>--- Source/JavaScriptCore/runtime/VM.h	(revision 231666)
>+++ Source/JavaScriptCore/runtime/VM.h	(working copy)
>@@ -171,6 +171,7 @@ class Signature;
> 
> struct HashTable;
> struct Instruction;
>+struct ValueProfile;
> 
> struct LocalTimeOffsetCache {
>     LocalTimeOffsetCache()
>@@ -721,6 +722,8 @@ public:
>     RTTraceList* m_rtTraceList;
> #endif
> 
>+    std::unique_ptr<ValueProfile> noJITValueProfileSingleton;
>+
>     JS_EXPORT_PRIVATE void resetDateCache();
> 
>     RegExpCache* regExpCache() { return m_regExpCache; }
>Index: Source/WTF/ChangeLog
>===================================================================
>--- Source/WTF/ChangeLog	(revision 231666)
>+++ Source/WTF/ChangeLog	(working copy)
>@@ -1,3 +1,13 @@
>+2018-05-10  Saam Barati  <sbarati@apple.com>
>+
>+        Don't allocate value profiles when the JIT is disabled
>+        https://bugs.webkit.org/show_bug.cgi?id=185525
>+
>+        Reviewed by Michael Saboff.
>+
>+        * wtf/RefCountedArray.h:
>+        (WTF::RefCountedArray::RefCountedArray):
>+
> 2018-05-10  Tim Horton  <timothy_horton@apple.com>
> 
>         Fix the build after r231393
>Index: Source/WTF/wtf/RefCountedArray.h
>===================================================================
>--- Source/WTF/wtf/RefCountedArray.h	(revision 231666)
>+++ Source/WTF/wtf/RefCountedArray.h	(working copy)
>@@ -63,6 +63,7 @@ public:
>     explicit RefCountedArray(size_t size)
>     {
>         if (!size) {
>+            // NOTE: JSC's LowLevelInterpreter relies on this being nullptr when the size is zero.
>             PtrTraits::exchange(m_data, nullptr);
>             return;
>         }

Actions: View | Formatted Diff | Diff

Attachments on bug 185525: 340144 | 340162 | 340164