sqlx performance and stability issues

[r10s]

although reported differently in between, also by me:

since we decided to pull in sqlx (#2089), we still encounter issues with sqlx:

1. crashes in the step-function crash in core, sqlite3_step(), probably related to sqlx deltachat/deltachat-ios#1129, have been mitigated by enabling the "statement cache" in Update sqlx to enable statement cache #2331

2. performance issues where sqlx is 3x slower were first obviously visible on android's global search (performance degradation #2353) and were worked around by enabling sqlite's "shared cache" (sql: enable shared cache #2356, SQLite step serialization overhead launchbadge/sqlx#1175)

3. the "shared cache" somehow moved the crash from the background to the time the app was started, so it became more visible (last comment in crash in core, sqlite3_step(), probably related to sqlx deltachat/deltachat-ios#1129)

4. also, on ios, the app feels less snappy since "shared cache" enabled. on start from backgroud, there is half a second anr, although things are dispatched to bg-thread, so there is some new blocking. also on loading messages in bg on startup, this blocking can be noticed.

5. there are more performance issues popping up, not sure if they are related to "shared cache" or not, eg. on android motog4: getChatMsgs: 560ms on rusqlite vs. 770ms sqlx - which is still 1.4x slower (although sqlx uses 2 additional caches that rusqlite does not use!)
   weird thing is that some other queries do not make much difference, eg. getChatlist (but even there, rusqlite is always a bit faster). i did not measure all calls recently, but overall, the app feels generally slower.
   EDIT: more scientific benchmarks were done by @link2xt at sql: set PRAGMA synchronous=normal #2382

6. EDIT: much higher ram-usage, see below

i personally cannot explain this interaction of async/blocking/threads/etc. i can only see the result, however, @link2xt and @dignifiedquire have some good ideas and insights and can explain things better.

all performance issues are visible on a not-super-new-phone with a larger database, so, eg. on iphone7 or motog4 - which, otoh are not super-old and are in wide use.

rusqlite does not have any of these issues.

the initial idea of pulling in sqlx was to improve performance by less blocking and more caching. at a first glance, that seems to work, however, at the end, the recent performance issues were all ui related - and blocking seems to be worse on sqlx. other advantages of sqlx are nicer bindings, support of other sql-engines, possible compile-time-sql-checks, type-safety, however, as we do not really have issues with these parts, this is a weak point.

also, it is questionable if there are not even more issues, it feels a bit like fighting against the hydra 🐍

good thing is that we did not release yet and also we improved several sql-statements while fiddling around with sqlx.

so - question is, what to do on that. in the last weeks, we put much effort in trying to get a grip on that - but, tbh, meanwhile, i am pretty sceptical to use sqlx in this state and i fear that it eats too many resources in the future.

[link2xt]

Simple fix for sqlx segfaults, possibly at the cost of performance, is to wrap statement (which is a StatementHandle, a wrapper for SQLite statement pointer) into Arc::new() here and everywhere where it is used:
https://github.com/launchbadge/sqlx/blob/5a8418e5fd68e185c9d90099def813b1621f16cd/sqlx-core/src/sqlite/statement/virtual.rs#L162
All other structures in the lines below are wrapped, but here sqlx authors decided it was safe to make C pointer wrapper Copy and not reference-count it, which was a mistake obviously now that we get segfaults.
sqlite3_finalize call should then be moved into Drop implementation of StatementHandle. This way all the uses of statement will be reference-counted and it will never be finalized before the last reference to it is removed.

[r10s]

thanks, running ios with the fix now. EDIT: there were still crashes using sqlx, see deltachat/deltachat-ios#1129 (comment)

i took the chance to measure the things from above also in iphone7 (each time is the average of ten runs):
getChatMsgs: 149ms on rusqlite vs. 361ms sqlx
getChatlist when coming back from chat: 264ms rusqlite vs 320ms
getChatlist on opening app is pretty much the same on both, for whatever reason: 180ms

[r10s]

nb, ftr: sqlx also uses significantly more ram, probably related to the two additional caches needed to be enabled to improve performance; however, esp. on lower-ended phones that can become an issue on its own.

this was reported by @Hocuri in the testing channel, who measured the memory after 3 hours, left sqlx and right rusqlite:

[link2xt]

probably related to the two additional caches that needed to be enabled to improve performance

Even worse, it segfaults without caches.

[Hocuri]

Seems like the high memory consumption was not because of sqlx, it happened again after the update: deltachat/deltachat-android#1917

[madadam]

Possibly related: launchbadge/sqlx#1300