Performance drops on 4.6.1

[jcrabanal]

Hello, this is related to #27, but I'm adding new info. It seems like simply opening the database is enough to trigger the JNI warning. Using the byte array password constructor or disabling WAL did not help.

JNI critical lock held for 451.804ms on Thread[43,tid=12140,Runnable,Thread*=0x7b64527350,peer=0x13a83b18,"LoadMappings"]

SQLiteDatabase.openDatabase(sPath,
    sPassword,
    null,
    nFlags,
    dbCorrupted -> errorHandler.onCorruption(new Database(dbCorrupted)),
    null);

[developernotes]

Hi @jcrabanal,

Unfortunately, the timing from the JNI log isn't quite enough to determine an overall issue. Both the hardware you run the operation on, and whether your are using the Community vs. Commercial edition of SQLCipher for Android will have an impact on the time required to derive an encryption key. Additionally, it would be beneficial to run this same test using the older android-database-sqlcipher library on the same hardware for comparison purposes. There are options to adjust key derivation to improve the performance, however, you should be aware that doing so may impact the overall level of security provided by the library.

[jcrabanal]

Hello, I've run my tests on a Pixel 7, a fairly decent device. I've tried android-database-sqlcipher and sqlcipher-android back and forth several times (changing a few things because of the API changes). The difference is pretty big.

It definitely seems related to opening/closing the databases. I am going to try providing raw keys, to see if it makes a difference.

[developernotes]

Hi @jcrabanal,

The difference is pretty big.

What was the actual difference? Was the test using the same database?

It definitely seems related to opening/closing the databases. I am going to try providing raw keys, to see if it makes a difference.

Please make sure you secure the raw key material if you choose to continue with this approach.

[jcrabanal]

Same database. I have run tests with databases created on 4.6.1 and 4.5.4 and tested each one on both versions. Version 4.6.1 was always slower, with naked eye noticeable hiccups.

[jcrabanal]

Hello @developernotes, key derivation doesn't seem to be causing the slowdown. Something else must be going on, unless I'm doing it wrong.

I've converted plaintext databases using this test key:

SQLiteDatabase db = SQLiteDatabase.openOrCreateDatabase(fDb,
        CryptoUtils.EMPTY_STRING,
        null,
        (dbCorrupted) -> CryptoUtils.DebugLog("Corruption detected in database " + fDb.getAbsolutePath() + " while trying to encrypt it"),
        null);
int nVersion = db.getVersion();
db.rawExecSQL("ATTACH DATABASE '" + fDbEncrypted.getAbsolutePath() + "' AS encrypted KEY \"x'98483C6EB40B6C31A448C22A66DED3B5E5E8D5119CAC8327B655C8B5C483648101010101010101010101010101010101\"");
db.rawExecSQL("SELECT sqlcipher_export('encrypted')");
db.rawExecSQL("DETACH DATABASE encrypted");
CryptoUtils.closeSafely(db);

Then, to use them:

SQLiteDatabase db = SQLiteDatabase.openDatabase(sPath,
    "x'98483C6EB40B6C31A448C22A66DED3B5E5E8D5119CAC8327B655C8B5C483648101010101010101010101010101010101",
    null,
    nFlags,
    dbCorrupted -> errorHandler.onCorruption(new Database(dbCorrupted)),
    null);

[sjlombardo]

@jcrabanal - if that code is copy pasted from your app, you are not bypassing key derivation. Both of the raw key formats you are using are missing the closing single quote (') at the end of the hex string, i.e. they should look like

"x'98483C6EB40B6C31A448C22A66DED3B5E5E8D5119CAC8327B655C8B5C483648101010101010101010101010101010101'"

not

"x'98483C6EB40B6C31A448C22A66DED3B5E5E8D5119CAC8327B655C8B5C483648101010101010101010101010101010101"

[jcrabanal]

Aha, that's it. No slowdowns with the raw key, now I can move on. Thank you!

However, if key derivation was the problem, shouldn't the old version also be slower? I've run tests on 4.5.4 with databases created on 4.6.1 and those had no issue.

[sjlombardo]

@jcrabanal There isn't enough information to tell why based on your descriptions so far. I strongly suspect that multiple connections are being opened with sqlcipher-android, and each one is incurring the cost of key derivation (the old library didn't provide connection pooling).

If you would like us to investigate further, you need to put together a small standalone reference application that clearly demonstrates the problem, e.g. with one branch using android-database-sqlcipher, the another branch using sqlcipher-android. It should show a measured difference in performance between the two libraries under a well-defined use case. Then publish it to github and send us the link. We could then review and troubleshoot the behavior you are seeing.

[MohamadJaara]

Hi over at wire- android we also noticed the same thing, especially for complex queries, for the same DB reverting back to 4.5.6 fixed it
the query where we noticed most delay is https://github.com/wireapp/kalium/blob/release/candidate/persistence/src/commonMain/db_user/com/wire/kalium/persistence/Messages.sq#L501 especially when having a lot of messages (+500k) so my bet is the planner is getting confused and is causing a full table scan, i will keep investigating from my side but please let me know if i need to provide more info
@sjlombardo

[sjlombardo]

@MohamadJaara I am closing this issue now for inactivity. I would suggest opening a new issue about your problem if you are able to cleanly reproduce it. It is almost certainly unrelated to the issue initially reported here.