Optimize List.toList/.sublist and List.from on lists.

Optimize List.toList/.sublist and List.from on lists.

R=fschneider@google.com, srdjan@google.com

Committed: https://code.google.com/p/dart/source/detail?r=39579

[Lasse Reichstein Nielsen, 2014-08-19 12:48:31]

https://codereview.chromium.org/485043002/diff/1/runtime/lib/array.dart
File runtime/lib/array.dart (right):

https://codereview.chromium.org/485043002/diff/1/runtime/lib/array.dart#newco...
runtime/lib/array.dart:70: if (ClassID.getID(iterable) ==
ClassID.cidOneByteString) {
This looks wrong: OneByteString?

https://codereview.chromium.org/485043002/diff/1/runtime/lib/class_id.dart
File runtime/lib/class_id.dart (right):

https://codereview.chromium.org/485043002/diff/1/runtime/lib/class_id.dart#ne...
runtime/lib/class_id.dart:7: }
Ack, accidental whitespace change.

https://codereview.chromium.org/485043002/diff/1/runtime/lib/growable_array.dart
File runtime/lib/growable_array.dart (right):

https://codereview.chromium.org/485043002/diff/1/runtime/lib/growable_array.d...
runtime/lib/growable_array.dart:155: void _setData(_List array) native
"GrowableList_setData";
If there was a _getData, we could use _List._copyFromObjectArray to copy into
it. Does that sound like a useful think, or is it too dangeous?

[Lasse Reichstein Nielsen, 2014-08-21 11:04:25]

Should be stable now, PTAL.

[srdjan, 2014-08-22 18:31:51]

LGTM after verifying that there are no performance regressions.

[Lasse Reichstein Nielsen, 2014-08-26 13:06:26]

lrn@google.com changed reviewers:
+ fschneider@google.com

[Florian Schneider, 2014-08-26 13:17:06]

LGTM.

The crash would have been caught by our assertions if the setLength method was
not intrinsified because intrinsic code does not check the arguments even in
DEBUG mode.

https://codereview.chromium.org/485043002/diff/120001/runtime/lib/array.dart
File runtime/lib/array.dart (right):

https://codereview.chromium.org/485043002/diff/120001/runtime/lib/array.dart#...
runtime/lib/array.dart:514: var length = this.length;
Also check for length > 0 here?

https://codereview.chromium.org/485043002/diff/120001/runtime/lib/growable_ar...
File runtime/lib/growable_array.dart (right):

https://codereview.chromium.org/485043002/diff/120001/runtime/lib/growable_ar...
runtime/lib/growable_array.dart:238: //    if (new_length < _kDefaultCapacity)
new_length = _kDefaultCapacity;
Remove commented code?

[Lasse Reichstein Nielsen, 2014-08-26 13:46:53]

https://codereview.chromium.org/485043002/diff/120001/runtime/lib/array.dart
File runtime/lib/array.dart (right):

https://codereview.chromium.org/485043002/diff/120001/runtime/lib/array.dart#...
runtime/lib/array.dart:514: var length = this.length;
Yes, good catch.

https://codereview.chromium.org/485043002/diff/120001/runtime/lib/growable_ar...
File runtime/lib/growable_array.dart (right):

https://codereview.chromium.org/485043002/diff/120001/runtime/lib/growable_ar...
runtime/lib/growable_array.dart:238: //    if (new_length < _kDefaultCapacity)
new_length = _kDefaultCapacity;
Or uncomment it, as was the intention.
I just commented it out to check the performance.

Probably safer to remove it now, and do some more performance checks before
adding it again.

[Lasse Reichstein Nielsen, 2014-08-27 08:01:00]

Committed patchset #8 manually as 39579 (presubmit successful).