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

Use string concatenation, not arrays #292

Closed
wants to merge 1 commit into from

Conversation

fromaline
Copy link

It seems like string concatenation is more performant than array manipulation, so I replaced classes.push( & classes.join(' ') with classes += ' ' +arg & classes.trimStart() in the core classnames function.

All tests are passing, no new features were added. so I didn't add any new tests.

jsperf.com from CONTRIBUTING.md doesn't work, so I tested on my machine and see massive performance gain in all cases:
(local is the default, my is the changed one)

* local#strings x 6,245,646 ops/sec ±1.00% (94 runs sampled)
* my#strings x 9,570,213 ops/sec ±2.59% (97 runs sampled)

> Fastest is my#strings

* local#object x 7,165,102 ops/sec ±4.42% (95 runs sampled)
* my#object x 12,929,752 ops/sec ±3.28% (99 runs sampled)

> Fastest is my#object

* local#strings, object x 5,413,534 ops/sec ±3.46% (90 runs sampled)
* my#strings, object x 7,503,844 ops/sec ±2.32% (86 runs sampled)

> Fastest is my#strings, object

* local#mix x 3,176,253 ops/sec ±2.91% (89 runs sampled)
* my#mix x 4,480,843 ops/sec ±2.08% (92 runs sampled)

> Fastest is my#mix

* local#arrays x 1,282,299 ops/sec ±1.22% (93 runs sampled)
* my#arrays x 2,133,194 ops/sec ±1.08% (96 runs sampled)

> Fastest is my#arrays

seems like string concatenation is more performant than array manipulation
@dcousens
Copy link
Collaborator

dcousens commented Oct 29, 2022

Similar strategy to #197, but without all the other changes. Nice pull request @fromaline.

If I can reproduce this improvement in a number of different, modern environments, then I see no reason why we won't merge.
The problem has always been that this "improvement" is variable between environments.

@dcousens dcousens changed the title Improve performance of the core function Use string concatenation, not arrays Oct 29, 2022
@dcousens dcousens self-assigned this Oct 29, 2022
@fromaline
Copy link
Author

Oh, get it, thanks. I’m here if you need any help on my side.

Btw, thanks for the awesome library, I use it on the daily basis and it’s very helpful.

@fromaline
Copy link
Author

fromaline commented Nov 2, 2022

Any updates on this? Do you need help with benchmarking this “improvement” in different environments?

@dcousens
Copy link
Collaborator

dcousens commented Nov 3, 2022

Do you need help with benchmarking this “improvement” in different environments?

Yes 💛

How to manifest that help at the moment, I'm not sure?
Maybe we could integrate these measurements into CI, or maybe you can verify on your end that the results are consistent across what browsers you have available?

In any event, the help is really appreciated 🙏

@fromaline
Copy link
Author

Ok, I'll think about it, try different approaches and let you know once done.

@fromaline
Copy link
Author

fromaline commented Nov 3, 2022

I went with the simplest solution, hope it's enough for now. Current benchmarks don't work for me, so I created new ones and extend existing cases. I run all tests on MacBook Air M1, 2020 (8 GB, macOS 12.4). You may find my benchmarks here.

The results:

As you said, this "improvement" is variable between environments, so I'm not sure now if we need it at all.

Chrome

Case strings:
- array-based cn is 4,014,452.027 op/sec
- string-based cn is 4,278,990.158 op/sec
Conclusion: string-based cn is faster by 6.18%

Case object:
- array-based cn is 5,621,135.469 op/sec
- string-based cn is 5,425,935.974 op/sec
Conclusion: array-based cn is faster by 3.47%

Case strings & objects:
- array-based cn is 4,985,044.865 op/sec
- string-based cn is 4,743,833.017 op/sec
Conclusion: array-based cn is faster by 4.84%

Case arrays:
- array-based cn is 2,382,086.708 op/sec
- string-based cn is 2,857,959.417 op/sec
Conclusion: string-based cn is faster by 16.65%

Case strings & objects & arrays:
- array-based cn is 3,121,098.627 op/sec
- string-based cn is 3,434,065.934 op/sec
Conclusion: string-based cn is faster by 9.11%

Safari

Case strings:
- array-based cn is 2,512,562.814 op/sec
- string-based cn is 2,506,265.664 op/sec
Conclusion: array-based cn is faster by 0.25%

Case object:
- array-based cn is 2,610,966.057 op/sec
- string-based cn is 2,673,796.791 op/sec
Conclusion: string-based cn is faster by 2.35%

Case strings & objects:
- array-based cn is 2,570,694.087 op/sec
- string-based cn is 2,525,252.525 op/sec
Conclusion: array-based cn is faster by 1.77%

Case arrays:
- array-based cn is 1,082,251.082 op/sec
- string-based cn is 1,132,502.831 op/sec
Conclusion: string-based cn is faster by 4.44%

Case strings & objects & arrays:
- array-based cn is 1,618,122.977 op/sec
- string-based cn is 1,631,321.37 op/sec
Conclusion: string-based cn is faster by 0.81%

Firefox

Case strings:
- array-based cn is 3,257,328.99 op/sec
- string-based cn is 2,493,765.586 op/sec
Conclusion: array-based cn is faster by 23.44%

Case object:
- array-based cn is 2,840,909.091 op/sec
- string-based cn is 2,040,816.327 op/sec
Conclusion: array-based cn is faster by 28.16%

Case strings & objects:
- array-based cn is 2,881,844.38 op/sec
- string-based cn is 2,057,613.169 op/sec
Conclusion: array-based cn is faster by 28.60%

Case arrays:
- array-based cn is 2,169,197.397 op/sec
- string-based cn is 1,560,062.402 op/sec
Conclusion: array-based cn is faster by 28.08%

Case strings & objects & arrays:
- array-based cn is 1,626,016.26 op/sec
- string-based cn is 1,340,482.574 op/sec
Conclusion: array-based cn is faster by 17.56%

Node.js

Case strings:
- array-based cn is 3,467,621.808 op/sec
- string-based cn is 3,949,787.021 op/sec
Conclusion: string-based cn is faster by 12.21%

Case object:
- array-based cn is 4,705,921.108 op/sec
- string-based cn is 5,016,186.607 op/sec
Conclusion: string-based cn is faster by 6.19%

Case strings & objects:
- array-based cn is 4,113,779.608 op/sec
- string-based cn is 4,570,430.034 op/sec
Conclusion: string-based cn is faster by 9.99%

Case arrays:
- array-based cn is 1,960,371.898 op/sec
- string-based cn is 2,788,989.259 op/sec
Conclusion: string-based cn is faster by 29.71%

Case strings & objects & arrays:
- array-based cn is 2,556,558.534 op/sec
- string-based cn is 3,259,376.282 op/sec
Conclusion: string-based cn is faster by 21.56%

Deno

Case strings:
- array-based cn is 3,649,635.036 op/sec
- string-based cn is 3,649,635.036 op/sec
Conclusion: string-based cn is faster by 0.00%

Case object:
- array-based cn is 4,950,495.05 op/sec
- string-based cn is 4,672,897.196 op/sec
Conclusion: array-based cn is faster by 5.61%

Case strings & objects:
- array-based cn is 4,629,629.63 op/sec
- string-based cn is 4,166,666.667 op/sec
Conclusion: array-based cn is faster by 10.00%

Case arrays:
- array-based cn is 2,109,704.641 op/sec
- string-based cn is 2,380,952.381 op/sec
Conclusion: string-based cn is faster by 11.39%

Case strings & objects & arrays:
- array-based cn is 2,673,796.791 op/sec
- string-based cn is 2,824,858.757 op/sec
Conclusion: string-based cn is faster by 5.35%

@dcousens
Copy link
Collaborator

dcousens commented Nov 4, 2022

This is awesome work @fromaline, if not a little disappointing that it doesn't lend itself to an easy merge. 💛

@fromaline
Copy link
Author

Oh, do you want to merge my benchmarks?
I can polish them and merge into main branch in forked repo first, if needed.

@dcousens
Copy link
Collaborator

dcousens commented Nov 4, 2022

Current benchmarks don't work for me

Out of interest, what went wrong?

@fromaline
Copy link
Author

browserify didn’t work for some reason
maybe I skipped some important step, ‘cause I never used it before

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

Successfully merging this pull request may close these issues.

None yet

2 participants