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

SQL cache: enable WAL (Write Ahead Logging) by default #5843

Closed
wants to merge 1 commit into from
Closed

Conversation

BoD
Copy link
Contributor

@BoD BoD commented Apr 22, 2024

WAL has shown performance gains when executing many independent inserts (see #5834).

@BoD BoD requested a review from martinbonnin as a code owner April 22, 2024 15:25
Copy link

netlify bot commented Apr 22, 2024

Deploy Preview for apollo-android-docs canceled.

Name Link
🔨 Latest commit 6acf13f
🔍 Latest deploy log https://app.netlify.com/sites/apollo-android-docs/deploys/6626815675084b00080d86a7

Copy link
Contributor

@martinbonnin martinbonnin left a comment

Choose a reason for hiding this comment

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

No strong opinion here. Without a clear benchmark showing improvement, I'd probably err on the side of leaving things untouched. I'll leave it to you though.

@BoD
Copy link
Contributor Author

BoD commented Apr 24, 2024

Running this particular benchmark a few times, I can see a ~12% improvement when using WAL (same with the incubating version of the same test). Confidence is not ultra high however because I get a 8-15% variation between each test.

@eduardb
Copy link
Contributor

eduardb commented Apr 24, 2024

Running this particular benchmark a few times, I can see a ~12% improvement when using WAL (same with the incubating version of the same test). Confidence is not ultra high however because I get a 8-15% variation between each test.

Sorry for the drive-by, but we are currently focused on app performance improvements, so we are quite invested in these topics. Just curious, what device did you run the benchmark on? Could there have been external factors on the device that introduced the variation?

Btw, there's also this great article from P-Y that could help: https://blog.p-y.wtf/statistically-rigorous-android-macrobenchmarks.

Also, using a rooted device should help, at least according to the documentation: https://developer.android.com/topic/performance/benchmarking/microbenchmark-overview#lock-clocks.

I hope this helps, and I am not being annoying xD

@BoD
Copy link
Contributor Author

BoD commented Apr 24, 2024

Hey @eduardb no worries at all, quite the contrary! Advice on this topic is very much welcome and appreciated. This is also a hot topic for us currently.

Thanks for the article link, a very good one indeed. The spreadsheet template is promising.

My methodology was a lot more naive: ran the test 6 times each (with/without WAL), discarded the lowest and highest values, and looked at the averages.
Benchmarks ran on a (non rooted) Samsung S10. My hunch is that cpu throttling is probably the cause of the variance. Airplane mode to avoid external apps waking up is probably a good advice too.

@BoD
Copy link
Contributor Author

BoD commented May 6, 2024

Well, after running the benchmark (a tweaked version of this one) 40 times, and with the help of the template from PY's article, I see an improvement of only ~1%, which is not really significant (results here).

Closing this one - users can still opt-in for WAL in their app if wanted, via the configure lambda.

@BoD BoD closed this May 6, 2024
@martinbonnin martinbonnin deleted the enable-wal branch July 31, 2024 12:13
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.

3 participants