1189170 - Explicitly scope transactions

Status: NEW

(Firefox for iOS :: Data Storage, defect)

Description

[Richard Newman [:rnewman]]

Basically everywhere we do a write to BrowserDB we end up creating a transaction -- we call db.run, it calls db.transaction with a block, and SwiftData calls BEGIN EXCLUSIVE on our behalf.

This is terrible for performance, both in the app (go grepping for "TODO: transactions!") and in test running itself. The setup work for our frecency perf test takes about 20 seconds in the simulator.

We should do better than this. The key problem is the same as in the use of synchronization or concurrency constructs: re-entrancy, scope, nesting. We also use Deferred, so we can't just throw blocks at this -- there will always be some manual work.

There are a few approaches that overlap:

* Hope that SQlite.swift somehow does better here, and we fix our code as we port. Bug 1189063.

* Use dispatch_queue_set_specific to flag that we're in a transaction, and avoid creating a new one for the current connection. We'd have to be very careful about lifecycles.

* Explicitly thread "I already have a transaction open" flags, or a `run` function, as arguments to methods. This is painful but simple.

* Stop automatically establishing transactions inside SwiftData. Do it at a much higher level, and rely on sqlite to implicitly establish transactions if we need them.
*

Comment 1

[Richard Newman [:rnewman]]

A note, just in case you're tempted: don't vacuum within a transaction. Y'can't.

Comment 2

[Richard Newman [:rnewman]]

I did a little hacking on this.

Things that will be potential trip-ups:

Transactions should be async, giving us all of our nice Deferred chaining. They can encapsulate the creation, rollback, and committing of transactions.

But all of these DB operations need to occur on a SQLiteDBConnection.

Those are vended by SwiftData, and because we currently run all operations inside a withConnection block, we don't have to worry about references to those connections running away, or work being queued up on a connection that is subsequently closed.

Additionally, we need to run the work itself on the connection's queue (through which we serialize operations).

The queue is private, and again, work might reach the head of the queue after the connection is closed. Async work needs to check if the connection has been closed each time it runs!


It's worth considering that SQLite transactions are scoped to the connection, not to the caller.

So if work is interleaved on the connection's queue -- which seems bound to happen, given that callers get free access to call withConnection at any point, and seems to part of the value of doing all this! -- arbitrary work can happen within an open transaction. Not only does that confuse matters for the code that owns the transaction, but it also could lead to the work done by the intruder being rolled back unexpectedly.

Worst-case we crash by attempting to vacuum while a transaction is open.


A proper treatment of this hides much more of the guts of SwiftData. We could make transactions truly exclusive (so one would own the connection for the entire duration of a transaction). We could open another connection, issuing them from a pool and getting the scoping we need that way.

Whichever approach we take, it seems likely that we can't safely coexist with SwiftData as it currently is used. That means a mini-rewrite.