fix(sql_connect): Refactor to improve caching performance for large result sets#18430
fix(sql_connect): Refactor to improve caching performance for large result sets#18430aashishpatil-g wants to merge 4 commits into
Conversation
Large query caching can block main thread as it tries to normalize the data and write to sqlite. Moving this logic off the main thread to a separate isolate will prevent locking the main thread. Other optimizations also include prepared sql statements and use of a transaction to wrap the full normalization process to consolidate multiple writes.
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. |
|
/gemini review |
aashishpatil-g
changed the title
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces background isolate processing for the Firebase Data Connect cache on non-web platforms to prevent blocking the main thread during large result tree normalization, alongside adding transaction support and resource disposal to cache providers. The review feedback highlights critical issues in the isolate implementation, including potential database locks and corruption due to concurrent message processing, a race condition and resource leak in ReceivePort handling, and unhandled asynchronous exceptions from unreturned completers. Additionally, robust error handling is recommended for SQLite3CacheProvider to prevent LateInitializationError and connection leaks if initialization fails.
| final int directedBy_deleteMany; | ||
| final int movie_deleteMany; | ||
| final int person_deleteMany; |
There was a problem hiding this comment.
what change results in the renaming of the generated file?
There was a problem hiding this comment.
There were some changes done recently to support batch mutations. I suspect this might have caused the code generator to change.
There was a problem hiding this comment.
Took a look at the gql operation definitions.
The example project got regenerated and this correctly matches the gql definitions. Its possible the original names had been manually tweaked in the generated code.
2 participants
Description
Large query caching can block main thread as it tries to normalize the data and write to sqlite. Moving this logic off the main thread to a separate isolate will prevent locking the main thread. Other optimizations
These changes only impact native platforms. Web caching is still done on main isolate as isolates aren't supported on web. Web performance will be a future topic.
Also, added tests for
Related Issues
Fixes - firebase/firebase-tools#10776
Breaking Change
This is NOT a breaking change.