Skip to content

Performance issues after updating to sqlcipher-android from android-database-sqlcipher #27

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
eygraber opened this issue Nov 17, 2023 · 42 comments

Comments

@eygraber
Copy link
Contributor

I updated my app from android-database-sqlcipher 4.5.4 to sqlcipher-android 4.5.5 (I also tried sqlcipher-android 4.5.4 and have the same issue).

When my app starts I make a few network calls and store the responses in the db. After the update to sqlcipher-android the startup performance of my app is absolutely trashed. It takes multiple seconds for my UI to render. I see the following in the logs:

19:16:43.028  I  Database keying operation returned:0
19:16:43.199  W  JNI critical lock held for 167.642ms on Thread[21,tid=30010,Runnable,Thread*=0xb400007b6848e460,peer=0x172c4980,"DefaultDispatcher-worker-3"]
19:16:43.200  I  Database keying operation returned:0
19:16:43.364  W  JNI critical lock held for 162.610ms on Thread[53,tid=30088,Runnable,Thread*=0xb400007b684f15b0,peer=0x13b40738,"DefaultDispatcher-worker-8"]
19:16:43.365  I  Database keying operation returned:0
19:16:43.559  W  JNI critical lock held for 192.855ms on Thread[51,tid=30086,Runnable,Thread*=0xb400007b684ede10,peer=0x130404a8,"DefaultDispatcher-worker-6"]
19:16:43.560  I  Database keying operation returned:0
19:16:43.803  W  JNI critical lock held for 239.118ms on Thread[54,tid=30089,Runnable,Thread*=0xb400007b684fbc90,peer=0x130c1d68,"DefaultDispatcher-worker-9"]
19:16:43.804  I  Database keying operation returned:0
19:16:44.009  W  JNI critical lock held for 203.410ms on Thread[20,tid=30009,Runnable,Thread*=0xb400007b68490030,peer=0x172c44d0,"DefaultDispatcher-worker-2"]
19:16:44.011  I  Database keying operation returned:0
19:16:44.214  W  JNI critical lock held for 202.158ms on Thread[56,tid=30091,Runnable,Thread*=0xb400007b684f6920,peer=0x13c00000,"DefaultDispatcher-worker-11"]
19:16:44.216  I  Database keying operation returned:0
19:16:44.389  W  JNI critical lock held for 171.822ms on Thread[55,tid=30090,Runnable,Thread*=0xb400007b684f84f0,peer=0x13aa7828,"DefaultDispatcher-worker-10"]
+ more after that

This only seems to happen on a cold app open, and happens on both debug and release builds.

I'm trying to profile the build to see what's going on, but Android Studio isn't cooperating at the moment.

@developernotes
Copy link
Member

Hi @eygraber,

It appears your application is opening many connections to the same, or different databases. The log statement for keying operation returning 0 is from here. The query that follows triggers key derivation based on your provided password which is slow by design.

Do you have a large number of database files you maintain connections to? If you only have a single database, it would be good to review why the application appears to open the connection repeatedly.

@eygraber
Copy link
Contributor Author

I only have one database file and access to it is managed by a singleton that opens the connection when it is instantiated. There is a 3rd party layer in between that I'm investigating.

Is the design of sqlcipher-android different than android-database-sqlcipher in this regard? Because there's no performance issue with the same code using android-database-sqlcipher.

@developernotes
Copy link
Member

Hi @eygraber,

Yes, sqlcipher-android is different in that it has improved support for concurrent operations when used with WAL mode, this is a big improvement over the android-database-sqlcipher behavior.

sqlcipher-android will use connection pooling when the database is configured to use WAL mode. It can create up to 10 connections that it will provide on-demand. Each SQL operation is processed via a thread local session. Are you executing different SQL operations from different threads at the time the log statements are generated?

@eygraber
Copy link
Contributor Author

That sounds like it may be it. I'm using WAL and when the app starts it makes a bunch of parallel API calls and writes their responses to the db.

Is there any way to configure the pooling so I can test if that's the issue?

@developernotes
Copy link
Member

Hi @eygraber,

The connection pool isn't configurable currently. Can you try disabling WAL mode to see if your performance behavior changes?

@eygraber
Copy link
Contributor Author

Yes disabling WAL resolves the performance issues. I still see Database keying operation returned:0 logged twice though (instead of the 7-10 times it gets logged with WAL enabled). Is that expected?

@developernotes
Copy link
Member

Hi @eygraber,

