Shrink growable arrays

runtime/lib/growable_array.dart

Index: runtime/lib/growable_array.dart
diff --git a/runtime/lib/growable_array.dart b/runtime/lib/growable_array.dart
index 39b4522c72711d9e3cb1cffd56f875dd5eec54f8..75a17643bf89b78c64732fb7f0d6ccaa5807733d 100644
--- a/runtime/lib/growable_array.dart
+++ b/runtime/lib/growable_array.dart
@@ -113,13 +113,24 @@ class _GrowableList<T> extends ListBase<T> {
   factory _GrowableList.withData(_List data)
     native "GrowableList_allocate";
 
-  int get length native "GrowableList_getLength";
-
   int get _capacity native "GrowableList_getCapacity";
 
+  int get length native "GrowableList_getLength";
+
   void set length(int new_length) {
     if (new_length > _capacity) {

[Ivan Posva, 2016/02/16 23:54:54]

Judging from Srdjan's comment we should not intermix new_length and new_capacity
interchangably here.

How about:
void set length(int new_length) {
  var new_capacity = (new_length == 0) ? _kDefaultCapacity : new_length;
  if (new_capacity > capacity) {
    ...
  }

and changing the parameter name in _grow and _shrink to be clearer (e.g.
new_capacity).

[Cutch, 2016/02/17 21:12:54]

Done.

       _grow(new_length);
+      _setLength(new_length);
+      return;
+    }
+    // We are shrinking. Pick the method which has fewer writes.
+    // In the shrink-to-fit path, we write 2 * |new_length| words
+    // (null fill + copy).

[Ivan Posva, 2016/02/16 23:54:54]

How about "(null init + copy)" and "(null overwrite)"?

[Cutch, 2016/02/17 21:12:54]

Done.

+    // In the non-shrink-to-fit path, we write |length - new_length| words
+    // (null fill).
+    final bool shouldShrinkToFit = (2 * new_length) < (length - new_length);

[Ivan Posva, 2016/02/16 23:54:54]

Which would mean here (new_capacity + new_length) < (length - new_length).

[Cutch, 2016/02/17 21:12:54]

Done.

+    if (shouldShrinkToFit) {
+      _shrink(new_length);
     } else {
       for (int i = new_length; i < length; i++) {
         this[i] = null;
@@ -225,6 +236,14 @@ class _GrowableList<T> extends ListBase<T> {
     _setData(new_data);
   }
 
+  void _shrink(int new_length) {

[srdjan, 2016/02/16 23:45:29]

if (new_length == _capacity) return

[Ivan Posva, 2016/02/16 23:54:54]

I don't think this would work here as the code above assumes that the data array
will be initialized with nulls automatically. The new call does this implicitly.

[Cutch, 2016/02/17 21:12:54]

Acknowledged.

[Cutch, 2016/02/17 21:12:54]

Acknowledged.

+    var new_data = new _List(new_length == 0 ? _kDefaultCapacity : new_length);
+    for (int i = 0; i < new_length; i++) {
+      new_data[i] = this[i];
+    }
+    _setData(new_data);
+  }
+
   // Iterable interface.
 
   void forEach(f(T element)) {