Attachment 368605 Details for Bug 197304 – patch

[patch] patch

b-backup.diff (text/plain), 6.70 KB, created by Saam Barati on 2019-04-30 13:46:22 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Saam Barati

Created: 2019-04-30 13:46:22 PDT

Size: 6.70 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 244792)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,31 @@
>+2019-04-30  Saam barati  <sbarati@apple.com>
>+
>+        CodeBlock::m_instructionCount is wrong
>+        https://bugs.webkit.org/show_bug.cgi?id=197304
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Currently this is only used for inlining heuristics, but in the future,
>+        I plan to use it for an actual bounds check. Since this is used to drive
>+        inlining and tier up heuristics, there is a chance it affects performance
>+        on benchmarks. My initial testing of JetStream 2's CLI version shows this
>+        is neutral. 
>+
>+        * bytecode/BytecodeDumper.cpp:
>+        (JSC::BytecodeDumper<Block>::dumpBlock):
>+        * bytecode/CodeBlock.cpp:
>+        (JSC::CodeBlock::CodeBlock):
>+        (JSC::CodeBlock::finishCreation):
>+        * bytecode/CodeBlock.h:
>+        (JSC::CodeBlock::instructionCount const):
>+        * bytecode/InstructionStream.cpp:
>+        (JSC::InstructionStream::sizeInBytes const): Deleted.
>+        * bytecode/InstructionStream.h:
>+        * bytecode/UnlinkedCodeBlock.cpp:
>+        (JSC::UnlinkedCodeBlock::visitChildren):
>+        (JSC::UnlinkedCodeBlock::estimatedSize):
>+        (JSC::UnlinkedCodeBlock::setInstructions):
>+
> 2019-04-30  Brian Burg  <bburg@apple.com>
> 
>         Web Automation: use a more informative key to indicate automation availability
>Index: Source/JavaScriptCore/bytecode/BytecodeDumper.cpp
>===================================================================
>--- Source/JavaScriptCore/bytecode/BytecodeDumper.cpp	(revision 244785)
>+++ Source/JavaScriptCore/bytecode/BytecodeDumper.cpp	(working copy)
>@@ -210,7 +210,7 @@ void BytecodeDumper<Block>::dumpBlock(Bl
>         static_cast<unsigned long>(instructionCount),
>         static_cast<unsigned long>(wideInstructionCount),
>         static_cast<unsigned long>(instructionWithMetadataCount),
>-        static_cast<unsigned long>(instructions.sizeInBytes() + block->metadataSizeInBytes()),
>+        static_cast<unsigned long>(instructions.size() + block->metadataSizeInBytes()),
>         static_cast<unsigned long>(block->metadataSizeInBytes()),
>         block->numParameters(), block->numCalleeLocals(), block->numVars());
>     out.print("; scope at ", block->scopeRegister());
>Index: Source/JavaScriptCore/bytecode/CodeBlock.cpp
>===================================================================
>--- Source/JavaScriptCore/bytecode/CodeBlock.cpp	(revision 244785)
>+++ Source/JavaScriptCore/bytecode/CodeBlock.cpp	(working copy)
>@@ -298,7 +298,6 @@ CodeBlock::CodeBlock(VM* vm, Structure* 
>     , m_hasDebuggerStatement(false)
>     , m_steppingMode(SteppingModeDisabled)
>     , m_numBreakpoints(0)
>-    , m_instructionCount(other.m_instructionCount)
>     , m_scopeRegister(other.m_scopeRegister)
>     , m_hash(other.m_hash)
>     , m_unlinkedCode(*other.vm(), this, other.m_unlinkedCode.get())
>@@ -524,7 +523,6 @@ bool CodeBlock::finishCreation(VM& vm, S
>     const InstructionStream& instructionStream = instructions();
>     for (const auto& instruction : instructionStream) {
>         OpcodeID opcodeID = instruction->opcodeID();
>-        m_instructionCount += opcodeLengths[opcodeID];
>         switch (opcodeID) {
>         LINK(OpHasIndexedProperty, arrayProfile)
> 
>Index: Source/JavaScriptCore/bytecode/CodeBlock.h
>===================================================================
>--- Source/JavaScriptCore/bytecode/CodeBlock.h	(revision 244785)
>+++ Source/JavaScriptCore/bytecode/CodeBlock.h	(working copy)
>@@ -384,7 +384,7 @@ public:
> 
>     size_t predictedMachineCodeSize();
> 
>-    unsigned instructionCount() const { return m_instructionCount; }
>+    unsigned instructionCount() const { return instructions().size(); }
> 
>     // Exactly equivalent to codeBlock->ownerExecutable()->newReplacementCodeBlockFor(codeBlock->specializationKind())
>     CodeBlock* newReplacement();
>@@ -963,7 +963,6 @@ private:
>             unsigned m_numBreakpoints : 30;
>         };
>     };
>-    unsigned m_instructionCount { 0 };
>     VirtualRegister m_scopeRegister;
>     mutable CodeBlockHash m_hash;
> 
>Index: Source/JavaScriptCore/bytecode/InstructionStream.cpp
>===================================================================
>--- Source/JavaScriptCore/bytecode/InstructionStream.cpp	(revision 244785)
>+++ Source/JavaScriptCore/bytecode/InstructionStream.cpp	(working copy)
>@@ -35,11 +35,6 @@ InstructionStream::InstructionStream(Ins
>     : m_instructions(WTFMove(instructions))
> { }
> 
>-size_t InstructionStream::sizeInBytes() const
>-{
>-    return m_instructions.size();
>-}
>-
> bool InstructionStream::contains(Instruction* instruction) const
> {
> 
>Index: Source/JavaScriptCore/bytecode/InstructionStream.h
>===================================================================
>--- Source/JavaScriptCore/bytecode/InstructionStream.h	(revision 244785)
>+++ Source/JavaScriptCore/bytecode/InstructionStream.h	(working copy)
>@@ -41,8 +41,6 @@ class InstructionStream {
>     friend class InstructionStreamWriter;
>     friend class CachedInstructionStream;
> public:
>-    size_t sizeInBytes() const;
>-
>     using Offset = unsigned;
> 
> private:
>Index: Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp
>===================================================================
>--- Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp	(revision 244785)
>+++ Source/JavaScriptCore/bytecode/UnlinkedCodeBlock.cpp	(working copy)
>@@ -95,7 +95,7 @@ void UnlinkedCodeBlock::visitChildren(JS
>     visitor.appendValues(thisObject->m_constantRegisters.data(), thisObject->m_constantRegisters.size());
>     size_t extraMemory = thisObject->m_metadata->sizeInBytes();
>     if (thisObject->m_instructions)
>-        extraMemory += thisObject->m_instructions->sizeInBytes();
>+        extraMemory += thisObject->m_instructions->size();
>     visitor.reportExtraMemoryVisited(extraMemory);
> }
> 
>@@ -104,7 +104,7 @@ size_t UnlinkedCodeBlock::estimatedSize(
>     UnlinkedCodeBlock* thisObject = jsCast<UnlinkedCodeBlock*>(cell);
>     size_t extraSize = thisObject->m_metadata->sizeInBytes();
>     if (thisObject->m_instructions)
>-        extraSize += thisObject->m_instructions->sizeInBytes();
>+        extraSize += thisObject->m_instructions->size();
>     return Base::estimatedSize(cell, vm) + extraSize;
> }
> 
>@@ -313,7 +313,7 @@ void UnlinkedCodeBlock::setInstructions(
>         m_instructions = WTFMove(instructions);
>         m_metadata->finalize();
>     }
>-    Heap::heap(this)->reportExtraMemoryAllocated(m_instructions->sizeInBytes() + m_metadata->sizeInBytes());
>+    Heap::heap(this)->reportExtraMemoryAllocated(m_instructions->size() + m_metadata->sizeInBytes());
> }
> 
> const InstructionStream& UnlinkedCodeBlock::instructions() const

Flags:

ysuzuki: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 197304: 368605 | 368623