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

Additional SQLite connection optimizations #102

Merged
merged 3 commits into from
Nov 21, 2024

Conversation

daun
Copy link
Contributor

@daun daun commented Nov 21, 2024

(Hi 👋 Fantastic piece of software you've created, thanks a lot for open-sourcing it!)

I've been toying a bit with optimizing sqlite connections on production sites and thought these might make sense to include by default. The good thing is that Loupe is usually the only client of its particular database, so it can make a few assumptions about how to optimize the connection. Not sure if you agree with all of these suggestions, but it'd be great to get a discussion started.

  • Add a few optimizations to reduce disk I/O
  • Add a timeout to avoid errors if db is locked by another instance
  • Enable incremental vacuum to auto-reduce database size
  • Tested this on a few sites and I'm seeing about a 10% increase in speed (as measured by processingTimeMs)

@Toflar
Copy link
Contributor

Toflar commented Nov 21, 2024

Hey man, thanks a lot for the props, the PR and taking the time to contribute - love it!
Would you mind targeting this against develop (future version 0.8), please?
Regarding the bench: I've included 2 scripts in the bin folder, can you share your results pre and post PR? 😎

@Toflar
Copy link
Contributor

Toflar commented Nov 21, 2024

FYI: Indexing will take about 3 minutes :) You can adjust the config.php in the bin directory to use the movies_1000.json for quicker local tests before running the big one :)

@daun
Copy link
Contributor Author

daun commented Nov 21, 2024

@Toflar Sure! Will do the benchmarks when I find a minute.

@Toflar
Copy link
Contributor

Toflar commented Nov 21, 2024

I will bench on my machine too and let you know. I've just created another PR that improved indexing for me, would love to have your feedback on that too 😊 #103

@Toflar
Copy link
Contributor

Toflar commented Nov 21, 2024

Oh man, the boost on my system is huge!! #103 brings the time from about 220s to 188s and together with your PR we are now at 137s!

@daun
Copy link
Contributor Author

daun commented Nov 21, 2024

Sounds fantastic! I'm having trouble getting the benchmarks to run, but it looks like the results on your machine are speaking for themselves :)

@Toflar
Copy link
Contributor

Toflar commented Nov 21, 2024

Sure, please just rebase on develop, then I'll happily merge it :-)

@daun
Copy link
Contributor Author

daun commented Nov 21, 2024

On it :)

Signed-off-by: Philipp Daun <post@philippdaun.net>
@daun daun force-pushed the feat/sqlite-optimization branch from e8cd096 to a0ad3ca Compare November 21, 2024 16:22
@daun daun changed the base branch from main to develop November 21, 2024 16:22
@daun
Copy link
Contributor Author

daun commented Nov 21, 2024

Rebased. One last thing — could your try experimenting a bit with the page_size option? I've used a very modest increase from 4KB to 8KB, but I've seen very aggressive recommendations around. Maybe setting it to 16KB increases performance a bit more?

'PRAGMA page_size = 16384;',

@daun
Copy link
Contributor Author

daun commented Nov 21, 2024

This might hurt performance when reading/searching, though, that's why I've been a bit conservative with page size.

@Toflar
Copy link
Contributor

Toflar commented Nov 21, 2024

Does not have any effect for me :)

@Toflar Toflar added this to the 0.8 milestone Nov 21, 2024
@Toflar Toflar merged commit 3bfa6b4 into loupe-php:develop Nov 21, 2024
18 checks passed
@Toflar
Copy link
Contributor

Toflar commented Nov 21, 2024

Thanks a lot Philipp!

@daun daun deleted the feat/sqlite-optimization branch November 22, 2024 22:02
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