Title: [225315] trunk/Source/_javascript_Core
Revision
225315
Author
fpi...@apple.com
Date
2017-11-29 20:48:52 -0800 (Wed, 29 Nov 2017)

Log Message

CodeBlockSet::deleteUnmarkedAndUnreferenced can be a little more efficient
https://bugs.webkit.org/show_bug.cgi?id=180108

Reviewed by Saam Barati.
        
This was creating a vector of things to remove and then removing them. I think I remember writing
this code, and I did that because at the time we did not have removeAllMatching, which is
definitely better. This is a minuscule optimization for Speedometer. I wanted to land this
obvious improvement before I did more fundamental things to this code.

* heap/CodeBlockSet.cpp:
(JSC::CodeBlockSet::deleteUnmarkedAndUnreferenced):

Modified Paths

Diff

Modified: trunk/Source/_javascript_Core/ChangeLog (225314 => 225315)


--- trunk/Source/_javascript_Core/ChangeLog	2017-11-30 04:39:50 UTC (rev 225314)
+++ trunk/Source/_javascript_Core/ChangeLog	2017-11-30 04:48:52 UTC (rev 225315)
@@ -1,3 +1,18 @@
+2017-11-28  Filip Pizlo  <fpi...@apple.com>
+
+        CodeBlockSet::deleteUnmarkedAndUnreferenced can be a little more efficient
+        https://bugs.webkit.org/show_bug.cgi?id=180108
+
+        Reviewed by Saam Barati.
+        
+        This was creating a vector of things to remove and then removing them. I think I remember writing
+        this code, and I did that because at the time we did not have removeAllMatching, which is
+        definitely better. This is a minuscule optimization for Speedometer. I wanted to land this
+        obvious improvement before I did more fundamental things to this code.
+
+        * heap/CodeBlockSet.cpp:
+        (JSC::CodeBlockSet::deleteUnmarkedAndUnreferenced):
+
 2017-11-29  Filip Pizlo  <fpi...@apple.com>
 
         GC should support isoheaps

Modified: trunk/Source/_javascript_Core/heap/CodeBlockSet.cpp (225314 => 225315)


--- trunk/Source/_javascript_Core/heap/CodeBlockSet.cpp	2017-11-30 04:39:50 UTC (rev 225314)
+++ trunk/Source/_javascript_Core/heap/CodeBlockSet.cpp	2017-11-30 04:48:52 UTC (rev 225315)
@@ -28,6 +28,7 @@
 
 #include "CodeBlock.h"
 #include "JSCInlines.h"
+#include "SuperSampler.h"
 #include <wtf/CommaPrinter.h>
 
 namespace JSC {
@@ -74,19 +75,26 @@
 void CodeBlockSet::deleteUnmarkedAndUnreferenced(VM& vm, CollectionScope scope)
 {
     LockHolder locker(&m_lock);
-    Vector<CodeBlock*> unmarked;
     
+    // Destroying a CodeBlock takes about 1us on average in Speedometer. Full collections in Speedometer
+    // usually have ~2000 CodeBlocks to process. The time it takes to process the whole list varies a
+    // lot. In one extreme case I saw 18ms (on my fast MBP).
+    //
+    // FIXME: use Subspace instead of HashSet and adopt Subspace-based constraint solving. This may
+    // remove the need to eagerly destruct CodeBlocks.
+    // https://bugs.webkit.org/show_bug.cgi?id=180089
+    //
+    // FIXME: make CodeBlock::~CodeBlock a lot faster. It seems insane for that to take 1us or more.
+    // https://bugs.webkit.org/show_bug.cgi?id=180109
+    
     auto consider = [&] (HashSet<CodeBlock*>& set) {
-        for (CodeBlock* codeBlock : set) {
-            if (Heap::isMarked(codeBlock))
-                continue;;
-            unmarked.append(codeBlock);
-        }
-        for (CodeBlock* codeBlock : unmarked) {
-            codeBlock->structure(vm)->classInfo()->methodTable.destroy(codeBlock);
-            set.remove(codeBlock);
-        }
-        unmarked.shrink(0);
+        set.removeIf(
+            [&] (CodeBlock* codeBlock) -> bool {
+                if (Heap::isMarked(codeBlock))
+                    return false;
+                codeBlock->structure(vm)->classInfo()->methodTable.destroy(codeBlock);
+                return true;
+            });
     };
 
     switch (scope) {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to