Skip to content

Commit 60490f4

Browse files
committed
doc(sqlite): Add documentation to connection.
This patch explains why we create our own implementation of `deadpool` for `rusqlite`.
1 parent 6a828e3 commit 60490f4

File tree

1 file changed

+50
-0
lines changed

1 file changed

+50
-0
lines changed

crates/matrix-sdk-sqlite/src/connection.rs

Lines changed: 50 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,56 @@
1313
// limitations under the License.
1414

1515
//! An implementation of `deadpool` for `rusqlite`.
16+
//!
17+
//! Initially, we were using `deadpool-sqlite`, that is also using `rusqlite` as
18+
//! the SQLite interface. However, in the implementation of
19+
//! [`deadpool::managed::Manager`], when recycling an object (i.e. an SQLite
20+
//! connection), [a SQL query is run to detect whether the connection is still
21+
//! alive][connection-test]. It creates performance issues:
22+
//!
23+
//! 1. It runs a prepared SQL query, which has a non-negligle cost. Imagine each
24+
//! connection is used to run on average one query; when recycled, a second
25+
//! query was constantly run. Even if it's a simple query, it requires to
26+
//! prepare a statement, to run and to query it.
27+
//! 2. The SQL query was run in a blocking task. Indeed,
28+
//! `deadpool_runtime::spawn_blocking` is used (via
29+
//! `deadpool_sync::SyncWrapper::interact`), which includes [blocking the
30+
//! thread, acquiring a lock][spawn_blocking] etc. All this has more
31+
//! performance cost.
32+
//!
33+
//! Measures have shown it is a performance bottleneck for us, especially on
34+
//! Android. Why specifically on Android and not other systems? This is still
35+
//! unclear at the time of writing (2025-11-11), despites having spent several
36+
//! days digging and trying to find an answer to this question.
37+
//!
38+
//! We have tried to use another approach to test the aliveness of the
39+
//! connections without running queries. It has involved patching `rusqlite` to
40+
//! add more bindings to SQLite, and patching `deadpool` itself, but without any
41+
//! successful results.
42+
//!
43+
//! Finally, we have started questioning the reason of this test: why testing
44+
//! whether the connection was still alive? After all, there is no reason a
45+
//! connection should die in our case:
46+
//!
47+
//! - all connections are local,
48+
//! - all interactions are behind [WAL], which is local only,
49+
//! - even if for an unknown reason the connection died, using it next time
50+
//! would create an error… exactly what would happen when recycling the
51+
//! connection.
52+
//!
53+
//! Consequently, we have created a new implementation of `deadpool` for
54+
//! `rusqlite` that doesn't test the aliveness of the connections when recycled.
55+
//! We assume they are all alive.
56+
//!
57+
//! This implementation is, at the time of writing (2025-11-11):
58+
//!
59+
//! - 3.5 times faster on Android than `deadpool-sqlite`, removing the lock and
60+
//! thread contention entirely,
61+
//! - 2 times faster on iOS.
62+
//!
63+
//! [connection-test]: https://github.com/deadpool-rs/deadpool/blob/d6f7d58756f0cc7bdd1f3d54d820c1332d67e4d5/crates/deadpool-sqlite/src/lib.rs#L80-L100
64+
//! [spawn_blocking]: https://github.com/deadpool-rs/deadpool/blob/d6f7d58756f0cc7bdd1f3d54d820c1332d67e4d5/crates/deadpool-sync/src/lib.rs#L113-L131
65+
//! [WAL]: https://www.sqlite.org/wal.html
1666
1767
use std::{convert::Infallible, path::PathBuf};
1868

0 commit comments

Comments
 (0)