Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tests are done without checking the results. #14

Open
Pokute opened this issue Sep 10, 2019 · 5 comments
Open

Tests are done without checking the results. #14

Pokute opened this issue Sep 10, 2019 · 5 comments

Comments

@Pokute
Copy link

Pokute commented Sep 10, 2019

The results are practically invalid since the tests don't actually care about the results. This means that javascript engines can and will optimise away critical code.

*************** Map performance check ***************
js map: 15.636ms
for loop: 8.461ms
for each: 17.376ms
lodash map: 18.545ms
*************** Map performance check with result check ***************
js map: 6.106ms
sum: 49886314
for loop: 22.985ms
sum: 49886314
for each: 7.250ms
sum: 49886314
lodash map: 27.305ms
sum: 49886314

The latter case is where I just console.log the sum of commentCounts after each run and outside the timing blocks. This completely turns the results around.

@dg92
Copy link
Owner

dg92 commented Sep 10, 2019

@Pokute if you spend time reading the note in readme then you might not have to create an issue
Note
These results are computed using Node V8 v5.8.283.41
These result does not consider the JIT, inline caching, hidden classes, deoptimizations, garbage collection, pretenuring etc.
Result may vary as per env's.

Hope that answer the variation

@dg92
Copy link
Owner

dg92 commented Sep 10, 2019

@Pokute
Copy link
Author

Pokute commented Sep 10, 2019

This is more of a case of optimisation or dead code elimination.

   let foo = 10;
   foo = /* InsanelyComplicatedAndIntensiveCalculationWithNoSideEffects. */
   foo = 20;

A good JS engine will completely skip the first two assignments.

The fact that results change so dramatically from code that's outside the timing block is a telltale sign that some dead code elimination is taking place. But this test case's purpose is not to measure dead code elimination. It's purpose is to inform which method is the fastest for users, who use the code in real world situations where they do care about and do use the end result of map operation and thus dead code elimination will not occur.

@Pokute
Copy link
Author

Pokute commented Sep 10, 2019

@dg92
I strongly suggest you watch the following video on profiling / optimisations:
LXJS 2013 - Vyacheslav Egorov - Performance and benchmarking

@dg92
Copy link
Owner

dg92 commented Sep 10, 2019

@Pokute sure thanks for sharing.

Open to accept the MR if you feel it will bring a positive change.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants