Merge pull request #8889 from CesiumGS/remove-trailing-references

Remove trailing references from ManagedArray and Heap

Author: IanLilleyT

Source/Core/Heap.js

@@ -65,11 +65,20 @@ Object.defineProperties(Heap.prototype, {
       return this._maximumLength;
     },
     set: function (value) {
-      this._maximumLength = value;
-      if (this._length > value && value > 0) {
+      //>>includeStart('debug', pragmas.debug);
+      Check.typeOf.number.greaterThanOrEquals("maximumLength", value, 0);
+      //>>includeEnd('debug');
+      var originalLength = this._length;
+      if (value < originalLength) {
+        var array = this._array;
+        // Remove trailing references
+        for (var i = value; i < originalLength; ++i) {
+          array[i] = undefined;
+        }
         this._length = value;
-        this._array.length = value;
+        array.length = value;
       }
+      this._maximumLength = value;
     },
   },
 
@@ -211,6 +220,7 @@ Heap.prototype.pop = function (index) {
   var root = array[index];
   swap(array, index, --this._length);
   this.heapify(index);
+  array[this._length] = undefined; // Remove trailing reference
   return root;
 };
 

Source/Core/ManagedArray.js

@@ -29,10 +29,20 @@ Object.defineProperties(ManagedArray.prototype, {
       return this._length;
     },
     set: function (length) {
-      this._length = length;
-      if (length > this._array.length) {
-        this._array.length = length;
+      //>>includeStart('debug', pragmas.debug);
+      Check.typeOf.number.greaterThanOrEquals("length", length, 0);
+      //>>includeEnd('debug');
+      var array = this._array;
+      var originalLength = this._length;
+      if (length < originalLength) {
+        // Remove trailing references
+        for (var i = length; i < originalLength; ++i) {
+          array[i] = undefined;
+        }
+      } else if (length > array.length) {
+        array.length = length;
       }
+      this._length = length;
     },
   },
 
@@ -74,7 +84,7 @@ ManagedArray.prototype.set = function (index, element) {
   Check.typeOf.number("index", index);
   //>>includeEnd('debug');
 
-  if (index >= this.length) {
+  if (index >= this._length) {
     this.length = index + 1;
   }
   this._array[index] = element;
@@ -105,7 +115,12 @@ ManagedArray.prototype.push = function (element) {
  * @returns {*} The last element in the array.
  */
 ManagedArray.prototype.pop = function () {
-  return this._array[--this.length];
+  if (this._length === 0) {
+    return undefined;
+  }
+  var element = this._array[this._length - 1];
+  --this.length;
+  return element;
 };
 
 /**
@@ -142,7 +157,7 @@ ManagedArray.prototype.resize = function (length) {
  * @param {Number} [length] The length.
  */
 ManagedArray.prototype.trim = function (length) {
-  length = defaultValue(length, this.length);
+  length = defaultValue(length, this._length);
   this._array.length = length;
 };
 export default ManagedArray;

Specs/Core/HeapSpec.js

@@ -3,6 +3,15 @@ import { Heap } from "../../Source/Cesium.js";
 describe("Core/Heap", function () {
   var length = 100;
 
+  function expectTrailingReferenceToBeRemoved(heap) {
+    var array = heap._array;
+    var length = heap._length;
+    var reservedLength = array.length;
+    for (var i = length; i < reservedLength; ++i) {
+      expect(array[i]).toBeUndefined();
+    }
+  }
+
   function checkHeap(heap, comparator) {
     var array = heap.internalArray;
     var pass = true;
@@ -90,6 +99,34 @@ describe("Core/Heap", function () {
     expect(pass).toBe(true);
   });
 
+  it("pop removes trailing references", function () {
+    var heap = new Heap({
+      comparator: comparator,
+    });
+
+    for (var i = 0; i < 10; ++i) {
+      heap.insert(Math.random());
+    }
+
+    heap.pop();
+    heap.pop();
+
+    expectTrailingReferenceToBeRemoved(heap);
+  });
+
+  it("setting maximum length less than current length removes trailing references", function () {
+    var heap = new Heap({
+      comparator: comparator,
+    });
+
+    for (var i = 0; i < 10; ++i) {
+      heap.insert(Math.random());
+    }
+
+    heap.maximumLength = 5;
+    expectTrailingReferenceToBeRemoved(heap);
+  });
+
   it("insert returns the removed element when maximumLength is set", function () {
     var heap = new Heap({
       comparator: comparator,
@@ -170,4 +207,13 @@ describe("Core/Heap", function () {
       currentId = element.id;
     }
   });
+
+  it("maximumLength setter throws if length is less than 0", function () {
+    var heap = new Heap({
+      comparator: comparator,
+    });
+    expect(function () {
+      heap.maximumLength = -1;
+    }).toThrowDeveloperError();
+  });
 });

Specs/Core/ManagedArraySpec.js

@@ -1,6 +1,15 @@
 import { ManagedArray } from "../../Source/Cesium.js";
 
 describe("Core/ManagedArray", function () {
+  function expectTrailingReferenceToBeRemoved(managedArray) {
+    var array = managedArray._array;
+    var length = managedArray._length;
+    var reservedLength = array.length;
+    for (var i = length; i < reservedLength; ++i) {
+      expect(array[i]).toBeUndefined();
+    }
+  }
+
   it("constructor has expected default values", function () {
     var array = new ManagedArray();
     expect(array.length).toEqual(0);
@@ -42,6 +51,13 @@ describe("Core/ManagedArray", function () {
     }).toThrowDeveloperError();
   });
 
+  it("length setter throws if length is less than 0", function () {
+    var array = new ManagedArray();
+    expect(function () {
+      array.length = -1;
+    }).toThrowDeveloperError();
+  });
+
   it("set resizes array", function () {
     var array = new ManagedArray();
     array.set(0, "a");
@@ -90,6 +106,24 @@ describe("Core/ManagedArray", function () {
     }
   });
 
+  it("pop removes trailing references", function () {
+    var length = 10;
+    var array = new ManagedArray(length);
+    array.set(0, Math.random());
+    array.set(1, Math.random());
+    array.set(2, Math.random());
+    array.pop();
+    array.pop();
+    expectTrailingReferenceToBeRemoved(array);
+  });
+
+  it("pop returns undefined if array is empty", function () {
+    var array = new ManagedArray();
+    array.push(1);
+    expect(array.pop()).toBe(1);
+    expect(array.pop()).toBeUndefined();
+  });
+
   it("reserve throws if length is less than 0", function () {
     var array = new ManagedArray();
     expect(function () {
@@ -130,6 +164,16 @@ describe("Core/ManagedArray", function () {
     expect(array.length).toEqual(5);
   });
 
+  it("resize removes trailing references", function () {
+    var length = 10;
+    var array = new ManagedArray(length);
+    array.set(0, Math.random());
+    array.set(1, Math.random());
+    array.set(2, Math.random());
+    array.resize(1);
+    expectTrailingReferenceToBeRemoved(array);
+  });
+
   it("trim", function () {
     var array = new ManagedArray(2);
     array.reserve(10);