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

Sort speed increase #762

Merged
merged 3 commits into from
Feb 26, 2023
Merged

Sort speed increase #762

merged 3 commits into from
Feb 26, 2023

Conversation

djacks6278
Copy link
Contributor

The current sorting algorithm runs hget every time and compares these values,
which has the disadvantage of heavy load and slow speed.

In addition, there was a huge problem with the existing alignment method.
The problem is that the larger the library, the more exponential it requires.
The load is also severe because hget is executed every operation.
The current method also tries to sort this value against the previous value each time.

To solve this problem,
The key values of the @filtered array and the value of the key value corresponding to the sort key are converted to Hash.
The key sorted based on the value value of this hash has been returned to the @sorted array.

We can process more quickly than before.
Currently, more than 100,000 sortings are often unsuccessful, but if you change them this way, 500,000 sortings can also be successful.

Based on the same performance for 100,000 sortings, traditionally it takes more than 300 seconds and the operation fails, but the improved code takes about 13 seconds.

Copy link
Owner

@Difegue Difegue left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this really helps!
Non-title sorting was the one part of the search revamp where I kinda ran out of time and defaulted to using the old code, but this looks much leaner in resources.

This'll still do a redis call for each ID(which is much better than the previous code), but I wonder if it wouldn't be possible to squeeze a bit more speed out of this by bundling all the hget calls in a single database call using multi/exec... Certainly some food for thought for next time.

Got a few suggested comments for the code in there, please have a look at 'em and correct me if I'm wrong 🙏

lib/LANraragi/Model/Search.pm Show resolved Hide resolved
lib/LANraragi/Model/Search.pm Outdated Show resolved Hide resolved
djacks6278 and others added 2 commits February 26, 2023 11:39
Co-authored-by: Difegue <8237712+Difegue@users.noreply.github.com>
Co-authored-by: Difegue <8237712+Difegue@users.noreply.github.com>
@djacks6278
Copy link
Contributor Author

All comments are correct.
When there is no tag, we use "zzzz" as the default, but if we change it to unicode emoji, we will be able to align it more clearly.

I'd like to add other improvements, but I couldn't spend a lot of time.

How about replacing Redis module to Redis::Fast module for incremental improvement?

@Difegue
Copy link
Owner

Difegue commented Feb 26, 2023

Looks perfect, thank you! Redis::Fast might be an interesting avenue, since it looks to be API-compatible it might not break anything -- But we can think about that at a later time. 😁

@holopin-bot @djacks6278

@holopin-bot
Copy link

holopin-bot bot commented Feb 26, 2023

Congratulations @djacks6278, you just earned a badge! Here it is: https://holopin.io/claim/clele32av01220fkyq7lbaioe

This badge can only be claimed by you, so make sure that your GitHub account is linked to your Holopin account. You can manage those preferences here: https://holopin.io/account.
Or if you're new to Holopin, you can simply sign up with GitHub, which will do the trick!

@Difegue Difegue merged commit a705eeb into Difegue:dev Feb 26, 2023
@holopin-bot
Copy link

holopin-bot bot commented Feb 26, 2023

Congratulations @djacks6278, you just earned a holobyte! Here it is: https://holopin.io/holobyte/clele4xlx08190fkyt49v88kr

This badge can only be claimed by you, so make sure that your GitHub account is linked to your Holopin account. You can manage those preferences here: https://holopin.io/account.
Or if you're new to Holopin, you can simply sign up with GitHub, which will do the trick!

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.

2 participants