Don't always call _copyFromObjectArray.

Don't always call _copyFromObjectArray.

The call is a runtime call, not internalized as I expected.
Only use it for large array slices.

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

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

[Lasse Reichstein Nielsen, 2014-09-02 14:27:52]

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

[Lasse Reichstein Nielsen, 2014-09-02 14:29:10]

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

https://codereview.chromium.org/518273004/diff/1/runtime/lib/array.dart#newco...
runtime/lib/array.dart:72: _copyFromObjectArray(iterable, skipCount, start,
length);
Good thing this wasn't called before due to the incorrect ClassID - it could do
the wrong thing if called with itself.

Currently, I'm not sure this call ever pays for itself, I'll have to check it
again when _copyFromObjectArray has been optimized.

[Vyacheslav Egorov (Google), 2014-09-02 14:31:15]

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

https://codereview.chromium.org/518273004/diff/1/runtime/lib/array.dart#newco...
runtime/lib/array.dart:106: if (length < 128) {
I don't like that this is repeated. 

Can we have a _copyArray() somewhere that does

if (length < ...) {
  // ... do this ...
} else {
  list._copyFromObjectArray(...)
}

?

This will open possibilities for uniform optimizations instead of searching for
other places that use it...

[Vyacheslav Egorov (Google), 2014-09-02 14:32:22]

Maybe I suggest actually making _copyFromObjectArray a Dart routine that does
length check and renaming current native to _copyFromObjectArrayImpl?

Also you can add _copyFromObjectArray to the inlining whitelist for the best
performance.

[Lasse Reichstein Nielsen, 2014-09-02 14:34:28]

Good idea. I'll move the code into _copyFromObjectArray, and make the native
function one step further down. Good thing the function wasn't called something
with "native" :)

[srdjan, 2014-09-02 18:33:57]

srdjan@google.com changed reviewers:
+ srdjan@google.com

[srdjan, 2014-09-02 18:33:58]

LGTM after addressing Slava's comments and checking that there are no
regressions.

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

https://codereview.chromium.org/518273004/diff/1/runtime/lib/array.dart#newco...
runtime/lib/array.dart:71: } else if (ClassID.getID(iterable) ==
ClassID.cidArray && length > 128) {
Please add parentheses.

[Vyacheslav Egorov (Google), 2014-09-03 12:37:58]

LGTM

[Lasse Reichstein Nielsen, 2014-09-03 12:39:43]

Committed patchset #3 (id:40001) manually as 39803 (presubmit successful).