bestiejs/benchmark.js

Consider using symbols for tracing data

jdmarshall opened this issue · 0 comments

This week I was writing a benchmark for some of our tracing code which I'm trying to refactor and speed up.

I stared at this error for far too long:

/Users/jason.marshall/Projects/foo/node_modules/benchmark/benchmark.js:1901
            size = sample.push(clone.times.period),
                          ^

TypeError: Cannot read property 'push' of undefined
    at Array.evaluate (/Users/jason.marshall/Projects/foo/node_modules/benchmark/benchmark.js:1901:27)

After a night's sleep it dawned on me that I might be clobbering internal state:

            event.target.stats = new ...;

The fact that the error doesn't talk about 'stats' kept me from connecting the dots.

If Benchmark is assuming ES6, which I believe it does, it might make sense for the bookkeeping data to be stored under symbols instead of as regular property names. That decreases the likelihood of someone accidentally changing internal state on the object.

Alternatively, are there other ways to do setup for a benchmark? The offending code above I believe was based off of following examples in the docs. If there's a safer way to write an onStart I'd be happy to file a PR to fix the docs.