Changeset 199342 in webkit

Timestamp:

Apr 12, 2016 1:25:48 AM (8 years ago)

Author:

Yusuke Suzuki

Message:

[JSC] addStaticGlobals should emit SymbolTableEntry watchpoints to encourage constant folding in DFG
​https://bugs.webkit.org/show_bug.cgi?id=155110

Reviewed by Saam Barati.

Source/JavaScriptCore:

addStaticGlobals does not emit SymbolTableEntry watchpoints for the added entries.
So, all the global variable lookups pointing to these static globals are not converted
into constants in DFGBytecodeGenerator: this fact leaves these lookups as GetGlobalVar.
Such thing avoids constant folding chance and emits CheckCell for @privateFunction inlining.
This operation is pure overhead.

Static globals are not configurable, and they are typically non-writable.
So they are constants in almost all the cases.

This patch initializes watchpoints for these static globals.
These watchpoints allow DFG to convert these nodes into constants in DFG BytecodeParser.
These watchpoints includes many builtin operations and undefined.

The microbenchmark, many-foreach-calls shows 5 - 7% improvement since it removes unnecessary CheckCell.

• bytecode/VariableWriteFireDetail.h:
• runtime/JSGlobalObject.cpp:

(JSC::JSGlobalObject::addGlobalVar):
(JSC::JSGlobalObject::addStaticGlobals):

• runtime/JSSymbolTableObject.h:

(JSC::symbolTablePutTouchWatchpointSet):
(JSC::symbolTablePutInvalidateWatchpointSet):
(JSC::symbolTablePut):
(JSC::symbolTablePutWithAttributesTouchWatchpointSet): Deleted.

• runtime/SymbolTable.h:

(JSC::SymbolTableEntry::SymbolTableEntry):
(JSC::SymbolTableEntry::operator=):
(JSC::SymbolTableEntry::swap):

Source/WebCore:

• bindings/js/JSDOMWindowBase.cpp:

(WebCore::JSDOMWindowBase::updateDocument):

Location:

trunk/Source

Files:

• JavaScriptCore/ChangeLog (modified) (1 diff)
• JavaScriptCore/bytecode/VariableWriteFireDetail.h (modified) (1 diff)
• JavaScriptCore/runtime/JSGlobalObject.cpp (modified) (2 diffs)
• JavaScriptCore/runtime/JSSymbolTableObject.h (modified) (5 diffs)
• JavaScriptCore/runtime/SymbolTable.h (modified) (2 diffs)
• WebCore/ChangeLog (modified) (1 diff)
• WebCore/bindings/js/JSDOMWindowBase.cpp (modified) (1 diff)

trunk/Source/JavaScriptCore/ChangeLog

@@ +1 @@
+2016-04-12  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] addStaticGlobals should emit SymbolTableEntry watchpoints to encourage constant folding in DFG
+        https://bugs.webkit.org/show_bug.cgi?id=155110
+
+        Reviewed by Saam Barati.
+
+        `addStaticGlobals` does not emit SymbolTableEntry watchpoints for the added entries.
+        So, all the global variable lookups pointing to these static globals are not converted
+        into constants in DFGBytecodeGenerator: this fact leaves these lookups as GetGlobalVar.
+        Such thing avoids constant folding chance and emits CheckCell for @privateFunction inlining.
+        This operation is pure overhead.
+
+        Static globals are not configurable, and they are typically non-writable.
+        So they are constants in almost all the cases.
+
+        This patch initializes watchpoints for these static globals.
+        These watchpoints allow DFG to convert these nodes into constants in DFG BytecodeParser.
+        These watchpoints includes many builtin operations and `undefined`.
+
+        The microbenchmark, many-foreach-calls shows 5 - 7% improvement since it removes unnecessary CheckCell.
+
+        * bytecode/VariableWriteFireDetail.h:
+        * runtime/JSGlobalObject.cpp:
+        (JSC::JSGlobalObject::addGlobalVar):
+        (JSC::JSGlobalObject::addStaticGlobals):
+        * runtime/JSSymbolTableObject.h:
+        (JSC::symbolTablePutTouchWatchpointSet):
+        (JSC::symbolTablePutInvalidateWatchpointSet):
+        (JSC::symbolTablePut):
+        (JSC::symbolTablePutWithAttributesTouchWatchpointSet): Deleted.
+        * runtime/SymbolTable.h:
+        (JSC::SymbolTableEntry::SymbolTableEntry):
+        (JSC::SymbolTableEntry::operator=):
+        (JSC::SymbolTableEntry::swap):
+
 2016-04-12  Alex Christensen  <achristensen@webkit.org>
 

trunk/Source/JavaScriptCore/bytecode/VariableWriteFireDetail.h

@@ -42 +42 @@
     }
     
-    void dump(PrintStream&) const override;
+    JS_EXPORT_PRIVATE void dump(PrintStream&) const override;
     
     JS_EXPORT_PRIVATE static void touch(WatchpointSet*, JSObject*, const PropertyName&);

trunk/Source/JavaScriptCore/runtime/JSGlobalObject.cpp

@@ -680 +680 @@
     SymbolTableEntry newEntry(VarOffset(offset), 0);
     newEntry.prepareToWatch();
-    symbolTable()->add(locker, ident.impl(), newEntry);
+    symbolTable()->add(locker, ident.impl(), WTFMove(newEntry));
     
     ScopeOffset offsetForAssert = addVariables(1, jsUndefined());
@@ -980 +980 @@
         ASSERT(global.attributes & DontDelete);
         
-        ScopeOffset offset;
+        WatchpointSet* watchpointSet = nullptr;
+        WriteBarrierBase<Unknown>* variable = nullptr;
         {
             ConcurrentJITLocker locker(symbolTable()->m_lock);
-            offset = symbolTable()->takeNextScopeOffset(locker);
+            ScopeOffset offset = symbolTable()->takeNextScopeOffset(locker);
             RELEASE_ASSERT(offset = startOffset + i);
             SymbolTableEntry newEntry(VarOffset(offset), global.attributes);
-            symbolTable()->add(locker, global.identifier.impl(), newEntry);
+            newEntry.prepareToWatch();
+            watchpointSet = newEntry.watchpointSet();
+            symbolTable()->add(locker, global.identifier.impl(), WTFMove(newEntry));
+            variable = &variableAt(offset);
         }
-        variableAt(offset).set(vm(), this, global.value);
+        symbolTablePutTouchWatchpointSet(vm(), this, global.identifier, global.value, variable, watchpointSet);
     }
 }

trunk/Source/JavaScriptCore/runtime/JSSymbolTableObject.h

@@ -142 +142 @@
 }
 
+template<typename SymbolTableObjectType>
+ALWAYS_INLINE void symbolTablePutTouchWatchpointSet(VM& vm, SymbolTableObjectType* object, PropertyName propertyName, JSValue value, WriteBarrierBase<Unknown>* reg, WatchpointSet* set)
+{
+    reg->set(vm, object, value);
+    if (set)
+        VariableWriteFireDetail::touch(set, object, propertyName);
+}
+
+template<typename SymbolTableObjectType>
+ALWAYS_INLINE void symbolTablePutInvalidateWatchpointSet(VM& vm, SymbolTableObjectType* object, PropertyName propertyName, JSValue value, WriteBarrierBase<Unknown>* reg, WatchpointSet* set)
+{
+    reg->set(vm, object, value);
+    if (set)
+        set->invalidate(VariableWriteFireDetail(object, propertyName)); // Don't mess around - if we had found this statically, we would have invalidated it.
+}
+
 enum class SymbolTablePutMode {
-    WithAttributes,
-    WithoutAttributes
+    Touch,
+    Invalidate
 };
 
 template<SymbolTablePutMode symbolTablePutMode, typename SymbolTableObjectType>
-inline bool symbolTablePut(
-    SymbolTableObjectType* object, ExecState* exec, PropertyName propertyName, JSValue value, unsigned attributes,
-    bool shouldThrowReadOnlyError, bool ignoreReadOnlyErrors, WatchpointSet*& set, bool& putResult)
-{
-    ASSERT(!Heap::heap(value) || Heap::heap(value) == Heap::heap(object));
-
+inline bool symbolTablePut(SymbolTableObjectType* object, ExecState* exec, PropertyName propertyName, JSValue value, bool shouldThrowReadOnlyError, bool ignoreReadOnlyErrors, bool& putResult)
+{
     VM& vm = exec->vm();
 
+    WatchpointSet* set = nullptr;
     WriteBarrierBase<Unknown>* reg;
     {
@@ -182 +195 @@
 
         set = iter->value.watchpointSet();
-        if (symbolTablePutMode == SymbolTablePutMode::WithAttributes)
-            iter->value.setAttributes(attributes);
         reg = &object->variableAt(offset);
     }
@@ -189 +200 @@
     // the right for barriers to be able to trigger GC. And I don't want to hold VM
     // locks while GC'ing.
-    reg->set(vm, object, value);
+    if (symbolTablePutMode == SymbolTablePutMode::Invalidate)
+        symbolTablePutInvalidateWatchpointSet(exec->vm(), object, propertyName, value, reg, set);
+    else
+        symbolTablePutTouchWatchpointSet(exec->vm(), object, propertyName, value, reg, set);
     putResult = true;
     return true;
@@ -199 +213 @@
     bool shouldThrowReadOnlyError, bool ignoreReadOnlyErrors, bool& putResult)
 {
-    WatchpointSet* set = nullptr;
-    unsigned attributes = 0;
-    bool result = symbolTablePut<SymbolTablePutMode::WithoutAttributes>(object, exec, propertyName, value, attributes, shouldThrowReadOnlyError, ignoreReadOnlyErrors, set, putResult);
-    if (set)
-        VariableWriteFireDetail::touch(set, object, propertyName);
-    return result;
+    ASSERT(!Heap::heap(value) || Heap::heap(value) == Heap::heap(object));
+    return symbolTablePut<SymbolTablePutMode::Touch>(object, exec, propertyName, value, shouldThrowReadOnlyError, ignoreReadOnlyErrors, putResult);
 }
 
@@ -212 +222 @@
     bool shouldThrowReadOnlyError, bool ignoreReadOnlyErrors, bool& putResult)
 {
-    WatchpointSet* set = nullptr;
-    unsigned attributes = 0;
-    bool result = symbolTablePut<SymbolTablePutMode::WithoutAttributes>(object, exec, propertyName, value, attributes, shouldThrowReadOnlyError, ignoreReadOnlyErrors, set, putResult);
-    if (set)
-        set->invalidate(VariableWriteFireDetail(object, propertyName)); // Don't mess around - if we had found this statically, we would have invalidated it.
-    return result;
-}
-
-template<typename SymbolTableObjectType>
-inline bool symbolTablePutWithAttributesTouchWatchpointSet(
-    SymbolTableObjectType* object, ExecState* exec, PropertyName propertyName,
-    JSValue value, unsigned attributes, bool& putResult)
-{
-    WatchpointSet* set = nullptr;
-    bool shouldThrowReadOnlyError = false;
-    bool ignoreReadOnlyErrors = true;
-    bool result = symbolTablePut<SymbolTablePutMode::WithAttributes>(object, exec, propertyName, value, attributes, shouldThrowReadOnlyError, ignoreReadOnlyErrors, set, putResult);
-    if (set)
-        VariableWriteFireDetail::touch(set, object, propertyName);
-    return result;
+    ASSERT(!Heap::heap(value) || Heap::heap(value) == Heap::heap(object));
+    return symbolTablePut<SymbolTablePutMode::Invalidate>(object, exec, propertyName, value, shouldThrowReadOnlyError, ignoreReadOnlyErrors, putResult);
 }
 

trunk/Source/JavaScriptCore/runtime/SymbolTable.h

@@ -200 +200 @@
     }
     
+    SymbolTableEntry(SymbolTableEntry&& other)
+        : m_bits(SlimFlag)
+    {
+        swap(other);
+    }
+
+    SymbolTableEntry& operator=(SymbolTableEntry&& other)
+    {
+        swap(other);
+        return *this;
+    }
+
+    void swap(SymbolTableEntry& other)
+    {
+        std::swap(m_bits, other.m_bits);
+    }
+
     bool isNull() const
     {
@@ -553 +570 @@
     }
     
-    void add(const ConcurrentJITLocker&, UniquedStringImpl* key, const SymbolTableEntry& entry)
+    template<typename Entry>
+    void add(const ConcurrentJITLocker&, UniquedStringImpl* key, Entry&& entry)
     {
         RELEASE_ASSERT(!m_localToEntry);
         didUseVarOffset(entry.varOffset());
-        Map::AddResult result = m_map.add(key, entry);
+        Map::AddResult result = m_map.add(key, std::forward<Entry>(entry));
         ASSERT_UNUSED(result, result.isNewEntry);
     }
     
-    void add(UniquedStringImpl* key, const SymbolTableEntry& entry)
+    template<typename Entry>
+    void add(UniquedStringImpl* key, Entry&& entry)
     {
         ConcurrentJITLocker locker(m_lock);
-        add(locker, key, entry);
-    }
-    
-    void set(const ConcurrentJITLocker&, UniquedStringImpl* key, const SymbolTableEntry& entry)
+        add(locker, key, std::forward<Entry>(entry));
+    }
+    
+    template<typename Entry>
+    void set(const ConcurrentJITLocker&, UniquedStringImpl* key, Entry&& entry)
     {
         RELEASE_ASSERT(!m_localToEntry);
         didUseVarOffset(entry.varOffset());
-        m_map.set(key, entry);
-    }
-    
-    void set(UniquedStringImpl* key, const SymbolTableEntry& entry)
+        m_map.set(key, std::forward<Entry>(entry));
+    }
+    
+    template<typename Entry>
+    void set(UniquedStringImpl* key, Entry&& entry)
     {
         ConcurrentJITLocker locker(m_lock);
-        set(locker, key, entry);
+        set(locker, key, std::forward<Entry>(entry));
     }
     

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2016-04-12  Yusuke Suzuki  <utatane.tea@gmail.com>
+
+        [JSC] addStaticGlobals should emit SymbolTableEntry watchpoints to encourage constant folding in DFG
+        https://bugs.webkit.org/show_bug.cgi?id=155110
+
+        Reviewed by Saam Barati.
+
+        * bindings/js/JSDOMWindowBase.cpp:
+        (WebCore::JSDOMWindowBase::updateDocument):
+
 2016-04-12  Sergio Villar Senin  <svillar@igalia.com>
 

trunk/Source/WebCore/bindings/js/JSDOMWindowBase.cpp

@@ -102 +102 @@
 void JSDOMWindowBase::updateDocument()
 {
+    // Since "document" property is defined as { configurable: false, writable: false, enumerable: true },
+    // users cannot change its attributes further.
+    // Reaching here, the attributes of "document" property should be never changed.
     ASSERT(m_wrapped->document());
     ExecState* exec = globalExec();
+    bool shouldThrowReadOnlyError = false;
+    bool ignoreReadOnlyErrors = true;
     bool putResult = false;
-    symbolTablePutWithAttributesTouchWatchpointSet(this, exec, exec->vm().propertyNames->document, toJS(exec, this, m_wrapped->document()), DontDelete | ReadOnly, putResult);
+    symbolTablePutTouchWatchpointSet(this, exec, exec->vm().propertyNames->document, toJS(exec, this, m_wrapped->document()), shouldThrowReadOnlyError, ignoreReadOnlyErrors, putResult);
 }