events: improve removeListener() performance

[mscdex]

array.shift() seems to be faster than arrayClone() when the item to remove is at the front (at least with V8 5.4).

Comparing the results of the events/ee-add-remove.js benchmark shows ~7% improvement:

                                   improvement significant      p.value
 events/ee-add-remove.js n=2500000      7.25 %         *** 8.925155e-24

NOTE: I have not yet tested array.shift() performance on V8<5.4, so this may/may not be applicable to node versions prior to 7.0.0.

CI: https://ci.nodejs.org/job/node-test-pull-request/5660/

Checklist

• make -j4 test (UNIX), or vcbuild test (Windows) passes
• commit message follows commit guidelines

Affected core subsystem(s)

• events

[benjamingr]

.splice and .shift use the same code, namely: .shift() is:

  var first = array[0];

  if (UseSparseVariant(array, len, IS_ARRAY(array), len)) {
    SparseMove(array, 0, 1, len, 0);
  } else {
    SimpleMove(array, 0, 1, len, 0);
  }

  array.length = len - 1;

  return first;

And splice is:

 if (UseSparseVariant(array, len, IS_ARRAY(array), changed_elements)) {
    %NormalizeElements(array);
    if (IS_ARRAY(deleted_elements)) %NormalizeElements(deleted_elements);
    SparseSlice(array, start_i, del_count, len, deleted_elements);
    SparseMove(array, start_i, del_count, len, num_elements_to_add);
  } else {
    SimpleSlice(array, start_i, del_count, len, deleted_elements);
    SimpleMove(array, start_i, del_count, len, num_elements_to_add);
  }

We only care about the simple case, so .splice is:

SimpleSlice(array, start_i, del_count, len, deleted_elements);
SimpleMove(array, start_i, del_count, len, num_elements_to_add);

Which should be about the same amount of work given the slice does mostly nothing in this case. That said, .splice has a lot more boilerplate around the implementation.

Anyway - sorry for the storytelling - change LGTM and can confirm the benchmark - just wanted to validate in code.

[mhdawson]

LGTM

[jasnell]

Previous CI run had some red... running again just to make sure: https://ci.nodejs.org/job/node-test-pull-request/5700/

[MylesBorins]

@mscdex how long should this bake before lts?