Fixing (and improving) internal benchmarks

[fturmel]

As pointed out in #1846, it appears that the internal benchmarking system is broken.

Would PRs for this be welcome? Any guidelines?

A few thoughts beyond just restoring functionality:

• Add more relevant comparisons like day.js and luxon since moment is somewhat deprecated for new projects
• Publish comparative results in docs with every release (table with relative values). Show improvements / regressions. Benchmarks are low value if the data is not publicly available and we can't see trends over time imo.
• Convert benchmark files to TypeScript (assuming it's possible)
• Determine what is not being benchmarked and work towards a more complete suite
• Is benchmark.js still the best tool for the job in 2021? I don't have a suggestion to make, but the last stable release was 4 years ago. Maybe headless Chrome/v8 would be better than benchmarkjs/phantomjs, could karma-chrome-launcher be leveraged for benchmarking too?

We can start small, or work on proof of concepts first. Let me know what you think.

[tan75]

hi @fturmel
This makes a lot of sense however it's not our priority atm therefore we would appreciate any help with that.
Certainly, PRs are always welcome!

Please see my answers below:

1. Good idea.
2. Good idea once the comparison mentioned above is available. Requires a lot of work.
3. Good idea.
4. Good point.
5. At the moment we have no idea on this one and would appreciate any research done.
   Please let us know if you really want to work on this one.

[fturmel]

OK, let me start by fixing the current benchmarks setup to get more familiar with it and close #1846. I have some time at the moment (but maybe not for long), so I can at least commit to that.

To add a point to the list, another possibility could be to extract benchmarking into a separate repo / project altogether. Definitely comes with its own set of pros and cons.

BTW, #289 introduced the benchmarks, and it seems like there might've been plans to do more. I wonder what happened... that was 4 and a half years ago. There was a mention of some dedicated infrastructure to run the benchmarks, is this in place but private, or did it simply never materialize?

[fturmel]

Update: I've been playing around with an interesting approach today. I have a lot more notes and cases to cover but I'm pretty excited about it.

• TypeScript, ts-node and benchmark.js only. This means decoupling benchmarking from karma and the build process, simplifying things a lot
• ts-node is running the benchmarked code about 7x faster than phantomjs on my machine, so we can expect benchmarks to finish quicker (see terminals screencaps below, roughly 1MM vs 7MM ops/sec for the add method).
• Very lean implementation, most of the work is happening in an abstract class and hiding library imports, benchmark framework integration, etc. It would make any refactoring super easy as well. If we want to dump benchmark.js and rewrite that part, none of the individual benchmark implementations would need to be rewritten.
• Optional smoke tests to verify that all implementations are functionally equivalent. (return values need to match goal value). Needs a strategy to be able to compare things that are not dates (ex: string output of format, utilities returning booleans, etc). Also need a way to differentiate a missing implementation from an impossible one because a library doesn't support a certain feature (prob return types or throw a specific exception).
• Can control easily which library implementations are required or optional. It's easy to add new libraries to test against too.
• "Native" benchmark would give us a good reference point to compare against what a realistic peak score could be.
• It will be easy to write some tooling to tell us which date-fns methods have or are missing benchmarks (and specifically which libraries for which date-fns methods are in place or missing)
• It will also be easy to output a more substantial JSON report than what was currently in place

More soon!

Implementation example for add, filename is add.bench.ts in ./benchmarks directory

Terminal output (the luxon implementation is just returning new Date(), so not matching the goal value and failing smoke test)

We can leave out optional library benchmarks...

...but not required ones

If you need a refresher, this what terminal output and (broken) implementation looks like now:

Current implementation More verbose, return values unused, string names are actually functional names in the JSON reporter so a typo could break things, everything loosely typed, need to import all libraries manually, closely coupled with benchmark.js etc

Current output (after installing missing benchmarkjs dependency it runs but this.date and this.moment are undefined)

[fturmel]

Super quick addition, I think using generics on the abstract class might be the way to go for changing the comparison type of the smoke tests.

Expecting Date

Expecting String