Do you have a small example where that is occurring with a single connection and WAL mode disabled? The connection pool does distinguish between a primary and non-primary connection based on the connection flags.

@eygraber
Copy link
Contributor Author

I'm sorry I don't. This is a pretty large app that I can't share. I'll see if I can get it to happen in a repro project.

@amc-apatel
Copy link

Hi @developernotes,

I'm also facing this issue after migrating to sqlcipher. Any suggestions to resolve the issue unblock us.

Please check this android example where I've implementated sqlcipher and observing JNI issue.

MyApplication3.zip

Logs from this sample:

2024-04-17 17:51:33.993 9710-9710 SQLiteConnection com.ex.myapplication I Database keying operation returned:0
2024-04-17 17:51:34.283 9710-9710 x.myapplication com.ex.myapplication W JNI critical lock held for 288.246ms on Thread[1,tid=9710,Runnable,Thread*=0x75c1a5f800,peer=0x73217120,"main"]
2024-04-17 17:51:35.323 9710-9710 ViewRootIm...nActivity] com.ex.myapplication I ViewPostIme pointer 0
2024-04-17 17:51:35.402 9710-9710 ViewRootIm...nActivity] com.ex.myapplication I ViewPostIme pointer 1
2024-04-17 17:51:35.426 9710-9710 SQLiteConnection com.ex.myapplication I Database keying operation returned:0
2024-04-17 17:51:35.737 9710-9710 x.myapplication com.ex.myapplication W JNI critical lock held for 311.096ms on Thread[1,tid=9710,Runnable,Thread*=0x75c1a5f800,peer=0x73217120,"main"]
2024-04-17 17:51:35.740 9710-9710 Patient details com.ex.myapplication D Id, name1713356493990Ap

@EpariNigam
Copy link

EpariNigam commented Sep 23, 2024

Hey @developernotes after migrating to sqlcipher-android I can clearly see that, it is taking much time to return result.
Same Repo : https://github.com/EpariNigam/SQLCipher

Steps -

  • Checkout main branch
  • Click insert string
  • Kill the app
  • Click fetch string and see the time.
  • Kill the app
  • Now checkout sql_cipher_migration branch
  • Click fetch string and see the time. It is almost 2-3 times slower.

@developernotes
Copy link
Member

Hi @EpariNigam,

I believe what you are seeing can be attributed to key derivation within your sample. If you adjust your onCreate method to perform a read from sqlite_master (which will trigger key derivation) you will see the query time to fetch users is much better:

override fun onCreate(savedInstanceState: Bundle?) {
        super.onCreate(savedInstanceState)
        tvStatus = findViewById(R.id.tv_status)
        tvData = findViewById(R.id.tv_data)
        db = EncryptedAppDatabase.getInstance(this)
        userDao = db.userDao()
        stringDao = db.stringDao()
        val cursor = db.query("select count(*) from sqlite_master;", null)
        if(cursor.moveToFirst()){
            val rows = cursor.getInt(0);
            Log.i(javaClass.simpleName, String.format("rows:%d", rows))
            cursor.close();
        }
        tvData.movementMethod = ScrollingMovementMethod()
    }

@EpariNigam
Copy link

Hey @developernotes Is there no other solution, other than this? Is it like on new library the key derivation is slower compared to old library or was there no key derivation technique used in old library?

@sjlombardo
Copy link
Member

@EpariNigam The number of key derivation rounds was increased in SQLCipher 4 by a factor of 4x. This was to increase security against brute force and dictionary attacks. You have a couple options:

  1. Use legacy SQLCipher 3 settings, as described by option 2 here. This has security implications as you wouldn't be using the latest settings, but would yield performance similar to SQLCipher 3 on the connection's first use.
  2. If your application is using random key material, disable key derivation using a Raw Key. Note that while this would eliminate the time spent with key derivation, it has major security implications. If you aren't using random key material this would be extremely insecure.

Regardless, before doing either of those, you should review the general performance optimization guidelines for your application. If you are adhering to the guidelines and using long running connections the impact of key derivation should be very minimal, since it only happens once per connection.

@sjlombardo
Copy link
Member

@EpariNigam disregard my earlier comment. It was directed more at general performance of key derivation, which I now recognize is different than the issue you're facing. We can take a look at this a bit closer, however, I cannot provide a time frame for an update at the moment.

@sjlombardo sjlombardo reopened this Sep 25, 2024
@EpariNigam
Copy link

EpariNigam commented Sep 26, 2024

Hey @sjlombardo,

Thanks for your reply. Will look forward on further insights on this.

@EpariNigam
Copy link

Hey @developernotes @sjlombardo,

Any update on this?

@EpariNigam
Copy link

Hey @sjlombardo @developernotes any update on this?

@EpariNigam
Copy link

Hey @sjlombardo @developernotes any update on this, still waiting to migrate our app to latest version. It is blocking us for android 15 compatible as well.

@sjlombardo
Copy link
Member

Hello @EpariNigam we're still investigating this. We'll provide an update once we have additional information.

@sjlombardo
Copy link
Member

Hello @EpariNigam - after reviewing your reference project, we have determined that the library is working as expected. When using the default configuration with WAL mode, at least 2 connections will be established in the connection pool. This involves performing key derivation more than once, which increases the time required to execute the first connection. On the other hand, this can substantially increase performance for concurrent database access.

If you want to disable pooling, you can turn off WAL mode, which should make first-execution performance about equivalent to the previous library. You can do this by calling .setJournalMode(JournalMode.TRUNCATE) on the databaseBuilder, i.e.

            return Room.databaseBuilder(context, EncryptedAppDatabase::class.java, DB_NAME)
                .openHelperFactory(factory)
                .setQueryExecutor(Dispatchers.IO.asExecutor())
                .setTransactionExecutor(Dispatchers.IO.asExecutor())
                .setJournalMode(JournalMode.TRUNCATE)
                .build()

That will restrict the library to using a single connection. The other options would be to adjust key derivation so it takes less time, or perhaps try out the official Commercial Edition packages which have an optimized cryptographic library and may offer better performance.

@eygraber
Copy link
Contributor Author

The connection pool isn't configurable currently - #27 (comment)

@sjlombardo would it be possible to make the pool configurable to help address this? Start up time is a critical metric for Android apps, and this is a real killer.

@sjlombardo
Copy link
Member

@eygraber It is something we could look into it. That said, what would you envision configuring on the pool, just the max connections? It's worth noting that adjusting the max connections on the pool would not impact the startup time for the example just posted.

@eygraber
Copy link
Contributor Author

I imagine it would help for my original issue though, right?

@EpariNigam
Copy link

Right @sjlombardo it might not help. But, giving an option to set pool count would be helpful.

@jcrabanal
Copy link

I'm facing the same issue. I'm migrating from the old library, net.zetetic:android-database-sqlcipher:4.5.4. In my case, the performance drop is abysmal when using net.zetetic:sqlcipher-android:4.6.1.

I'm using a SQLiteOpenHelper subclass. Disabling WAL mode in the constructor did not help.

My class has methods that use this format (all of them):

@CheckResult
@Nullable
public static synchronized String getNext() {
    SQLiteDatabase database = instance.getReadableDatabase();
    Cursor cursor = null;
    try {
        cursor = database.query(TABLE_NAME_MESSAGES,
                null,
                null,
                null,
                null,
                null,
                null);
        boolean bMoved = cursor.moveToFirst();
        if (!bMoved) {
            return null;
        }
        int nIndexText = cursor.getColumnIndex(COLUMN_NAME_TEXT);
        return cursor.getString(nIndexText);
    } finally {
        if (cursor != null) {
            cursor.close();
        }
        if (database != null) {
            database.close();
        }
    }
}

One thing I've noticed, is that if I do not close the database in those methods, performance is back to normal. I suppose that getReadableDatabase() is reusing the instance in multiple calls. If not, there might be a problem in the close() method.

@developernotes
Copy link
Member

Hi @jcrabanal,

You typically do not want to close the database connection returned from either getReadableDatabase(), or getWritableDatabase() of a SQLiteOpenHelper derivative at the end of a query. When a new connection is created (because it is not open), key derivation must be performed which is slow by design.

@jcrabanal
Copy link

Hi, I've removed the close() method calls on my SQLiteOpenHelper subclasses. Several stackoverflow posts say that it should not be necessary, but the reason it is there is because I'm worried about database corruption, if the process goes away with db connections not closed.

Do you have any info on this?

Is this also true for regular SQLiteDatabase.open() calls? Is closing the database necessary there?

@developernotes
Copy link
Member

Hello @jcrabanal,

It is desirable to leave the connection open for the lifetime of the application, if possible. This is to prevent repeated key derivation from occurring. You should close the connection when the application is exiting. SQLite has documented various ways a database can become corrupt 1.

Footnotes

  1. https://www.sqlite.org/howtocorrupt.html

@EpariNigam
Copy link

EpariNigam commented Nov 17, 2024

Hey @developernotes and @sjlombardo,

Can you share an example on how to reduce key derivation on Android, especially using room?

@sjlombardo
Copy link
Member

@EpariNigam See this section of the performance optimization guide: https://www.zetetic.net/sqlcipher/performance/#adjust-key-derivation

@EpariNigam
Copy link

@sjlombardo That I understood, but when to execute that is the question using Room database?

@sjlombardo
Copy link
Member

@EpariNigam - To adjust KDF iterations, use a SQLiteDatabaseHook calling PRAGMA kdf_iter from the postKey method and pass it to the SupportOpenHelperFactory, e.g.

SQLiteDatabaseHook hook = new SQLiteDatabaseHook() {
    public void preKey(SQLiteConnection connection) {}
    public void postKey(SQLiteConnection connection) {
      connection.execute("PRAGMA kdf_iter = 64000;", null, null);
    }
  };

To use a Raw Key you just need to format it properly.

@sjlombardo
Copy link
Member

The connection pool isn't configurable currently - #27 (comment)

@sjlombardo would it be possible to make the pool configurable to help address this? Start up time is a critical metric for Android apps, and this is a real killer.

@eygraber - the next release will allow you to modify the pool size for use with WAL mode.

@DavidCorrado
Copy link

Hey I noticed that this thread is used often for performance issues related to updating the different libraries. I am seeing the same kind of issues here. We upgraded to the new library and found similar performance issues. Specifically on older Android API version devices. We basically were getting app starts that are over the 5 second threshold for cold starts. So we had an engineer with Android 10 test out some numbers. Might be useful
Before upgrade we were around 958 ms. Then after upgrade 2010 ms. Then after not using WAL. .setJournalMode(RoomDatabase.JournalMode.TRUNCATE) 912 ms. So it appears the WAL does affect start greatly.

So it appears its known that the non WAL mode is known to be slower on app start but the thought is its faster after that. Is there a way we can get documentation and measurement towards this? Then also set an appropriate default based on that. Like just updating to WAL seems to put old API versions at risk of being super slow start which might not be what you set as the default until the WAL configuration is figured out on how to improve app start time.

Let me know what you are thinking. In the mean time disabling WAL is what we are doing.

@DavidCorrado
Copy link

Should we add this WAL disabling to the performance considerations?
https://www.zetetic.net/sqlcipher/performance/
We are actively auditing those.

It seems like if we use a random hash we also do not need the key derivation too right?

@developernotes
Copy link
Member

Hi @DavidCorrado,

We are considering some changes for the next release of SQLCipher which would adjust the default connection handling behavior within SQLCipher for Android along with support for adjusting the connection pool size. In addition, we plan to include some documentation updates to the SQLCipher performance guidance to further clarify the needs when using the Room API in conjunction with SQLCipher.

@EpariNigam
Copy link

Hey @sjlombardo I tested, I can see some improvements.

But it is not working for existing databases, how to handle them?

@sjlombardo
Copy link
Member

But it is not working for existing databases, how to handle them?

You would need to use sqlcipher_export() to convert the original databases to use a reduced number of KDF iterations:

https://www.zetetic.net/sqlcipher/sqlcipher-api/#sqlcipher_export

This is an advanced operation that is outside the realm of standard public support. The operations are all documented, but this is not something we would provide step-by-step support for on this ticket.

@developernotes
Copy link
Member

Hi @DavidCorrado,

It seems like if we use a random hash we also do not need the key derivation too right?

If you provide a raw key 1, key derivation will not occur.

Footnotes

  1. https://www.zetetic.net/sqlcipher/sqlcipher-api/#example-2-raw-key-data-without-key-derivation

@EpariNigam
Copy link

Hey @developernotes any timeline for setting number of DB connections?

@developernotes
Copy link
Member

Hi @EpariNigam,

Changes will be included in the next public release of SQLCipher. We do not have a date to share at this time, however, we will post on our blog as well as the SQLCipher Discuss site upon release.

@sjlombardo
Copy link
Member

The recent release of SQLCipher 4.7.0 includes changes to the default connection handling behavior within SQLCipher for Android which should improve initial performance. You can also adjust connection pool size via SQLiteGlobal.setWALConnectionPoolSize(). The release also includes some significant performance improvements related to CursorWindow. Taken together, these should improve performance for the cases described in this thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants