Possible memory leak in keyboard view #267
Comments
|
Hah the good ol' I will investigate into this. Does this always occur when going into the emoji view or does this only occur after some time has passed using FlorisBoard continuously (without a service restart)? If it is the latter, this could point to a memory leak, which is problematic and can cause all sorts of issues. |
|
It's definitely not the emoji-option, so probably the second thing. Also, this is the first time this ever happened to me. |
|
Are you able to reproduce this bug or did it just randomly appear once and now it is gone again? |
|
It just randomly happened once and now everything works finde again. |
|
This makes it definitely harder to fix this crash. I just did a memory stress test on my testing device but couldn't get the app to crash with an |
patrickgold
added
the
not reproducible
|
I'll do my best to contact you if and when it occurs again. |
patrickgold
changed the title
|
Sooo... Here you go: It happened while I was trying tp type sth. on Whatsapp while an overlay-youtube-video from Firefox was playing. The keyboard had lagged for a few minutes already (I think). It loaded for like a second, then disappeared. "Florisboard has stopped working". |
|
What really makes me wonder is why the emoji file is getting parsed in the background, even though you have been actively using FlorisBoard for quite a time. Normally, the emoji file gets parsed once (at the beginning of the Service lifetime) and that's it. So I am very concerned that something is resetting the layout cache and triggering an emoji reload over and over. As this file is quite big, this can very fast grow to a memory leak. Are you by any chance using adaptive themes? I have some suspicion that this could be (one of) the reason(s) for the internal rebuild trigger. |
What exactly is a memory leak?
At the time of my first report, I did use adaptive themes (i think). |
Under normal circumstances, an application dynamically allocates and dis-allocates memory from the system while in use. Some actions which happen often allocate memory often for temporary processing etc., but this temporary objects should get cleared. As Android runs on Java, memory dis-allocation is handled by the Garbage collector (GC). The GC checks every now and then if some allocated objects have any references. If they do, the GC skips this objects, else it dis-allocates the object and thus creating free memory again. If the app's logic has some flaw where not all references are cleared, the GC will not dis-allocate the memory and thus over time the RAM usage goes up and up but never down, until it hits the memory limit, where Java throws the
Hmm maybe themes in general do trigger an emoji re-parse, I will investigate the theme setter function in the core. |
Thank you for the thorough explanation!
Does that mean it shouldn't happen with the default theme? |
Nope, it will happen with any theme set I think, if the problem is the core theme rebuild logic. |
|
I see. This hasn't happened to anyone else, right? |
|
Not that I am aware of at least. I will conduct further memory stress tests on my older physical devices, maybe I am able to catch the source of the leak. |
|
The weird thing is: My phone is pretty good, actually. Making it confusing to me that the bug only seems to happen here. |
Yep this could very likely be the reason of this bug and would explain the layout/emoji data re-parse, as an orientation change causes an internal service re-build. |
|
Yay :) |
|
Okay, I can replicate it now.
It started loading long enough that I could notice the loading-screen, then it ltarted taking its time before even showing the loading-screen. Then the screen-rotations felt like they took longer and longer to start. ((The log in my previous comment is not the log from this last reproduction-crash)) |
|
Replicated, Moto G Power 2020, Android 10, 0.3.5
…On Mon, Feb 1, 2021, 06:04 Glitchy-Tozier ***@***.***> wrote:
Okay, I can replicate it now.
I just
1. opened some app (Github)
2. clicked on a text-field so that the keyboard was active
3. turned the screen back and forth 1-2 minutes.
It started loading long enough that I could notice the loading-screen,
then it ltarted taking its time before even showing the loading-screen.
Then the screen-rotations felt like they took longer and longer to start.
Then FlorisBoard got stuck on the loading-screen for a few seconds, before
crashing.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#267 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AN54XP7U4U6MJSKJ5XJGEULS42YHRANCNFSM4WXLP3VQ>
.
|
|
Thanks for your observations! I've profiled the memory state of FlorisBoard when rotating the screen often and found out, that emoji parsing is indeed triggered a lot, but it isn't even the biggest memory leak. The internal event listener chain didn't clear itself up (except for the rewritten theme manager), so the event listener count got bigger and bigger, up to a few 100k (!) items (The normal count is ~1800 listeners at max). This of course massively slowed down things. Another thing I observed is that there seems to be a memory leak in the creation of the UI itself, which of course happens again and again for every rotation. Smaller leaks are within the TabLayout from Android itself, but this is currently unfixable for me. Also, setting the corner radius for drawables seems to not allow the GC to clean up the previous cached bitmap, I will investigate into this as well. I am currently fixing and re-testing the memory state and will get back to you with more results. |
patrickgold
added
bug-confirmed
|
Above commit adds improvements and prevents memory leaks, in detail:
Note, that this is still far from perfect and I am almost certain that you'll be able to reproduce the crash even in the newest version, but it will probably take way more rotations of the screen before this will occur. Because of this, I will let this issue open, as it isn't fully resolved yet. |
|
I see. Are the remaining parts of this bug the ones you said you didn't know how to fix right now? |
|
Partly, but there are also some issues in some generic dispatcher handles, where I don't know yet what FlorisBoard method they are actually representing. |
Glitchy-Tozier
changed the title
|
I got this using the latest master version (commit 38baac1). Not sure if it is related to this, I can create a new issue if it's not. |
Thanks for reporting this, I think it is best if it stays within this issue, as this is related to a memory leak.
As you have the latest master debug version, did you have suggestions enabled or not? This makes a huge difference, because the memory management in the suggestions has some bugs in it. |
suggestions enabled and only English QWERTY subtype added I think it's the same problem as #403 |
|
Yup, I also think there's something wrong with my suggestion algorithm, I am currently looking into it. |
|
Hey there! Anyway, the keyboard is awesome, thank you for your hard work on it! |
This indicates to me that the described memory spikes in #711 could theoretically be one reason for the many crashes.
Yes, I have. Sadly LeakCanary only works properly for Activity or Fragment contexts, this IME's issues are primary in the Service context, which LeakCanary cannot really detect. |
No idea if that's helpful information but it's also possible to make Florisboard crash in a password-field, simply by rotating the device back and fourth. (v.0.3.10, suggestions and glide typing both disabled.) |
|
Also, the Florisboard-keyboard doesn't need to be active to crash. You can use it until it starts lagging, then go into the florisboard settings. When I deleted a few layouts, it Florisboard crashed. EDIT: Also, it looked as if the last changes I made seconds before the crash were never implemented. I had to re-delete layouts. |
The service class is active as soon as it was started and will be active until either it crashes, it is force-closed or if you switch to another keyboard. In all other cases, the service will continue to receive input events, even if it is physically not visible. |
Because the shared preference batch together changes to avoid writing to the disk too often, this issue occurs if the keyboard crashes seconds after setting a preference (deleting the subtype is settings a preference value). |
|
Reviewing the backlog of issues I saw how old the first report of OOM crashes is. A lot of work and changes has undergone this project since then and most of the classes described in this issue's stacktrace do not even exist in that form anymore. The main point of this issue where memory leaks in the text keyboard view (which have been pretty much fixed since v0.3.11-beta02's layout rework) and memory leaks in the emojis (which have been partially fixed and the UI creation has been vastly improved). The rotation bug will still be reproducible in v0.3.11, but should be a lot harder to get, as more and more objects are cached and reused instead of rebuilt when rebuilding the UI. As the OOM errors in general are batched together in v0.3.11, I think that closing this issue is best to keep the list clean. |
|
Kk, good job with 0.3.11, I'll have to find new ways of breaking your app then ;) |
The question is not if you find a new way to break the logic, but more when :) |
Version: 0.3.5
Source: Playstore
Android 10
Samsung Galaxy s9
The text was updated successfully, but these errors were encountered: