Skip to content

sync_local: Re-use prepared statements #79

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

Merged
merged 1 commit into from
May 28, 2025
Merged

Conversation

rkistner
Copy link
Contributor

This keeps prepared statements around for the last table, to avoid preparing a new statement for every row that we insert/delete.

I did not fully confirm, but I expect that the way the query plan is structured that the rows would typically be processed one table at a time, making it efficient to only keep the tables for the last query. It is not an issue if the order is different - we may just lose a little of the performance benefits here.

In my tests on native desktop with an in-memory db, this reduced the overall sync_local duration by around 25% (2786ms -> 2075ms for 500k rows). When taking into account filesystem overhead, the impact will be a little less, but still significant.

@rkistner rkistner requested a review from simolus3 May 27, 2025 08:43
simolus3
simolus3 previously approved these changes May 27, 2025
Copy link
Contributor

@simolus3 simolus3 left a comment

Choose a reason for hiding this comment

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

Looks good to me 👍 I agree that we don't have to worry about enforcing table order here.

@rkistner rkistner force-pushed the reuse-prepared-statements branch from bd2acf0 to 42e3da1 Compare May 28, 2025 14:37
@rkistner rkistner changed the base branch from optimize-sync-local to main May 28, 2025 14:38
@rkistner rkistner dismissed simolus3’s stale review May 28, 2025 14:38

The base branch was changed.

@rkistner rkistner requested a review from simolus3 May 28, 2025 14:41
@simolus3 simolus3 merged commit a21580a into main May 28, 2025
21 checks passed
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.

2 participants