Performance degradation in v2 compared to 1.2.2

[svtokarevsv]

Old package performance

v2 version performance

The new version is 5 times less performant.
Not sure if you're worried of performance on that scale, just wanted to let you know.
I'll be still using old version because of this reason.
Thanks

[niieani]

Hi @svtokarevsv. Thanks for checking this. I was aware there would be a performance impact due to added support for unicode characters. Unicode in itself wouldn't cause the perf impact, but we'd need another refactor to convert all the strings into arrays internally. Right now we're iterating the strings (hash, salt, alphabet, seps) multiple times with each encode and decode.
Changing it to arrays should fully mitigate the perf issues (perhaps even result in better output performance than v1).

I didn't think performance would be an issue, that's why I went with the current implementation.

I don't have the time to do this now, but I'm open to PRs.

[jpike88]

This performance issue blew my production servers up, this is significant. Downgrading to 1.2.2 seems to fix it

[jpike88]

Actually because I'm OCD as hell, I've upgraded back to it, and put some caching in place to minimize the amount of times it gets called

[niieani]

Sorry about that @jpike88! If anybody is up for the refactor, I'm happy to accept PRs. Just can't invest much time at the moment personally (getting married next week! 😄), but feel free to use 1.2.2 if your perf is impacted.

[JaneJeon]

In addition, during fuzz testing, I've discovered a memory leak of sorts with v2 that would literally BLOW UP the entire server, unfortunately... would be great to see this fixed.

For now I'm reverting to v1.2.2.

[niieani]

@JaneJeon that's intriguing! 🤔 We don't keep any references stashed away, so I honestly have no idea about the source of the memory leak you mentioned. Could you share the source of your testing repo?

[JaneJeon]

@niieani I don't have the exact code for the moment (mainly because it was involving a plugin that used hashids) BUT through debugger I was able to narrow down the "freeze" down to the following:

1. Instantiate a hashids instance
2. Call decode with that instance repeatedly with random strings. <--- this is the step that freezes the entire process and blows up the memory if called repeatedly.

I was using version 2.0.1, and the random strings were generated by faker.js. So, in effect, I was doing fuzz testing when step 2 broke. I only had to call the decode function <30 times for it to blow up, it usually happened around step 15-25.

[JaneJeon]

Even if you decide not to fix this issue, I still think we should add a fuzz testing of sorts to make sure it works with arbitrary strings.

[niieani]

That's a very good idea! I have limited time for OSS lately, but I'm very much open to contributions from anyone who'd like to tackle this.

[JaneJeon]

@niieani also for some reason my specific issue seemed to get fixed in v2.1.0... while it wasn't [object Object] I was decoding, fixing #126 seems to have also fixed a whole bunch of other "incorrect" ID's.

[niieani]

Great to hear that @JaneJeon. Before 2.1.0 there was a bad condition check in a while loop, so it got stuck in an endless loop. This is probably what you observed.

Anyway, I've bit the bullet and spent a few hours tuning performance (#206).

Results look really positive!

1.2.2	 	decoding	 67,839 ops/sec ±0.32% (196736 samples)
2.1.0	 	decoding	 21,974 ops/sec ±0.34% (65133 samples)
this(dist)	decoding	 95,617 ops/sec ±0.29% (274244 samples)
this		decoding	 99,728 ops/sec ±0.29% (285774 samples)

1.2.2	 	encoding	 139,561 ops/sec ±0.23% (393444 samples)
2.1.0	 	encoding	 81,586 ops/sec ±0.31% (235682 samples)
this(dist) 	encoding	 210,082 ops/sec ±0.26% (575353 samples)
this		encoding	 213,644 ops/sec ±0.25% (583851 samples)

[niieani]

🎉 This issue has been resolved in version 2.2.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