Simplify HeapSnapshotFilteredOrderedIterator implementation

Source/devtools/front_end/HeapSnapshot.js

 /*
  * Copyright (C) 2011 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are
  * met:
  *
  *     * Redistributions of source code must retain the above copyright
  * notice, this list of conditions and the following disclaimer.
  *     * Redistributions in binary form must reproduce the above
@@ skipping 636 matching lines @@
     },
 
     next: function()
     {
         this.node.nodeIndex = this.node._nextNodeIndex();
     }
 }
 
 
 /**
+ * @constructor
+ */
+WebInspector.HeapSnapshotIndexRangeIterator = function(iterator, indexes)

[alph, 2014/03/20 09:58:04]

annotate plz

[yurys, 2014/03/20 11:47:42]

Done.

+{
+    this._iterator = iterator;
+    this._indexes = indexes;
+    this._position = 0;
+    this._forcedIndex = -1;
+}
+
+WebInspector.HeapSnapshotIndexRangeIterator.prototype = {
+    /**
+     * @return {boolean}
+     */
+    hasNext: function()
+    {
+        return this._position < this._indexes.length

[alph, 2014/03/20 09:58:04]

; missing

[yurys, 2014/03/20 11:47:42]

Done.

+    },
+
+    /**
+     * @return {number}
+     */
+    index: function()

[loislo, 2014/03/20 09:58:26]

Can we just use a free function for converting index into an item.
because it looks strange when we stick the iterator to a particular index and
don't allow it to move next.

[yurys, 2014/03/20 11:47:42]

We will need to pass it as a separate object as it needs a context (snapshot and
item's class). I'll address this in a separate patch.

+    {
+        if (this._forcedIndex !== -1)
+            return this._forcedIndex;
+        return this._indexes[this._position];
+    },
+
+    /**
+     * @param {number} newIndex
+     */
+    setIndex: function(newIndex)
+    {
+        this._forcedIndex = newIndex;
+    },
+
+    /**
+     * @return {!WebInspector.HeapSnapshotNode}
+     */
+    item: function()
+    {
+        this._iterator.setIndex(this.index());
+        return this._iterator.item();
+    },
+
+    next: function()
+    {
+        ++this._position;
+    }
+}
+
+
+/**
+ * @constructor
+ */
+WebInspector.HeapSnapshotFilteredIterator = function(iterator, filter)
+{
+    this._iterator = iterator;
+    this._filter = filter;
+    this._skipFilteredItems();
+}
+
+WebInspector.HeapSnapshotFilteredIterator.prototype = {
+    /**
+     * @return {boolean}
+     */
+    hasNext: function()
+    {
+        return this._iterator.hasNext();
+    },
+
+    /**
+     * @return {number}
+     */
+    index: function()
+    {
+        return this._iterator.index();
+    },
+
+    /**
+     * @param {number} newIndex
+     */
+    setIndex: function(newIndex)
+    {
+        this._iterator.setIndex(newIndex);
+    },
+
+    /**
+     * @return {!WebInspector.HeapSnapshotNode}
+     */
+    item: function()
+    {
+        return this._iterator.item();
+    },
+
+    next: function()
+    {
+        this._iterator.next();
+        this._skipFilteredItems();
+    },
+
+    _skipFilteredItems: function()
+    {
+        while (this._iterator.hasNext() && !this._filter(this._iterator.item())) {
+            this._iterator.next();
+        }
+    }
+}
+
+
+/**
  * @param {!WebInspector.HeapSnapshotWorkerDispatcher=} dispatcher
  * @constructor
  */
 WebInspector.HeapSnapshotProgress = function(dispatcher)
 {
     this._dispatcher = dispatcher;
 }
 
 WebInspector.HeapSnapshotProgress.prototype = {
     /**
@@ skipping 1218 matching lines @@
      * @return {!WebInspector.HeapSnapshotCommon.StaticData}
      */
     updateStaticData: function()
     {
         return new WebInspector.HeapSnapshotCommon.StaticData(this.nodeCount, this._rootNodeIndex, this.totalSize, this._maxJsNodeId());
     }
 };
 
 /**
  * @constructor
- * @param {(!Array.<number>|!Uint32Array)=} unfilteredIterationOrder
  */
-WebInspector.HeapSnapshotFilteredOrderedIterator = function(iterator, filter, unfilteredIterationOrder)
+WebInspector.HeapSnapshotFilteredOrderedIterator = function(iterator)

[alph, 2014/03/20 09:58:04]

I'd suggest drop "Filtered" from the class name.

[yurys, 2014/03/20 11:47:42]

Done.

 {
-    this._filter = filter;
     this._iterator = iterator;
-    this._unfilteredIterationOrder = unfilteredIterationOrder;
-    /** @type {?Array.<number>|?Uint32Array} */
+    this._isEmpty = !iterator.hasNext();
+    /** @type {?Array.<number>} */
     this._iterationOrder = null;
-    this._position = 0;
     this._currentComparator = null;
     this._sortedPrefixLength = 0;
     this._sortedSuffixLength = 0;
 }
 
 WebInspector.HeapSnapshotFilteredOrderedIterator.prototype = {
     _createIterationOrder: function()
     {
         if (this._iterationOrder)
             return;
-        if (this._unfilteredIterationOrder && !this._filter) {
-            this._iterationOrder = this._unfilteredIterationOrder;
-            this._unfilteredIterationOrder = null;
-            return;
-        }
         this._iterationOrder = [];
-        var iterator = this._iterator;
-        if (!this._unfilteredIterationOrder && !this._filter) {
-            for (iterator.rewind(); iterator.hasNext(); iterator.next())
-                this._iterationOrder.push(iterator.index());
-        } else if (!this._unfilteredIterationOrder) {
-            for (iterator.rewind(); iterator.hasNext(); iterator.next()) {
-                if (this._filter(iterator.item()))
-                    this._iterationOrder.push(iterator.index());
-            }
-        } else {
-            var order = this._unfilteredIterationOrder;
-            for (var i = 0, l = order.length; i < l; ++i) {
-                iterator.setIndex(order[i]);
-                if (this._filter(iterator.item()))
-                    this._iterationOrder.push(iterator.index());
-            }
-            this._unfilteredIterationOrder = null;
-        }
-    },
-
-    rewind: function()
-    {
-        this._position = 0;
+        for (var iterator = this._iterator; iterator.hasNext(); iterator.next())
+            this._iterationOrder.push(iterator.index());
     },
 
     /**
-     * @return {boolean}
-     */
-    hasNext: function()
-    {
-        return this._position < this._iterationOrder.length;
-    },
-
-    /**
      * @return {boolean}
      */
     isEmpty: function()
     {
-        if (this._iterationOrder)
-            return !this._iterationOrder.length;
-        if (this._unfilteredIterationOrder && !this._filter)
-            return !this._unfilteredIterationOrder.length;
-        var iterator = this._iterator;
-        if (!this._unfilteredIterationOrder && !this._filter) {
-            iterator.rewind();
-            return !iterator.hasNext();
-        } else if (!this._unfilteredIterationOrder) {
-            for (iterator.rewind(); iterator.hasNext(); iterator.next())
-                if (this._filter(iterator.item()))
-                    return false;
-        } else {
-            var order = this._unfilteredIterationOrder;
-            for (var i = 0, l = order.length; i < l; ++i) {
-                iterator.setIndex(order[i]);
-                if (this._filter(iterator.item()))
-                    return false;
-            }
-        }
-        return true;
+        return this._isEmpty;
     },
 
     /**
-     * @return {*}
-     */
-    item: function()
-    {
-        this._iterator.setIndex(this._iterationOrder[this._position]);
-        return this._iterator.item();
-    },
-
-    /**
-     * @type {number}
-     */
-    get length()
-    {
-        this._createIterationOrder();
-        return this._iterationOrder.length;
-    },
-
-    next: function()
-    {
-        ++this._position;
-    },
-
-    /**
      * @param {number} begin
      * @param {number} end
      * @return {!WebInspector.HeapSnapshotCommon.ItemsRange}
      */
     serializeItemsRange: function(begin, end)
     {
         this._createIterationOrder();
         if (begin > end)
             throw new Error("Start position > end position: " + begin + " > " + end);
         if (end > this._iterationOrder.length)
             end = this._iterationOrder.length;
         if (this._sortedPrefixLength < end && begin < this._iterationOrder.length - this._sortedSuffixLength) {
             this.sort(this._currentComparator, this._sortedPrefixLength, this._iterationOrder.length - 1 - this._sortedSuffixLength, begin, end - 1);
             if (begin <= this._sortedPrefixLength)
                 this._sortedPrefixLength = end;
             if (end >= this._iterationOrder.length - this._sortedSuffixLength)
                 this._sortedSuffixLength = this._iterationOrder.length - begin;
         }
-
-        this._position = begin;
-        var startPosition = this._position;
+        var position = begin;
         var count = end - begin;
         var result = new Array(count);
-        for (var i = 0 ; i < count && this.hasNext(); ++i, this.next())
-            result[i] = this.item().serialize();
-        return new WebInspector.HeapSnapshotCommon.ItemsRange(startPosition, this._position, this._iterationOrder.length, result);
+        var iterator = this._iterator;
+        for (var i = 0 ; i < count; ++i) {
+            iterator.setIndex(this._iterationOrder[position++]);
+            result[i] = iterator.item().serialize();

[alph, 2014/03/20 09:58:04]

Consider joining setIndex+item into itemByIndex

[yurys, 2014/03/20 11:47:42]

Let me address this in a separate patch.

+        }
+        return new WebInspector.HeapSnapshotCommon.ItemsRange(begin, end, this._iterationOrder.length, result);
     },
 
     sortAndRewind: function(comparator)
     {
         this._currentComparator = comparator;
         this._sortedPrefixLength = 0;
         this._sortedSuffixLength = 0;
-        this.rewind();
     }
 }
 
 /**
  * @constructor
  * @extends {WebInspector.HeapSnapshotFilteredOrderedIterator}
  */
 WebInspector.HeapSnapshotEdgesProvider = function(snapshot, filter, edgesIter)
 {
     this.snapshot = snapshot;
-    WebInspector.HeapSnapshotFilteredOrderedIterator.call(this, edgesIter, filter);
+    if (filter)
+        edgesIter = new WebInspector.HeapSnapshotFilteredIterator(edgesIter, filter);
+    WebInspector.HeapSnapshotFilteredOrderedIterator.call(this, edgesIter);
 }
 
 WebInspector.HeapSnapshotEdgesProvider.prototype = {
     /**
      * @param {!WebInspector.HeapSnapshotCommon.ComparatorConfig} comparator
      * @param {number} leftBound
      * @param {number} rightBound
      * @param {number} windowLeft
      * @param {number} windowRight
      */
@@ skipping 71 matching lines @@
             this._iterationOrder.sortRange(compareNodeAndNode, leftBound, rightBound, windowLeft, windowRight);
     },
 
     __proto__: WebInspector.HeapSnapshotFilteredOrderedIterator.prototype
 }
 
 
 /**
  * @constructor
  * @extends {WebInspector.HeapSnapshotFilteredOrderedIterator}
- * @param {(!Array.<number>|!Uint32Array)=} nodeIndexes
+ * @param {?function(!WebInspector.HeapSnapshotNode):boolean} filter
+ * @param {(!Array.<number>|!Uint32Array)} nodeIndexes
  */
 WebInspector.HeapSnapshotNodesProvider = function(snapshot, filter, nodeIndexes)
 {
     this.snapshot = snapshot;
-    WebInspector.HeapSnapshotFilteredOrderedIterator.call(this, snapshot._allNodes(), filter, nodeIndexes);
+    var it = new WebInspector.HeapSnapshotIndexRangeIterator(snapshot._allNodes(), nodeIndexes);
+    if (filter)
+        it = new WebInspector.HeapSnapshotFilteredIterator(it, filter);
+    WebInspector.HeapSnapshotFilteredOrderedIterator.call(this, it);
 }
 
 WebInspector.HeapSnapshotNodesProvider.prototype = {
     /**
      * @param {string} snapshotObjectId
      * @return {number}
      */
     nodePosition: function(snapshotObjectId)
     {
         this._createIterationOrder();
@@ skipping 64 matching lines @@
      * @param {number} windowRight
      */
     sort: function(comparator, leftBound, rightBound, windowLeft, windowRight)
     {
         this._iterationOrder.sortRange(this._buildCompareFunction(comparator), leftBound, rightBound, windowLeft, windowRight);
     },
 
     __proto__: WebInspector.HeapSnapshotFilteredOrderedIterator.prototype
 }