Title: [295036] trunk/Source/_javascript_Core/heap
Revision
295036
Author
ysuz...@apple.com
Date
2022-05-30 16:41:10 -0700 (Mon, 30 May 2022)

Log Message

[JSC] Make Strong::set cheap
https://bugs.webkit.org/show_bug.cgi?id=241090

Reviewed by Mark Lam.

HandleSet::writeBarrier is frequently called because it is called every time we set a value in Strong<>.
This patch optimizes it,

1. We should make it inline function since it has a super fast path major use can be covered. And this function is small.
2. We should not always remove a node from the list first. We should insert / remove it only when necessary.
3. Remove m_immediateList since it is not necessary.
4. Make HandleNode as a derived class of BasicRawSentinelNode to make implementation simpler.

This change improves promise benchmarks score since promise uses microtasks which hold values via Strong<>.

        ToT
        Time(doxbee-async-bluebird): 42.8 ms.
        Time(doxbee-async-es2017-babel): 36.4 ms.
        Time(doxbee-async-es2017-native): 28.3 ms.
        Time(doxbee-promises-bluebird): 514.2 ms.
        Time(doxbee-promises-es2015-native): 44.8 ms.
        Time(fibonacci-async-es2017-babel): 380.5 ms.
        Time(fibonacci-async-es2017-native): 218.2 ms.
        Time(parallel-async-bluebird): 648.8 ms.
        Time(parallel-async-es2017-babel): 116.9 ms.
        Time(parallel-async-es2017-native): 115.6 ms.
        Time(parallel-promises-bluebird): 638 ms.
        Time(parallel-promises-es2015-native): 82 ms.

        Patched
        Time(doxbee-async-bluebird): 38 ms.
        Time(doxbee-async-es2017-babel): 27 ms.
        Time(doxbee-async-es2017-native): 19.5 ms.
        Time(doxbee-promises-bluebird): 508.3 ms.
        Time(doxbee-promises-es2015-native): 33.3 ms.
        Time(fibonacci-async-es2017-babel): 349.1 ms.
        Time(fibonacci-async-es2017-native): 151 ms.
        Time(parallel-async-bluebird): 639.6 ms.
        Time(parallel-async-es2017-babel): 100.9 ms.
        Time(parallel-async-es2017-native): 101.9 ms.
        Time(parallel-promises-bluebird): 614 ms.
        Time(parallel-promises-es2015-native): 70.9 ms.

* Source/_javascript_Core/heap/HandleSet.cpp:
(JSC::HandleSet::writeBarrier): Deleted.
* Source/_javascript_Core/heap/HandleSet.h:
(JSC::HandleSet::heapFor):
(JSC::HandleSet::allocate):
(JSC::HandleSet::deallocate):
(JSC::HandleSet::writeBarrier):
(JSC::HandleSet::toHandle): Deleted.
(JSC::HandleSet::toNode): Deleted.
(JSC::HandleNode::HandleNode): Deleted.
(JSC::HandleNode::setPrev): Deleted.
(JSC::HandleNode::prev): Deleted.
(JSC::HandleNode::setNext): Deleted.
(JSC::HandleNode::next): Deleted.
* Source/_javascript_Core/heap/Strong.h:
(JSC::Strong::set):

Canonical link: https://commits.webkit.org/251131@main

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/heap/HandleSet.cpp (295035 => 295036)


--- trunk/Source/_javascript_Core/heap/HandleSet.cpp	2022-05-30 22:10:58 UTC (rev 295035)
+++ trunk/Source/_javascript_Core/heap/HandleSet.cpp	2022-05-30 23:41:10 UTC (rev 295036)
@@ -70,27 +70,6 @@
 template void HandleSet::visitStrongHandles(AbstractSlotVisitor&);
 template void HandleSet::visitStrongHandles(SlotVisitor&);
 
