5 ms·
This is the smoking gun, unfortunately; I was right. This code is not correct. You're doing a single insert per transaction (which is the same thing you'd get i
by electroly 3y ago
This is the smoking gun, unfortunately; I was right. This code is not correct. You're doing a single insert per transaction (which is the same thing you'd get if you hadn't used an explicit transaction at all). What you want is a single transaction with ALL of the inserts in it. Do it again with a single transaction and you'll find it massively improves the performance. This is a common pitfall for beginners.
Edit: You've changed the link after I wrote this reply. Did you fix the code? Is the blog post using the original code or this updated code?
- vishnumohandas 3y agoMy bad, I linked to the wrong portion of the code. Here's the updated link[1]. We are executing the entire write in a single batch query. From what we observed, and have documented, it is the serialization that is causing the bottleneck, not the write itself. Also, more than writes, it is the latency associated with deserializing the read data into models that pushed us to look at options outside SQLite. [1]: https://github.com/ente-io/edge-db-benchmarks/blob/43273607d7c55982693d498f10df184a06ed5930/lib/databases/sqlite_db.dart#L65-L66 https://github.com/ente-io/edge-db-benchmarks/blob/43273607d...
- electroly 3y agoWhy are you comparing DB engines if the bottleneck is the serialization? You're the one doing the serialization! You could store the embedding directly into SQLite columns if you wanted.
- vishnumohandas 3y agoI agree that deserialization is not the responsibility of the database. But as an end consumer, the overall throughput while using SQLite as the DB engine was not sufficient to serve our use case of reading 100k embeddings.
- hinkley 3y agoWhat people are saying is your premise is wrong, and you’re getting defensive. Either of those would not be an interesting problem except that your title is calling out a particular and beloved database as being the culprit. Which feels a lot like defamation. Even if deserialization is the bottleneck, bottlenecks are relative and don’t have to be singular. You can have multiple bottlenecks and being blind to one because of PEBCAK can still skew results and thus conclusions. The people you’re responding to want you to fix your code and run it again. I think that’s reasonable. And maybe deserves a follow up post or a rework of the existing one.
- vishnumohandas 3y agoSorry if I sounded defensive! I have nothing against SQLite, but I unfortunately haven't yet understood how to write and read Lists more efficiently. Once I do, I'll definitely post a follow up :)
- vishnumohandas 3y agoUpdate: https://ente.io/blog/tech/sqlite-objectbox-isar#update https://ente.io/blog/tech/sqlite-objectbox-isar#update TL;DR: Removing Protobuf and directly serializing the list to a blob helped!
- lqet 3y agoIf the serialization is really the bottleneck, as you say, then you aren't serializing the data in the Isar evaluation. Which makes the comparison unfair. Your comment reads a bit like saying you don't like a Toyota car for transporting apples, because it takes too long to bake a pie out of them first. BTW, if Isar's data format really is "very close to in-memory representation", then why aren't you simply packing the 512 floats into an array (which I guess is how this data is represented anyhow), and use this continuous block of memory directly as the serialized value?
- vishnumohandas 3y agoHey, since SQLite does not yet support lists, if there's a better way to serialize so that reads can be sped up, do let me know, it would help!
- TedDoesntTalk 3y ago> deserializing the read data into models What does this have to do with SQLite?
- jjk7 3y agoAre batch operations automatically atomic in SQLite? SQLite makes no mention of batch operations. Looking at the library, it seems it's just using prepared statements outside of a transaction... https://github.com/powersync-ja/sqlite_async.dart/blob/f994e165d5410c9ccca9a187de9c51fcedfd1182/lib/src/sqlite_connection_impl.dart#L239 https://github.com/powersync-ja/sqlite_async.dart/blob/f994e... Seems the other commenter is correct that wrapping that line in a tx will fix it.
- hinkley 3y agoWithin the DB world, “batch” usually means a sequence of queries in a single transaction. In software a batch is clumping tasks to run together with any sort of separation from other clumps. Logical, time… If you use one definition while everyone else is using the other, there’s going to be a loud and confused argument. Since we are talking about databases, we’d all better be using the former, but I suspect maybe the author is not.
- epcoa 3y agohttps://github.com/powersync-ja/sqlite_async.dart/blob/f994e165d5410c9ccca9a187de9c51fcedfd1182/lib/src/sqlite_queries.dart#L138 https://github.com/powersync-ja/sqlite_async.dart/blob/f994e... https://github.com/powersync-ja/sqlite_async.dart/blob/f994e165d5410c9ccca9a187de9c51fcedfd1182/lib/src/sqlite_database.dart#L20 https://github.com/powersync-ja/sqlite_async.dart/blob/f994e... I think you may be looking too deeply. It does look like that object will wrap in a tx.
- jjk7 3y agoAh, yeah I think you're right.
- slingnow 3y agoYou're asking too many questions. Didn't you read the title? SQLite isn't enough! /s
- cooljacob204 3y agoJust because you could increase the performance by putting everything into a transaction doesn't make it right to benchmark with. Most people won't be inserting large amounts in a single transaction. Ex webserver with many concurrent clients inserting.