Attachment 339894 Details for Bug 185453 – the patch

[patch] the patch

blah.patch (text/plain), 3.42 KB, created by Filip Pizlo on 2018-05-08 16:13:57 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: Filip Pizlo

Created: 2018-05-08 16:13:57 PDT

Size: 3.42 KB

patch

obsolete

>Index: Source/JavaScriptCore/ChangeLog
>===================================================================
>--- Source/JavaScriptCore/ChangeLog	(revision 231516)
>+++ Source/JavaScriptCore/ChangeLog	(working copy)
>@@ -1,3 +1,17 @@
>+2018-05-08  Filip Pizlo  <fpizlo@apple.com>
>+
>+        DFG::FlowMap::resize() shouldn't resize the shadow map unless we're in SSA
>+        https://bugs.webkit.org/show_bug.cgi?id=185453
>+
>+        Reviewed by NOBODY (OOPS!).
>+        
>+        Tiny improvement for compile times.
>+
>+        * dfg/DFGFlowMap.h:
>+        (JSC::DFG::FlowMap::resize): Remove one Vector::resize() when we're not in SSA.
>+        * dfg/DFGInPlaceAbstractState.cpp:
>+        (JSC::DFG::InPlaceAbstractState::beginBasicBlock): Record some data about how long we spend in different parts of this and add a FIXME linking bug 185452.
>+
> 2018-05-08  Eric Carlson  <eric.carlson@apple.com>
> 
>         Consecutive messages logged as JSON are coalesced
>Index: Source/JavaScriptCore/dfg/DFGFlowMap.h
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGFlowMap.h	(revision 231513)
>+++ Source/JavaScriptCore/dfg/DFGFlowMap.h	(working copy)
>@@ -50,7 +50,8 @@ public:
>     void resize()
>     {
>         m_map.resize(m_graph.maxNodeCount());
>-        m_shadowMap.resize(m_graph.maxNodeCount());
>+        if (m_graph.m_form == SSA)
>+            m_shadowMap.resize(m_graph.maxNodeCount());
>     }
>     
>     Graph& graph() const { return m_graph; }
>Index: Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp
>===================================================================
>--- Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp	(revision 231514)
>+++ Source/JavaScriptCore/dfg/DFGInPlaceAbstractState.cpp	(working copy)
>@@ -34,6 +34,7 @@
> #include "JSCInlines.h"
> #include "PutByIdStatus.h"
> #include "StringObject.h"
>+#include "SuperSampler.h"
> 
> namespace JSC { namespace DFG {
> 
>@@ -53,17 +54,23 @@ InPlaceAbstractState::~InPlaceAbstractSt
> 
> void InPlaceAbstractState::beginBasicBlock(BasicBlock* basicBlock)
> {
>+    // This function is ~1.6-2% of execution time.
>+    
>     ASSERT(!m_block);
>     
>     ASSERT(basicBlock->variablesAtHead.numberOfLocals() == basicBlock->valuesAtHead.numberOfLocals());
>     ASSERT(basicBlock->variablesAtTail.numberOfLocals() == basicBlock->valuesAtTail.numberOfLocals());
>     ASSERT(basicBlock->variablesAtHead.numberOfLocals() == basicBlock->variablesAtTail.numberOfLocals());
> 
>-    m_abstractValues.resize();
>+    m_abstractValues.resize(); // This part is ~0.1-0.4% of execution time.
> 
>     AbstractValueClobberEpoch epoch = AbstractValueClobberEpoch::first(basicBlock->cfaStructureClobberStateAtHead);
>     m_effectEpoch = epoch;
>-    
>+
>+    // This loop is 0.9-1.2% of execution time.
>+    // FIXME: Lazily populate m_variables when GetLocal/SetLocal happens. Apply the same idea to
>+    // merging. Alternatively, we could just use liveness here.
>+    // https://bugs.webkit.org/show_bug.cgi?id=185452
>     for (size_t i = m_variables.size(); i--;) {
>         AbstractValue& value = m_variables[i];
>         value = basicBlock->valuesAtHead[i];
>@@ -71,6 +78,7 @@ void InPlaceAbstractState::beginBasicBlo
>     }
>     
>     if (m_graph.m_form == SSA) {
>+        // This loop is 0.05-0.17% of execution time.
>         for (NodeAbstractValuePair& entry : basicBlock->ssa->valuesAtHead) {
>             if (entry.node.isStillValid()) {
>                 AbstractValue& value = m_abstractValues.at(entry.node);

Flags:

msaboff: review+

Actions: View | Formatted Diff | Diff

Attachments on bug 185453: 339894