Collect: review presence of "largeHeap = true" from android manifest

[pranavgupta1234]

Software and hardware versions

Android 5.0.2 API Level 21

Problem description

largeHeap option is enabled

Steps to reproduce the problem

Check manifest file

Expected behavior

This option should be removed

Other information

refer : http://stackoverflow.com/questions/27396892/what-are-advantages-of-setting-largeheap-to-true

[pranavgupta1234]

I had mostly experienced that enabling largeHeap is not considered a good practice and causes bad user experience. @lognaturel @yanokwa what are your views on this idea?

[yanokwa]

@pranavgupta1234 Please search the git history and find out what commit added largeHeap. Is there a comment in that comment that explains why it was added?

[batkinson]

@pranavgupta1234, like Yaw said, probably want to look at the reasons for the change. In this case, it looks like it was done for good reason. However, I wonder, is the interpretation and reason still the same once we bump the API version to 16+?

$ git log -SlargeHeap
...
$ git show 9545113e4411f7
commit 9545113e4411f7b1732bb4a68af7247a4c5d717e
Author: mitchellsundt <[email protected]>
Date:   Tue Jun 4 11:40:17 2013 -0700

    Add the largeHeap attribute to support larger forms on Android 3.0 and higher

[pranavgupta1234]

I am also not too sure (needs research) also android doc says :
android:largeHeap
Whether your application's processes should be created with a large Dalvik heap. This applies to all processes created for the application. It only applies to the first application loaded into a process; if you're using a shared user ID to allow multiple applications to use a process, they all must use this option consistently or they will have unpredictable results.

Most apps should not need this and should instead focus on reducing their overall memory usage for improved performance. Enabling this also does not guarantee a fixed increase in available memory, because some devices are constrained by their total available memory.
To query the available memory size at runtime, use the methods getMemoryClass() or getLargeMemoryClass().

[yanokwa]

Release notes from https://github.com/opendatakit/opendatakit/wiki/Collect-Release-Notes say:

supply 'largeHeap' attribute to enable ODK Collect to run with a larger heap (including stack) on some devices (dependent upon hardware manufacturer). Can enable more complex regex() functions.

I'm guessing largeHeap=true is needed because the underlying JavaRosa library can run out of memory with complex forms.

@pranavgupta1234 If you want to remove this, we'll need a form that has this problem. One with lots of regex functions running on a low memory emulator might give you the memory problems that the largeHeap solves. It might be good to search the ODK mailing lists.

Alternatively, @mitchellsundt do you have a form (or know how we can generate a form) that causes this problem?

If we can't reproduce the original problem, then my gut says we should leave this alone.

[batkinson]

So, I am not sure whether there were test cases used to validate the original issue being addressed. If there are concrete examples of large forms we wish to support, it would be a good idea to use a memory profiler to look at where the actual memory is being tied up. Often, especially when people aren't regularly measuring the actual memory profile, you can make huge improvements without going with the nuclear option (like largeHeap).

Haha, @yanokwa beat me to it.

[yanokwa]

@pranavgupta1234 You can find some sample XML to start at https://groups.google.com/d/msg/opendatakit/dQpdVooTiKE/ubF3NuOl6REJ and https://groups.google.com/d/msg/opendatakit/tPTYUnHVNT4/xTgg45NfB_cJ. I would remove the larger heap and see if you can get it to crash on a low RAM emulator...

[yanokwa]

@pranavgupta1234 Would you like to take this on?

[pranavgupta1234]

Sure @yanokwa I will try it out :)

[wbrunette]

Confirming @yanokwa "I'm guessing largeHeap=true is needed because the underlying JavaRosa library can run out of memory with complex forms." People who have complex forms (e.g. 500+ questions, some people have 1000+ question form) can run out of memory with JavaRosa, largeHeap enables more memory for the process on the mobile device. Unfortunately, JavaRosa is not written in a memory efficient way since it keeps everything in memory.

Memory options are a per device setting with Dalvik. There many per device settings depending on the OS (e.g., dalvik.vm.heapsize, dalvik.vm.heapgrowthlimit).

[grzesiek2010]

It was one of the points we mentioned during v1.27 planning so I investigated the issue and here are my findings:
I used Samsung Galaxy S4 with Android 5 and a form with multiple questions on one screen with images like:
images4.xlsx

with largeHeap="true" I was able to open a form with up to 90 questions without only up to 20.
Select one/multiple work much better because those widgets use RecyclerView so without largeHeap="true" I was able to open a question with 100 question and probably more also would work. There might be a crash during scrolling but only sometimes and it's not easy to reproduce.

I tried also big forms like those from: https://forum.getodk.org/t/share-your-biggest-or-slowest-form-and-win-a-badge/8984
but wasn't able to reproduce the crash.
I even tried mbtiles but also wasn't able to reproduce the crash.
However I think it might be possible to cause the crash in those two cases too it might be just more difficult.

My conclusion is that removing that flag without improvements in Collect is a bit dangerous.

[lognaturel]

Great analysis, @grzesiek2010.

When you tried the big forms, did you try entering in some data so that a large chain of dependent calculations would need to be re-evaluated? That's when we might expect memory issues.

I see display performance issues with field lists on my Galaxy Tab A and moderately-sized field lists so I think that's something we do need to address.

Currently widgets are just added in a huge LinearLayout which is absolutely not ideal. Moving towards using a RecyclerView there would make a lot of sense, I think.

Great to get more visibility into where the bottlenecks are and aren't. I think we need to discuss sequencing and prioritization as this wouldn't be a small change.