Well, LGTM, but I never liked the current solution anyway :)


http://codereview.chromium.org/1513020/diff/7001/8001
File src/array.js (right):

http://codereview.chromium.org/1513020/diff/7001/8001#newcode649
src/array.js:649: var element = a[i];
Come to think of it, using binary search seems overkill, since we are
going to read and write every element after the found position anyway
when we place the element.
Just do a backwards search-and-copy until finding the correct location.
I.e, something like:

  for (var j = i - 1; j >= from; j--) {
    var tmp = a[j];
    var order = tmp === element ? 0 : comparefn.call(null,tmp,element);
    if (order > 0) {
      a[j + 1] = tmp;
    } else if (order == 0) {
      j--;
      break;
    }
  }
  a[j + 1] = element;

http://codereview.chromium.org/1513020/diff/7001/8001#newcode658
src/array.js:658: 0 : comparefn.call(null, a[mid], element);
Reading a[mid] twice. Maybe assign to temporary variable.

http://codereview.chromium.org/1513020/diff/7001/8001#newcode660
src/array.js:660: min = max = mid;
No need to assign to max.

http://codereview.chromium.org/1513020/diff/7001/8001#newcode716
src/array.js:716: // If it isn't, we are allowed arbitrary behavior by
ECMA 15.4.4.11.
Comment doesn't make sense any more (not here, at least).

http://codereview.chromium.org/1513020

--
v8-dev mailing list
[email protected]
http://groups.google.com/group/v8-dev

To unsubscribe, reply using "remove me" as the subject.

Reply via email to