Adjust for API differences in bench code

[apostolos]

Benchmark code wrongly applies fixture args on classcat. That causes the actual call to be:
e.g. for ["one", "two", "three"], it currently benchmarks:

cc("one", "two", "three");

instead of:

cc(["one", "two", "three"])

This means the benchmark compares different work parameters for classcat and classNames.
Furthermore, as far as I can tell, Benchmark.Suite() does not validate function output. The expected value under ./bench/fixtures.js is ignored that's why this error was easy to miss.

This PR adjusts benchmark code to make comparison fair for both functions.

Although classcat is still faster than classNames, it is not 5x times faster. The old Strings & Objects benchmark giving such an impossible result should have been a red flag.

New results

Node 9.4.0 on an AMD Ryzen 1500X

Old Classcat – Strings × 17,286,017 ops/sec
Actual Classcat – Strings × 7,947,812 ops/sec
classNames – Strings × 4,261,603 ops/sec
Fastest is Old Classcat – Strings
Fastest is Actual Classcat – Strings

Old Classcat – Objects × 5,754,260 ops/sec
Actual Classcat – Objects × 7,158,794 ops/sec
classNames – Objects × 3,634,329 ops/sec
Fastest is Actual Classcat – Objects

Old Classcat – Strings & Objects × 16,294,161 ops/sec
Actual Classcat – Strings & Objects × 4,732,941 ops/sec
classNames – Strings & Objects × 3,325,816 ops/sec
Fastest is Old Classcat – Strings & Objects
Fastest is Actual Classcat – Strings & Objects

Old Classcat – Mixed × 7,319,186 ops/sec
Actual Classcat – Mixed × 2,645,379 ops/sec
classNames – Mixed × 2,356,888 ops/sec
Fastest is Old Classcat – Mixed
Fastest is Actual Classcat – Mixed

Old Classcat – Arrays × 2,007,313 ops/sec
Actual Classcat – Arrays × 2,254,936 ops/sec
classNames – Arrays × 912,869 ops/sec
Fastest is Actual Classcat – Arrays

[codecov-io]

Codecov Report

Merging #17 into master will not change coverage.
The diff coverage is n/a.

@@          Coverage Diff          @@
##           master    #17   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           1      1           
  Lines          16     16           
  Branches       11     11           
=====================================
  Hits           16     16

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 586d67a...8bc38be. Read the comment docs.

[jorgebucaran]

Thanks, @apostolos! Maybe we need to tweak the benchmarks a bit more.

Classcat is meant to be used as:

cc(["foo", "bar", "baz"]) // => foo bar baz

whereas classNames is like:

cc("foo", "bar", "baz") // => foo

So, it would be great if we could bench each function how they are supposed to be used.

[apostolos]

I think that's what I'm doing already. You'll notice that before adding a benchmark function to the Suite, I first measure the args[] length:

• If args has more than one item I pass args to classcat as a single argument in order to be used as it should be, since args is already an array! For classNames I left it like before, since a classNames user would pass args.length number of arguments to the function.

• However, when args has a single argument, it is unfair to pass an array to classcat. That's why I pass just the first element (args[0]). That way, we avoid penalizing classcat with an unnecessary iteration over a single-element array (in this case the "Objects" test).

You'll also notice that I pass different functions for each case to the suite to avoid measuring the args.length check.

[apostolos]

UPDATE:

I refactored the code a bit:

• A single object argument is passed for both classNames and classcat in the "Objects" test
• In multi-item tests, the args[] array:
  • is passed directly to classcat
  • it is expanded as arguments with apply to classNames

That way we avoid any potential object/array lookups when not needed and function calls are always how they are supposed to be used.

New bench numbers (on a i7-6500U @ 2.50GHz):

Classcat – Strings × 8,213,876 ops/sec
classNames – Strings × 3,677,145 ops/sec
Fastest is Classcat – Strings

Classcat – Objects × 7,448,948 ops/sec
classNames – Objects × 3,681,655 ops/sec
Fastest is Classcat – Objects

Classcat – Strings & Objects × 5,119,115 ops/sec
classNames – Strings & Objects × 3,015,089 ops/sec
Fastest is Classcat – Strings & Objects

Classcat – Mixed × 2,983,494 ops/sec
classNames – Mixed × 2,228,106 ops/sec
Fastest is Classcat – Mixed

Classcat – Arrays × 2,550,737 ops/sec
classNames – Arrays × 910,095 ops/sec
Fastest is Classcat – Arrays

[jorgebucaran]

@apostolos Okay, I finally a chance to have a look at this and you were totally right! I still think your code can be simplified a bit more, so I'll just take care of it. 😅

I want to rewrite the bench this way instead:

suite
  .add(`Classcat – ${fixed.description}`, () => cc(fixed.args))
  .add(`classNames – ${fixed.description}`, () => cx.apply({}, fixed.args))

[jorgebucaran]

@apostolos I've updated the benchmarks and added the new (correct) results to #16. Thanks! 👋