-void HandleSet::writeBarrier(HandleSlot slot, const JSValue& value)
-{
-    if (!value == !*slot && slot->isCell() == value.isCell())
-        return;
-
-    Node* node = toNode(slot);
-#if ENABLE(GC_VALIDATION)
-    RELEASE_ASSERT(isLiveNode(node));
-#endif
-    SentinelLinkedList<Node>::remove(node);
-    if (!value || !value.isCell()) {
-        m_immediateList.push(node);
-        return;
-    }
-
-    m_strongList.push(node);
-#if ENABLE(GC_VALIDATION)
-    RELEASE_ASSERT(isLiveNode(node));
-#endif
-}
-
 unsigned HandleSet::protectedGlobalObjectCount()
 {
     unsigned count = 0;

Modified: trunk/Source/_javascript_Core/heap/HandleSet.h (295035 => 295036)


--- trunk/Source/_javascript_Core/heap/HandleSet.h	2022-05-30 22:10:58 UTC (rev 295035)
+++ trunk/Source/_javascript_Core/heap/HandleSet.h	2022-05-30 23:41:10 UTC (rev 295036)
@@ -39,24 +39,20 @@
 class VM;
 class JSValue;
 
-class HandleNode {
+class HandleNode final : public BasicRawSentinelNode<HandleNode> {
 public:
-    HandleNode(WTF::SentinelTag);
-    HandleNode();
+    HandleNode() = default;
     
     HandleSlot slot();
     HandleSet* handleSet();
 
-    void setPrev(HandleNode*);
-    HandleNode* prev();
+    static HandleNode* toHandleNode(HandleSlot slot)
+    {
+        return bitwise_cast<HandleNode*>(bitwise_cast<uintptr_t>(slot) - OBJECT_OFFSETOF(HandleNode, m_value));
+    }
 
-    void setNext(HandleNode*);
-    HandleNode* next();
-
 private:
-    JSValue m_value;
-    HandleNode* m_prev;
-    HandleNode* m_next;
+    JSValue m_value { };
 };
 
 class HandleSet {
@@ -74,7 +70,8 @@
 
     template<typename Visitor> void visitStrongHandles(Visitor&);
 
-    JS_EXPORT_PRIVATE void writeBarrier(HandleSlot, const JSValue&);
+    template<bool isCellOnly>
+    void writeBarrier(HandleSlot, JSValue);
 
     unsigned protectedGlobalObjectCount();
 
@@ -82,26 +79,24 @@
 
 private:
     typedef HandleNode Node;
-    static HandleSlot toHandle(Node*);
-    static Node* toNode(HandleSlot);
 
     JS_EXPORT_PRIVATE void grow();
     
 #if ENABLE(GC_VALIDATION) || ASSERT_ENABLED
-    bool isLiveNode(Node*);
+    JS_EXPORT_PRIVATE bool isLiveNode(Node*);
 #endif
 
     VM& m_vm;
     DoublyLinkedList<HandleBlock> m_blockList;
 
-    SentinelLinkedList<Node> m_strongList;
-    SentinelLinkedList<Node> m_immediateList;
+    using NodeList = SentinelLinkedList<Node, BasicRawSentinelNode<Node>>;
+    NodeList m_strongList;
     SinglyLinkedList<Node> m_freeList;
 };
 
 inline HandleSet* HandleSet::heapFor(HandleSlot handle)
 {
-    return toNode(handle)->handleSet();
+    return HandleNode::toHandleNode(handle)->handleSet();
 }
 
 inline VM& HandleSet::vm()
@@ -109,16 +104,6 @@
     return m_vm;
 }
 
-inline HandleSlot HandleSet::toHandle(HandleSet::Node* node)
-{
-    return reinterpret_cast<HandleSlot>(node);
-}
-
-inline HandleSet::Node* HandleSet::toNode(HandleSlot handle)
-{
-    return reinterpret_cast<HandleSet::Node*>(handle);
-}
-
 inline HandleSlot HandleSet::allocate()
 {
     if (m_freeList.isEmpty())
@@ -126,29 +111,17 @@
 
     HandleSet::Node* node = m_freeList.pop();
     new (NotNull, node) HandleSet::Node();
-    m_immediateList.push(node);
-    return toHandle(node);
+    return node->slot();
 }
 
 inline void HandleSet::deallocate(HandleSlot handle)
 {
-    HandleSet::Node* node = toNode(handle);
-    SentinelLinkedList<HandleSet::Node>::remove(node);
+    HandleSet::Node* node = HandleNode::toHandleNode(handle);
+    if (node->isOnList())
+        NodeList::remove(node);
     m_freeList.push(node);
 }
 
-inline HandleNode::HandleNode()
-    : m_prev(nullptr)
-    , m_next(nullptr)
-{
-}
-
-inline HandleNode::HandleNode(WTF::SentinelTag)
-    : m_prev(nullptr)
-    , m_next(nullptr)
-{
-}
-
 inline HandleSlot HandleNode::slot()
 {
     return &m_value;
@@ -159,26 +132,6 @@
     return HandleBlock::blockFor(this)->handleSet();
 }
 
-inline void HandleNode::setPrev(HandleNode* prev)
-{
-    m_prev = prev;
-}
-
-inline HandleNode* HandleNode::prev()
-{
-    return m_prev;
-}
-
-inline void HandleNode::setNext(HandleNode* next)
-{
-    m_next = next;
-}
-
-inline HandleNode* HandleNode::next()
-{
-    return m_next;
-}
-
 template<typename Functor> void HandleSet::forEachStrongHandle(const Functor& functor, const HashCountedSet<JSCell*>& skipSet)
 {
     for (Node& node : m_strongList) {
@@ -191,4 +144,33 @@
     }
 }
 
+template<bool isCellOnly>
+inline void HandleSet::writeBarrier(HandleSlot slot, JSValue value)
+{
+    bool valueIsNonEmptyCell = value && (isCellOnly || value.isCell());
+    bool slotIsNonEmptyCell = *slot && (isCellOnly || slot->isCell());
+    if (valueIsNonEmptyCell == slotIsNonEmptyCell)
+        return;
+
+    Node* node = HandleNode::toHandleNode(slot);
+#if ENABLE(GC_VALIDATION)
+    if (node->isOnList())
+        RELEASE_ASSERT(isLiveNode(node));
+#endif
+    if (!valueIsNonEmptyCell) {
+        ASSERT(slotIsNonEmptyCell);
+        ASSERT(node->isOnList());
+        NodeList::remove(node);
+        return;
+    }
+
+    ASSERT(!slotIsNonEmptyCell);
+    ASSERT(!node->isOnList());
+    m_strongList.push(node);
+
+#if ENABLE(GC_VALIDATION)
+    RELEASE_ASSERT(isLiveNode(node));
+#endif
+}
+
 } // namespace JSC

Modified: trunk/Source/_javascript_Core/heap/Strong.h (295035 => 295036)


--- trunk/Source/_javascript_Core/heap/Strong.h	2022-05-30 22:10:58 UTC (rev 295035)
+++ trunk/Source/_javascript_Core/heap/Strong.h	2022-05-30 23:41:10 UTC (rev 295036)
@@ -141,7 +141,7 @@
     {
         ASSERT(slot());
         JSValue value = HandleTypes<T>::toJSValue(externalType);
-        HandleSet::heapFor(slot())->writeBarrier(slot(), value);
+        HandleSet::heapFor(slot())->template writeBarrier<std::is_base_of_v<JSCell, T>>(slot(), value);
         *slot() = value;
     }
 };
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to