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

Autofill Memory Limits exceeded on relatively small database #353

Open
ixs opened this issue Mar 23, 2024 · 5 comments
Open

Autofill Memory Limits exceeded on relatively small database #353

ixs opened this issue Mar 23, 2024 · 5 comments
Labels
bug Something isn't working

Comments

@ixs
Copy link

ixs commented Mar 23, 2024

Description
I have recently started running into the memory issues related to autofill described already in
https://keepassium.com/articles/autofill-memory-limits/.

Database has always been at around 850kB but suddenly grew to around 4MB.
The DB is AES encrypted and thus should be easy on memory use. I think a 4MB file should not be a huge problem, even when expanded.

I did a bit of digging into the database using the pykeepass library.
Dumping the prettyfied XML structure seems to clock in at around 18MB. This does not seem that much for the database itself objectively but it seems this is already too much, as autofill just dies silently.

I was astonished however to see that my CustomIcons list is pretty big. The kdbx4 file on disk is 3.760.867 Bytes big.
Iterating over the Icon stucture, decoding the base64 stored icons and adding the binary-size together all the icons add up to 3.389.518 Bytes.

I am currently wrangling with the pykeepass library to see if I cannot clean the icons in an automated fashion as there are some 300+kB favicons in there which is ridiculous.

But in the meantime, I am wondering if the autofill extension couldn't be changed slightly to disregard icons completely when running in autofill mode.

That might save some memory and maybe prevent the extension from crashing?

Environment:

  • Device: iPhone 13mini
  • OS: 14.7.1
  • App Version: 1.49

This is happening with the KeePassium Pro version (the black icon one), not sure if it's relevant.

@ixs ixs added the bug Something isn't working label Mar 23, 2024
@keepassium
Copy link
Owner

Dumping the prettyfied XML structure seems to clock in at around 18MB. This does not seem that much for the database itself objectively but it seems this is already too much, as autofill just dies silently.

I agree, such an XML this should not be too big of a deal. Dying silently means running out of memory at XML parsing or processing stage.

As much as I would love to skip custom icons and attachments while loading in AutoFill, this is quite problematic. Due to an early design mistake, KeePassium uses a DOM parser (which loads the whole XML tree to memory), rather than an event-based SAX parser (which parses XML in small chunks and can make decisions during the process). To my defense, at the time of this decision AutoFill was not even announced, so memory consumption was not that important.

Well, now memory became important, but optimization requires rewriting the whole database processing code. Which is a large all-or-nothing task that cannot be done incrementally. It is indeed on the list, but there is always something more urgent…

</rant>

there are some 300+kB favicons in there which is ridiculous.

This might be the reason.

When adding larger favicons, KeePassium specifically downsizes them to 128x128 PNG. But when showing the existing ones, resizing is done only in UI. Which means, according to this post, that the original image, in full resolution and colors, is decompressed to memory for displaying. I guess a 300 kB compressed PNG turns into quite a heavy bitmap…

(By the way, were the large favicons added by KeePassium? If so, there is probably a separate issue in the downsizing code.)

@ixs
Copy link
Author

ixs commented Mar 27, 2024

I agree, such an XML this should not be too big of a deal. Dying silently means running out of memory at XML parsing or processing stage.

Thank you for confirming. This was my understanding as well.

As much as I would love to skip custom icons and attachments while loading in AutoFill, this is quite problematic. Due to an early design mistake, KeePassium uses a DOM parser (which loads the whole XML tree to memory), rather than an event-based SAX parser (which parses XML in small chunks and can make decisions during the process). To my defense, at the time of this decision AutoFill was not even announced, so memory consumption was not that important.

Right. DOM Parser seems like a sensible choice. If there were no memory pressure. Which there now unfortunately is. Bummer.

Well, now memory became important, but optimization requires rewriting the whole database processing code. Which is a large all-or-nothing task that cannot be done incrementally. It is indeed on the list, but there is always something more urgent…

Of course. But hey, easter is coming up, what better time to relax, ponder past sins and rewrite such code?! 😆

there are some 300+kB favicons in there which is ridiculous.

This might be the reason.

When adding larger favicons, KeePassium specifically downsizes them to 128x128 PNG. But when showing the existing ones, resizing is done only in UI. Which means, according to this post, that the original image, in full resolution and colors, is decompressed to memory for displaying. I guess a 300 kB compressed PNG turns into quite a heavy bitmap…

Interesting. Just read that post and if that's the case that would indeed explain the problem I'm seeing.

(By the way, were the large favicons added by KeePassium? If so, there is probably a separate issue in the downsizing code.)

I am not certain. I've been using KeepassXC and KeePassium in parallel and do not remember exactly, which icons were added where.

When I was looking for testcases for large icons in my database, I noticed a weird thing though:
The large icons in my db are all icons with 384 x 384px resolution.
But the source websites only offer smaller icons:

So it looks like something is actually saving the icons in a larger format than needed? Could that someone be KeePassium?

@keepassium
Copy link
Owner

keepassium commented Mar 27, 2024

Could that someone be KeePassium?

Guess what, you are absolutely right! KeePassium successfully "downscales" 180x180px icons into 128x128pt, which happens to be 384x384px on modern devices :) I've created a separate issue for that: #354

@keepassium
Copy link
Owner

Of course. But hey, easter is coming up, what better time to relax, ponder past sins and rewrite such code?! 😆

See, there is always something smaller and more urgent :)

@ixs
Copy link
Author

ixs commented Mar 27, 2024

See, there is always something smaller and more urgent :)

🤯

I wiped the 384x384px icons from the database.
Database is now back down to about 1MB and Autofill is not dying anymore with that size.
So I guess this is a win? 😆

Of course, the overall problem is still unsolved, autofill should not be dying over icons...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants