-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
OutOfMemoryError while opening a form with choices loaded from an external csv file #6446
Comments
The possibility I was thinking of is CSVs intended for search/pulldata parsing being read into memory instead or additionally, for example if forms weren't correctly being identified as needing search/pulldata parsing. |
I don't see anything like that, and even if it were true, I don't believe it would significantly impact memory usage because:
|
It's not the parsing that I'm worried about, it's the actually having it all in memory. As we saw with the 100k Entities form, modern low-end devices like my Samsung A13 don't have enough memory to represent even mid-sized choice lists. If a CSV was supposed to be parsed into a db for search/pulldata but was instead being read to memory, that would be a serious problem. Good to know that doesn't seem to be happening but it would be good to align our mental models on where memory gets used.
Doesn't it happen during the process of reading the form from file or cache? Regardless, we know for sure that the crash happens while loading a CSV into memory, right? Otherwise we wouldn't be in |
Yeah I don't see anything like that.
I was taking about CSVs used by
Yes and to be clear here it's about CVSs with choices but not these used by |
When There have been moments in time in the past where both happened in some cases (see getodk/javarosa#417, #5620 which is why the "extra" was introduced) so the situation I was describing is not theoretical. Now that I've refreshed my memory on that extra, I remember that it's used to so that the search/pulldata dbs are only populated if it's true so I agree with you that it's very unlikely to be related to the memory issue. |
As I remember I used the forms from #5780 (comment) for testing |
ODK Collect version
v2024.2.1 and newer versions
Device used
Most commonly OnePlus, Lenovo, Oppo, but also others that might appear more reliable, such as Samsung.
Problem description
We've recently received numerous reports in Crashlytics regarding
OutOfMemoryError
occurrences during form loading. See the following reports:These errors occur while reading external CSV attachments using
CsvExternalInstance
, which suggests they are triggered when forms use large choice lists loaded from external files (not throughsearch/pulldata
as initially suspected, @lognaturel). The stack trace rules out forms with entities as well, which would use a different parser (LocalEntitiesExternalInstanceParserFactory
).The code responsible for reading CSV files (
CsvExternalInstance
) was recently updated:However, I don't believe these changes could significantly impact memory usage, feel free to correct me if I'm mistaken, @seadowg.
I've also tested forms used last year when memory leaks were an issue (see this fix), and I don't see any major differences.
After thoroughly investigating, my conclusion is that this isn't a new issue. We likely encountered similar errors before, but they were previously caught and didn't appear as crashes. This changed in v2024.1, where we decided to let errors crash the app during form loading. While this isn't a new problem, we shouldn't ignore it, though I don't think it needs more immediate attention in the current release cycle.
Steps to reproduce the problem
This issue may be reproducible by opening a form with a large list of choices loaded from an external file on a device with limited resources. However, I have not been able to reproduce it.
Other information
One interesting aspect of these crash reports is that about 50% of them include an additional exception thrown while loading a form from the cache.
I've also looked into this and have some thoughts, but I'll file a separate issue for it since I don't believe it's directly related. While it might have a slight impact on memory usage, it shouldn't make a significant difference.
The text was updated successfully, but these errors were encountered: