Make quick-sqlite 2x quicker? #30
Conversation
|
The wrapper QuickValue is just a convenient way to encapsulate any type of values and return them to the calling JSI function to be later returned to the JS context. Memory allocation probably takes some time, so you are right, getting rid should decrease memory consumption. However mixing JSI code with the SQL code was not a good abstraction at the time (nor is a good encapsulation strategy) so I preferred good design over raw speed. |
|
Thanks @ospfranco, appreciate the quick reply! I figured that'd be the reasoning but that's positive in a way, I wanted to make sure there wasn't a hard technical reason why it would be necessary. I guess we'll patch this on our end for now as we do need to prioritise perf, but I still think there's value in exploring performance improvements more generally. Will leave this open for comments anyway |
|
I just took another look into your code and I realized you only modified the sync operations. You might be lacking some context here. Indeed it is no problem to create more JSI objects on sync operations and you will gain performance because you are halting the JavaScript context. There you can pass and create jsi::XXX values as the query executes because no other JS is running. The You also posted some memory crashes, which will depend on the device. You tested on some somewhat older devices, but there are a lot of smaller Android devices out there with very little memory and such large datasets will almost always crash on those. Even though I tried to make the fastest possible package, I still do not encourage running such large data sets on mobile devices. In any case, thank you for posting some metrics! I also took a look at the codebase again after many months and really enjoyed spelunking once again :) |
|
This might be a bit off topic, but inspired by not too dissimilar reasons I started to work on new sqlite bindings for JSI with a different API approach. The code can be found here: https://github.com/vhakulinen/sqlite-jsi. I would like to crunch it out to a complete package for react native, since but I don't have the time for it, perhaps the code can be a source of inspiration others. One thing you could try with the values (i.e.
you would have
I just realized this and haven't tried it in Another thing I suspect that might be worth of a closer look is the connection initialization. Currently And a third thing, preserve and reuse the prepared statements. Currently Lastly, thank you @ospfranco for creating this library. I has helped us immensely. |
|
Yeah, Hosted Objects was suggested at some point, I didn't quite see the benefit though, it gives some sort of lazy initialization on access, but if you iterate through the entire result set it should still result in the same runtime performance. I might be wrong though. As for the threading mode, I think there was some issue that forced serialized mode but I don't remember right now. Maybe going through the commit history on the original repo will shed some info. In any case, the current iteration of JSI libraries will all disappear was Hermes static disappears. I'm already looking forward to building a new version of the SQLite library based on it. Before I'm thinking about building a web-worker API that would be required to offload work to different threads. It would be based on message passing, so such huge workloads might not work, but for sync work, it should be orders of magnitude faster. |
|
Once you have thousands of items loaded from the database and displayed in, say, scrolled view the cost of copying everything upfront starts to matter. And the threading issue is likely that the code is currently sharing the same connection object across threads which is a big no-no without mutexes (which the serialized mode adds). @mjmasn one additional thing for the string performance: javascript handles strings in UTF-16, while JSI (from what I've seen) and sqlite uses UTF-8. Converting UTF-8 <-> UTF-16 has some cost. If you're loading a ton of text from sqlite that you're not displaying immediately to the user (or use otherwise), you could try to query the texts as BLOBs, load them to a array buffer and then, once required, turn the array buffer into a string. |
Yeah, you are right. I cannot stop people from squeezing every drop of performance out of the devices.
Yeah, I think that was the problem. I think at some point I tried multiple connections but the code gets messy fast.
JavaScript is retarded. Indeed internally every string is UTF-16 and there are a lot of conversions between UTF-8 and UTF-16 even when it interacts within itself. Actually, you guys inspired me to try to hack at this again and see what can be done to really squeeze the last bit of performance. Instead of using the QuickValue struct one can use std::any, which should use a lot less memory allocation. I also implemented a new version of a SmartHostObject (over a simpler previous implementation, the new version that uses an unordered_map internally which means faster access to fields) and this is what I got. I created a sample database with 300k records. A mixture of strings, ints, and floats. Then queried all of them at once. Using current implementation: 
New implementation: 
As stated the cost is shifted when accessing the lazy object fields. But if you only display a few of them at a time, should be absolutely negible. |
|
Created a new package. Mostly API compatible, you will need to relocate your DB or specify a path. But it is quite faster when loading large sets of data. |
|
Just catching up on this thread, could not have hoped for a better set of replies ❤️ In my initial tests I'm seeing a 3x perf boost on SELECTs using your new package @ospfranco, really appreciate you taking the time to look into this. |
|
cheers! access are a bit slower, so if you iterate over all of the data it will probably be slower, but I guess it is better when used in flatlists and what not. |
|
I just published a new version. I've now gotten Android to under a second and iOS to ~500ms to load the 300k records. @mjmasn give it a try. |
|
Hey @ospfranco, one thing I'm noticing is if I do a few large fetches in a row, the app is crashing due to OOM. It happens much less so with react-native-quick-sqlite. Not sure how to go about debugging that. For a 50k record fetch, 30 columns per row, memory use was increasing like 500MB each time on both iOS and Android yesterday. With your latest version it's more like 200-300MB. Interestingly this is in a new RN app containing both react-native-quick-sqlite and @op-engineering/op-sqlite - if I run a bunch of SELECTs on op-sqlite, memory increases, leave the app for a while, no change. As soon as a run the same query on react-native-quick-sqlite the memory does drop over time after that. Each package is using a different SQLite database file, containing identical data. I had to make some minor changes to react-native-quick-sqlite just to rename the One other thing, with the latest version of op-sqlite it seems running PRAGMA commands crashes the app. ADB crash logIf it'd be helpful for me to share my comparison app let me know, more than happy to get it into a sharable state at some point this week 👍🏻 |
|
Please open a ticket in the new repo and try the latest version 1.0.3 |
DISCLAIMER 1: This is not a PR intended to be merged, use at your own risk
DISCLAIMER 2: I don't really know anything about C++
Intro
This PR is intended to spark a discussion about improving the performance of this package. Sure it's like 5-8x faster than non-JSI packages but it's still not perfect. For background, we deal with some fairly large (for mobile anyway) data sets with up to 300000 records per table, maybe with 15-30 text/number/boolean columns on average. Selecting all these rows might take 20-30 seconds or more depending on the device. Obviously selecting 30 columns across 300k records at once is a worst case scenario but if we can improve the worst case performance, we'll probably be improving the best cases too, helping our apps feel a lot more responsive.
Potential Performance Bottlenecks
I've identified two major bottlenecks but there could be others.
QuickValue
Digging into the package code, I can see that there is an intermediate
QuickValuestate created for every bit of data passing from JSI to SQLite and back again. It feels like this isn't necessary, and indeed for query results I have been able to make it work without. I know @ospfranco has handed this package over but maybe he can shed some light onto the original reasons for this?JSI Objects
That gets us a bit more perf, but converting the SQLite results to JSI objects is still pretty slow. I had an inkling that using arrays would be more efficient here. Adding this feature in gets us a full 2x performance increase vs the current intermediate values + JSI objects.
Other???
I think the biggest issue is likely to be that creating JSI strings is slow, but that may be outside the scope of this package to solve. But maybe there are other issues that a more knowledgable eye could spot!
The PR
This PR is what I've essentially hacked together so far to improve the two main issues. I've duplicated the
executeandexecuteAsyncmethods asexecute2andexecuteAsync2to allow performance to be compared.Both of these new methods can take an additional
returnArraysboolean parameter to determine whether to return results as (false) an array of objects, and (true) an array of arrays.What I mean by this is:
vs
All seems well but it's probably quite likely I've done bad things given I hadn't touched C++ at all before looking into this.
I guess my hope is that this can get some eyes and feedback and hopefully form a basis for an actual PR into the project that will give users of the package a nice performance boost.
Benchmarks
Based on setting up the benchmark by inserting 200000 rows into the database, each with 30 text columns containing random 64-character strings, then running each test 3 times to ensure the results were accurate.
These specific tests were performed on a Google Pixel 6 Pro with Android 14 but I've seen similar performance gains on iOS simulator, iPhone 7 Plus, iPad 2019, Samsung Galaxy Tab A 2019.
Selecting 5000 records
Selecting 50000 records
Selecting 200000 records