-
Notifications
You must be signed in to change notification settings - Fork 332
Consider performance.now() #240
Comments
It seems that performance now is not desirable. https://stackoverflow.com/questions/30795525/performance-now-vs-date-now |
Can you elaborate? |
I added a stackoverflow link which says that performance.now is a magnitude slower than Date.now. This means that performance.now could mean that it could become a bottleneck. |
Is that actually an issue for benchmarking? I'd like to see an example that's problematic before dismissing it. |
We should always denote perf numbers with the version numbers of the runtime, which this SO response does not. I would recommend that @Uzlopak or someone else who is against this to provide a benchmark so that this can be evaluated. Or at least, I would say that on general principle that would be the case, but this is a benchmarking library, not code that runs in production. Who cares if it runs 50ms slower, if it's more accurate? One of the things that has perpetually confused me about NodeJS is that, for a runtime that claims to tackle the C10K problem, and pretends at the C100K problem, the resolution of performance timers being 1 millisecond until Node 8.9 makes absolutely no sense to me. When you are running thousands of requests at the same time, blocking the event queue for 1.4 ms versus 0.7 ms is a massive difference in system behavior. Benchmark has no control over this, but when someone fixes such a huge oversight upstream, you should jump on it like a canteen in the desert. I recently upgraded some tracing code that was erroneously using |
@domoritz Since My recent experience with doing this migration, almost all of the code I wrote was additional pinning tests, and moving one helper function with Feature Envy ("Refactoring", 1st Ed, Martin Fowler). I contemplated adding some truncation logic but the library function that does that gives you a string and I wasn't quite prepared to trace that or do a round trip number->string->number Most of the data was getting sent of to graphite anyway, and it was happy enough with decimal points. |
Why should I provide the proof? I dont claim, that performance.now is better. I think the burden of proof is with the person who wants to change,. |
You're the one telling people their fun is wrong. @domoritz After a bit of poking, I believe that this is the block of code where you would make that change. You'd slot that in somewhere between the other options there. Right before or right after the microtime conditional block https://github.com/bestiejs/benchmark.js/blob/master/benchmark.js#L1794 It's also clear from this code that benchmark.js rarely uses Date.now anyway, so comparing the speed between the two is probably a red herring, even ignoring the reasons I already stated above. |
I was wondering why this library doesn't seem to be using
performance.now()
.The text was updated successfully, but these errors were encountered: