Skip to content
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

make clearer that "database encryption" is highly experimental and will slow down devices and notifications #1989

Closed
Simon-Laux opened this issue Nov 19, 2023 · 5 comments · Fixed by #1990

Comments

@Simon-Laux
Copy link
Member

Simon-Laux commented Nov 19, 2023

This delays appstart and thus is annoying at least.
Also could be another contributing factor to our notification problems.

@Simon-Laux Simon-Laux added the bug label Nov 19, 2023
@r10s r10s added discussion and removed bug labels Nov 20, 2023
@r10s
Copy link
Member

r10s commented Nov 20, 2023

in general, iirc, sqlcipher uses key derivation, which is slow by design

this is also the reason that we keep database and app open in background as long as we can

nb: for me even unencrypted desktop takes a handful of seconds to start with a large database (another annecdotical datapoint :) - so 5s on a maybe older phone may be just fine

so, you'd need to provide more information and measurements, however, as the whole thing is experimental, and questionable on iOS at all, not sure if i would throw much time on that currently

but i agree, for notifications that may be really bad, if you have a small timeslot only. maybe we should end that experiment completely and reconsider when notofications are improved

@Simon-Laux
Copy link
Member Author

this is also the reason that we keep database and app open in background as long as we can

we can not do that because of #1979, maybe if we change where the db is saved and change how the share-extension works? maybe then apple would allow us to hold the locks in the background?

nb: for me even unencrypted desktop takes a handful of seconds to start with a large database (another annecdotical datapoint :) - so 5s on a maybe older phone may be just fine

On desktop:

  • it is one time, not every time I reopen the app
  • it does not look like the app is frozen in desktop during loading
  • much of it is probably also due to rosetta emulation

in general, iirc, sqlcipher uses key derivation, which is slow by design

That's why other programs like keepass offer a way to test how many derivations you can do in one second and apply that value.

@Simon-Laux Simon-Laux added the bug label Nov 20, 2023
@r10s
Copy link
Member

r10s commented Nov 20, 2023

this is also the reason that we keep database and app open in background as long as we can

we can not do that because of #1979

if we fix crashes: what prevents us from running the possible 30 seconds in the background?

@r10s r10s closed this as completed Nov 20, 2023
@r10s r10s reopened this Nov 20, 2023
@r10s
Copy link
Member

r10s commented Nov 20, 2023

On desktop:

it is one time, not every time I reopen the app
it does not look like the app is frozen in desktop during loading
much of it is probably also due to rosetta emulation

sure, sure, sure. i wanted to point out that 5-6 seconds may be normal for larger databases and slower devices

anyways, as discussed one-to-one, sqlcipher discussions, as being an experimental side-thing, should be removed from notification discussions, it is unclear how useful sqlcipher is on iOS at all.

but we can make things clearer in the UI that the feature is not recommended at all and will slow down things (eg. on anroid, we speak of "Highly experimental - use at your own risk")

i will do a pr for that

@r10s r10s changed the title Opening encrypted account takes 5-6 seconds make clearer that "database encryption" is highly experimental and will slow down devices and notifications Nov 20, 2023
@Simon-Laux
Copy link
Member Author

FTR: I seem to not have this (6 second) problem when there is no debugger attached, but I need to look at this aspect for a few more days using the current version.

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

Successfully merging a pull request may close this issue.

2 participants