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

Improve sync speed and updated dep. versions #2429

Merged
merged 1 commit into from
May 11, 2022

Conversation

BlackDex
Copy link
Collaborator

@BlackDex BlackDex commented Apr 22, 2022

Improved sync speed by resolving the N+1 query issues.
Solves #1402 and Solves #1453

With this change there is just one query done to retreive all the important data, and matching is done in-code/memory.
With a very large database the sync time went down about 3 times.

Also updated misc crates and Github Actions versions.

@BlackDex BlackDex force-pushed the sql-optimizations branch 4 times, most recently from a184bbe to 1fa2ed1 Compare April 24, 2022 17:27
@manofthepeace
Copy link
Contributor

Just wondering if this crate could be used https://crates.io/crates/tokio_cron_scheduler instead of a not so maintained version of the current job_scheduler. It is based on job_scheduler, and seem actively maintained for now at least.

@BlackDex
Copy link
Collaborator Author

@manofthepeace I'm not happy with the prost crate it is using currently.
Also i had some strange issues with deadlocks, but i have not tested the new version yet, because of the previous reason.

And, when i did try to integrate that crate, i bumbed into some issues with passing on some variables because of how async works.

Also, we think it isn't that bad to have the scheduled tasks to run in there own thread to not bother the application thread it self with this. As you can see i already forked it my self and updated the dependencies where needed.

I might tend to publish the crate since the original author of the crate doesn't seem to be working on it at all anymore.

@BlackDex
Copy link
Collaborator Author

BlackDex commented Apr 25, 2022

Small update.

I have fixed all the full-sync code to use the the CipherSyncData.
Depending on how many ciphers, collections and organizations a user has access to, with my tests I got a speedup of nearly 3x using this code.

I'm still checking for other optimizations to see if I can add those.

@BlackDex BlackDex changed the title WIP - Improve SQL Improve sync speed and updated dep. versions May 4, 2022
@BlackDex BlackDex marked this pull request as ready for review May 4, 2022 19:20
@BlackDex
Copy link
Collaborator Author

BlackDex commented May 4, 2022

Did some final checking and this looks like merge-ready.
Deleting multiple ciphers can still be optimized, but that currently is very entwined with functions calling other functions which could also call other functions.

There are also some optimizations which could be done with web-sockets, but also that need some more work and probably better for an other PR.

Improved sync speed by resolving the N+1 query issues.
Solves dani-garcia#1402 and Solves dani-garcia#1453

With this change there is just one query done to retreive all the
important data, and matching is done in-code/memory.

With a very large database the sync time went down about 3 times.

Also updated misc crates and Github Actions versions.
@dani-garcia dani-garcia merged commit 7f61dd5 into dani-garcia:main May 11, 2022
@dani-garcia
Copy link
Owner

Thanks for your work @BlackDex!

This was referenced May 11, 2022
@BlackDex BlackDex deleted the sql-optimizations branch May 18, 2022 18:15
@bendem
Copy link

bendem commented Jun 16, 2022

Just wanted to say that this is the greatest thing. Loading the home list went down from 4-8s to <0.5s. Best optimisation since the bread slicer!

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

Successfully merging this pull request may close these issues.

4 participants