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

Slightly improve performance #57

Merged
merged 8 commits into from
May 17, 2020
Merged

Slightly improve performance #57

merged 8 commits into from
May 17, 2020

Conversation

fregante
Copy link
Collaborator

@fregante fregante commented May 11, 2020

I made 2 minor changes that improve performance based on tests here: fregante/many-keys-map#1

The difference with nano-memoize is still ridiculous

A single primitive parameter...

 ┌──────────────────────┬─────────────┬──────────────────────────┬─────────────┐
 │ Name                 │ Ops / sec   │ Relative margin of error │ Sample size │
 ├──────────────────────┼─────────────┼──────────────────────────┼─────────────┤
-│ nano-memoize         │ 155,232,246 │ ± 0.50%                  │ 89          │
+│ nano-memoize         │ 155,182,282 │ ± 0.54%                  │ 90          │
 ├──────────────────────┼─────────────┼──────────────────────────┼─────────────┤
-│ mem                  │ 121,333,588 │ ± 0.57%                  │ 86          │
+│ mem                  │ 117,460,667 │ ± 0.40%                  │ 91          │
 ├──────────────────────┼─────────────┼──────────────────────────┼─────────────┤
-│ mem (JSON.stringify) │ 2,867,593   │ ± 0.72%                  │ 92          │
+│ mem (JSON.stringify) │ 2,970,036   │ ± 0.53%                  │ 84          │
 ├──────────────────────┼─────────────┼──────────────────────────┼─────────────┤
-│ mem (ManyKeysMap)    │ 994,907     │ ± 0.48%                  │ 88          │
+│ mem (ManyKeysMap)    │ 2,063,514   │ ± 0.49%                  │ 85          │
 └──────────────────────┴─────────────┴──────────────────────────┴─────────────┘
A single object parameter...

 ┌──────────────────────┬────────────┬──────────────────────────┬─────────────┐
 │ Name                 │ Ops / sec  │ Relative margin of error │ Sample size │
 ├──────────────────────┼────────────┼──────────────────────────┼─────────────┤
-│ nano-memoize         │ 91,343,897 │ ± 0.52%                  │ 90          │
+│ nano-memoize         │ 91,368,910 │ ± 0.60%                  │ 89          │
 ├──────────────────────┼────────────┼──────────────────────────┼─────────────┤
-│ mem                  │ 54,521,520 │ ± 0.49%                  │ 92          │
+│ mem                  │ 72,861,909 │ ± 0.47%                  │ 91          │
 ├──────────────────────┼────────────┼──────────────────────────┼─────────────┤
-│ mem (JSON.stringify) │ 1,409,511  │ ± 0.54%                  │ 91          │
+│ mem (JSON.stringify) │ 1,512,004  │ ± 0.79%                  │ 87          │
 ├──────────────────────┼────────────┼──────────────────────────┼─────────────┤
-│ mem (ManyKeysMap)    │ 812,027    │ ± 0.51%                  │ 89          │
+│ mem (ManyKeysMap)    │ 1,594,672  │ ± 0.52%                  │ 93          │
 └──────────────────────┴────────────┴──────────────────────────┴─────────────┘
Multiple parameters that contain only primitives...

 ┌──────────────────────┬────────────┬──────────────────────────┬─────────────┐
 │ Name                 │ Ops / sec  │ Relative margin of error │ Sample size │
 ├──────────────────────┼────────────┼──────────────────────────┼─────────────┤
-│ nano-memoize         │ 64,628,754 │ ± 0.40%                  │ 91          │
+│ nano-memoize         │ 64,243,965 │ ± 0.42%                  │ 88          │
 ├──────────────────────┼────────────┼──────────────────────────┼─────────────┤
-│ mem                  │ 4,866,665  │ ± 0.41%                  │ 92          │
+│ mem                  │ 5,377,428  │ ± 0.42%                  │ 90          │
 ├──────────────────────┼────────────┼──────────────────────────┼─────────────┤
-│ mem (JSON.stringify) │ 2,554,170  │ ± 0.45%                  │ 91          │
+│ mem (JSON.stringify) │ 2,650,342  │ ± 0.60%                  │ 88          │
 ├──────────────────────┼────────────┼──────────────────────────┼─────────────┤
-│ mem (ManyKeysMap)    │ 857,537    │ ± 0.61%                  │ 89          │
+│ mem (ManyKeysMap)    │ 1,729,710  │ ± 0.57%                  │ 90          │
 └──────────────────────┴────────────┴──────────────────────────┴─────────────┘
Multiple parameters that contain objects...

 ┌──────────────────────┬────────────┬──────────────────────────┬─────────────┐
 │ Name                 │ Ops / sec  │ Relative margin of error │ Sample size │
 ├──────────────────────┼────────────┼──────────────────────────┼─────────────┤
-│ nano-memoize         │ 61,496,064 │ ± 0.43%                  │ 93          │
+│ nano-memoize         │ 49,341,612 │ ± 4.80%                  │ 74          │
 ├──────────────────────┼────────────┼──────────────────────────┼─────────────┤
-│ mem (JSON.stringify) │ 1,304,533  │ ± 0.45%                  │ 88          │
+│ mem (JSON.stringify) │ 1,463,835  │ ± 0.59%                  │ 90          │
 ├──────────────────────┼────────────┼──────────────────────────┼─────────────┤
-│ mem (ManyKeysMap)    │ 747,594    │ ± 0.58%                  │ 92          │
+│ mem (ManyKeysMap)    │ 1,491,674  │ ± 0.48%                  │ 90          │
 └──────────────────────┴────────────┴──────────────────────────┴─────────────┘
Multiple parameters that contain *large* objects...

 ┌──────────────────────┬────────────┬──────────────────────────┬─────────────┐
 │ Name                 │ Ops / sec  │ Relative margin of error │ Sample size │
 ├──────────────────────┼────────────┼──────────────────────────┼─────────────┤
-│ nano-memoize         │ 61,360,571 │ ± 0.46%                  │ 90          │
+│ nano-memoize         │ 52,345,749 │ ± 0.44%                  │ 92          │
 ├──────────────────────┼────────────┼──────────────────────────┼─────────────┤
-│ mem (ManyKeysMap)    │ 746,505    │ ± 0.56%                  │ 91          │
+│ mem (ManyKeysMap)    │ 1,460,393  │ ± 0.55%                  │ 91          │
 ├──────────────────────┼────────────┼──────────────────────────┼─────────────┤
-│ mem (JSON.stringify) │ 403,795    │ ± 0.43%                  │ 91          │
+│ mem (JSON.stringify) │ 377,963    │ ± 0.49%                  │ 83          │
 └──────────────────────┴────────────┴──────────────────────────┴─────────────┘
Multiple parameters that contain *very large* objects...

 ┌──────────────────────┬────────────┬──────────────────────────┬─────────────┐
 │ Name                 │ Ops / sec  │ Relative margin of error │ Sample size │
 ├──────────────────────┼────────────┼──────────────────────────┼─────────────┤
-│ nano-memoize         │ 52,493,314 │ ± 0.39%                  │ 92          │
+│ nano-memoize         │ 33,918,764 │ ± 1.54%                  │ 87          │
 ├──────────────────────┼────────────┼──────────────────────────┼─────────────┤
-│ mem (ManyKeysMap)    │ 748,245    │ ± 0.58%                  │ 85          │
+│ mem (ManyKeysMap)    │ 1,446,014  │ ± 0.48%                  │ 91          │
 ├──────────────────────┼────────────┼──────────────────────────┼─────────────┤
-│ mem (JSON.stringify) │ 116,677    │ ± 0.57%                  │ 89          │
+│ mem (JSON.stringify) │ 123,815    │ ± 0.45%                  │ 93          │
 └──────────────────────┴────────────┴──────────────────────────┴─────────────┘

We're using the value right away.
We can skip .has because the value will never be falsy since we're storing an object.
@sindresorhus
Copy link
Owner

Regular reminder that micro-benchmarks do not reflect real-world usage and should be taken with a grain of salt.

@sindresorhus
Copy link
Owner

I'm happy to accept these changes, but I'm not really worried about the difference with nano-memoize.

@fregante fregante marked this pull request as ready for review May 16, 2020 11:58
@fregante
Copy link
Collaborator Author

This is good to go. I tried other changes but they did not improve performance

@fregante
Copy link
Collaborator Author

fregante commented May 16, 2020

It's best to merge #58 first. I'll rebase this

@fregante fregante marked this pull request as draft May 16, 2020 12:05
@fregante fregante changed the base branch from master to revert-56-Auto-WeakMap May 16, 2020 12:05
@fregante fregante changed the base branch from revert-56-Auto-WeakMap to master May 16, 2020 19:22
@fregante fregante marked this pull request as ready for review May 16, 2020 20:06
@sindresorhus sindresorhus merged commit ea88c5c into master May 17, 2020
@sindresorhus sindresorhus deleted the need-for-speed branch May 17, 2020 06:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants