[JSC] addStaticGlobals should emit SymbolTableEntry watchpoints to encourage constant...
authorutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Apr 2016 08:25:48 +0000 (08:25 +0000)
committerutatane.tea@gmail.com <utatane.tea@gmail.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 12 Apr 2016 08:25:48 +0000 (08:25 +0000)
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):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@199342 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/bytecode/VariableWriteFireDetail.h
Source/JavaScriptCore/runtime/JSGlobalObject.cpp
Source/JavaScriptCore/runtime/JSSymbolTableObject.h
Source/JavaScriptCore/runtime/SymbolTable.h
Source/WebCore/ChangeLog
Source/WebCore/bindings/js/JSDOMWindowBase.cpp

index 7ce405f..7851d2b 100644 (file)
@@ -1,3 +1,39 @@
+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>
 
         Build fix after r199299.
index 31b9200..1c79eac 100644 (file)
@@ -41,7 +41,7 @@ public:
     {
     }
     
-    void dump(PrintStream&) const override;
+    JS_EXPORT_PRIVATE void dump(PrintStream&) const override;
     
     JS_EXPORT_PRIVATE static void touch(WatchpointSet*, JSObject*, const PropertyName&);
 
index bae10b2..098ef6a 100644 (file)
@@ -679,7 +679,7 @@ void JSGlobalObject::addGlobalVar(const Identifier& ident)
     ScopeOffset offset = symbolTable()->takeNextScopeOffset(locker);
     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());
     RELEASE_ASSERT(offsetForAssert == offset);
@@ -979,15 +979,19 @@ void JSGlobalObject::addStaticGlobals(GlobalPropertyInfo* globals, int count)
         GlobalPropertyInfo& global = globals[i];
         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);
     }
 }
 
index d44cde0..a7fd01f 100644 (file)
@@ -141,20 +141,33 @@ inline bool symbolTableGet(
     return true;
 }
 
+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)
+inline bool symbolTablePut(SymbolTableObjectType* object, ExecState* exec, PropertyName propertyName, JSValue value, bool shouldThrowReadOnlyError, bool ignoreReadOnlyErrors, bool& putResult)
 {
-    ASSERT(!Heap::heap(value) || Heap::heap(value) == Heap::heap(object));
-
     VM& vm = exec->vm();
 
+    WatchpointSet* set = nullptr;
     WriteBarrierBase<Unknown>* reg;
     {
         SymbolTable& symbolTable = *object->symbolTable();
@@ -181,14 +194,15 @@ inline bool symbolTablePut(
             return false;
 
         set = iter->value.watchpointSet();
-        if (symbolTablePutMode == SymbolTablePutMode::WithAttributes)
-            iter->value.setAttributes(attributes);
         reg = &object->variableAt(offset);
     }
     // I'd prefer we not hold lock while executing barriers, since I prefer to reserve
     // 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;
 }
@@ -198,12 +212,8 @@ inline bool symbolTablePutTouchWatchpointSet(
     SymbolTableObjectType* object, ExecState* exec, PropertyName propertyName, JSValue value,
     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);
 }
 
 template<typename SymbolTableObjectType>
@@ -211,26 +221,8 @@ inline bool symbolTablePutInvalidateWatchpointSet(
     SymbolTableObjectType* object, ExecState* exec, PropertyName propertyName, JSValue value,
     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);
 }
 
 } // namespace JSC
index 632acad..80d1b88 100644 (file)
@@ -199,6 +199,23 @@ public:
         return *this;
     }
     
+    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
     {
         return !(bits() & ~SlimFlag);
@@ -552,31 +569,35 @@ public:
         return takeNextScopeOffset(locker);
     }
     
-    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);
+        add(locker, key, std::forward<Entry>(entry));
     }
     
-    void set(const ConcurrentJITLocker&, UniquedStringImpl* key, const SymbolTableEntry& entry)
+    template<typename Entry>
+    void set(const ConcurrentJITLocker&, UniquedStringImpl* key, Entry&& entry)
     {
         RELEASE_ASSERT(!m_localToEntry);
         didUseVarOffset(entry.varOffset());
-        m_map.set(key, entry);
+        m_map.set(key, std::forward<Entry>(entry));
     }
     
-    void set(UniquedStringImpl* key, const SymbolTableEntry& 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));
     }
     
     bool contains(const ConcurrentJITLocker&, UniquedStringImpl* key)
index 0fee806..fefe773 100644 (file)
@@ -1,3 +1,13 @@
+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>
 
         [css-grid] Pass GridSizingData instead of columnTracks to track sizing methods
index 22a42d2..4808320 100644 (file)
@@ -101,10 +101,15 @@ void JSDOMWindowBase::destroy(JSCell* cell)
 
 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);
 }
 
 ScriptExecutionContext* JSDOMWindowBase::scriptExecutionContext() const