OpenSeizureDetector / Android_Pebble_SD

The main OpenSeizureDetector Android App, that is published on the Android Play Store.
http://openseizuredetector.org.uk
GNU General Public License v3.0
9 stars 11 forks source link

Crash when trying to export data #120

Closed jones139 closed 10 months ago

jones139 commented 10 months ago

Using the data export function to export 12 hours of data resulted in the following crash. It works ok for 2 hours of data.

***** DEVICE INFO Brand: samsung Device: j5y17lte Model: SM-J530F Manufacturer: samsung Product: j5y17ltexx SDK: 28 Release: 9

***** APP INFO Version: 4.1.10 Installed On: 2022-04-14 16:19:04 Updated On: 2023-07-26 07:56:13 Current Date: 2023-08-21 08:53:57

***** ERROR LOG java.lang.OutOfMemoryError: Failed to allocate a 150994952 byte allocation with 25165824 free bytes and 61MB until OOM, max allowed footprint 162135992, growth limit 201326592 at java.util.Arrays.copyOf(Arrays.java:3260) at java.lang.AbstractStringBuilder.ensureCapacityInternal(AbstractStringBuilder.java:125) at java.lang.AbstractStringBuilder.append(AbstractStringBuilder.java:660) at java.lang.StringBuilder.append(StringBuilder.java:203) at org.json.JSONStringer.string(JSONStringer.java:344) at org.json.JSONStringer.value(JSONStringer.java:252) at org.json.JSONObject.writeTo(JSONObject.java:723) at org.json.JSONStringer.value(JSONStringer.java:237) at org.json.JSONArray.writeTo(JSONArray.java:613) at org.json.JSONArray.toString(JSONArray.java:585) at uk.org.openseizuredetector.LogManager.cursor2Json(LogManager.java:220) at uk.org.openseizuredetector.LogManager.lambda$getDatapointsByDate$0$uk-org-openseizuredetector-LogManager(LogManager.java:473) at uk.org.openseizuredetector.LogManager$$ExternalSyntheticLambda5.accept(Unknown Source:4) at uk.org.openseizuredetector.LogManager$SelectQueryTask.onPostExecute(LogManager.java:754) at uk.org.openseizuredetector.LogManager$SelectQueryTask.onPostExecute(LogManager.java:701) at android.os.AsyncTask.finish(AsyncTask.java:695) at android.os.AsyncTask.access$600(AsyncTask.java:180) at android.os.AsyncTask$InternalHandler.handleMessage(AsyncTask.java:712) at android.os.Handler.dispatchMessage(Handler.java:106) at android.os.Looper.loop(Looper.java:216) at android.app.ActivityThread.main(ActivityThread.java:7266) at java.lang.reflect.Method.invoke(Native Method) at com.android.internal.os.RuntimeInit$MethodAndArgsCaller.run(RuntimeInit.java:494) at com.android.internal.os.ZygoteInit.main(ZygoteInit.java:975)

jones139 commented 10 months ago

The export data function is extremely slow and it looks like it is eating loads of memory - there must be a better way to do it. The relevant code is here: https://github.com/OpenSeizureDetector/Android_Pebble_SD/blob/master/app/src/main/java/uk/org/openseizuredetector/ExportDataActivity.java#L268 There is definitely an issue with running this on the UI thread, but moving it to a separate thread will not solve the memory consumption problem....

I think the thing to do is not use LogManager. cursor2json but instead receive the cursor and write the csv file directly to avoid the overhead of turning it into json and parsing back?

jones139 commented 10 months ago

This is quite a serious bug as it prevents users obtaining their data easily, so I'll do a quick fix as V4.1.11 rather than waiting for V4.2.

AroonPro commented 10 months ago

Topic https://stackoverflow.com/questions/31367270/exporting-sqlite-database-to-csv-file-in-android

Is giving handles to it. Android has async callback to give feedback back to the UI

Op ma 21 aug. 2023 15:27 schreef Graham Jones @.***>:

This is quite a serious bug as it prevents users obtaining their data easily, so I'll do a quick fix as V4.1.11 rather than waiting for V4.2.

— Reply to this email directly, view it on GitHub https://github.com/OpenSeizureDetector/Android_Pebble_SD/issues/120#issuecomment-1686330221, or unsubscribe https://github.com/notifications/unsubscribe-auth/AV6J745443LP63P2PKVP3TTXWNO37ANCNFSM6AAAAAA3YMFKK4 . You are receiving this because you are subscribed to this thread.Message ID: @.*** com>

jones139 commented 10 months ago

Thanks, I'll have a look at that. Ours will be a bit more complicated because I'd like to parse the dataJSON string into the csv, but hopefully that solution might avoid the memory consumption problem.

Graham

On Mon, 21 Aug 2023, 16:14 AroonPro, @.***> wrote:

Topic

https://stackoverflow.com/questions/31367270/exporting-sqlite-database-to-csv-file-in-android

Is giving handles to it. Android has async callback to give feedback back to the UI

Op ma 21 aug. 2023 15:27 schreef Graham Jones @.***>:

This is quite a serious bug as it prevents users obtaining their data easily, so I'll do a quick fix as V4.1.11 rather than waiting for V4.2.

— Reply to this email directly, view it on GitHub < https://github.com/OpenSeizureDetector/Android_Pebble_SD/issues/120#issuecomment-1686330221>,

or unsubscribe < https://github.com/notifications/unsubscribe-auth/AV6J745443LP63P2PKVP3TTXWNO37ANCNFSM6AAAAAA3YMFKK4>

. You are receiving this because you are subscribed to this thread.Message ID: @.*** com>

— Reply to this email directly, view it on GitHub https://github.com/OpenSeizureDetector/Android_Pebble_SD/issues/120#issuecomment-1686523667, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACLSY652NAE5R7EF43A5QDXWN3NZANCNFSM6AAAAAA3YMFKK4 . You are receiving this because you were assigned.Message ID: @.***>

jones139 commented 10 months ago

a4b6a43 should fix the out of memory crash - we only process the data one row at a time rather than all at once. It is still a bit slow so you get some "app not responding" prompts which will need fixing in the future, but it works as long as you press 'wait', so closing this issue.

AroonPro commented 10 months ago

https://github.com/OpenSeizureDetector/Android_Pebble_SD/blob/V4.2.x/app/src/main/java/uk/org/openseizuredetector/LogManager.java#L503

HashMap is recreated inside an active loop. This will only be cleared when leaving the context.

Op ma 21 aug. 2023 17:32 schreef Graham Jones @.***>:

Thanks, I'll have a look at that. Ours will be a bit more complicated because I'd like to parse the dataJSON string into the csv, but hopefully that solution might avoid the memory consumption problem.

Graham

On Mon, 21 Aug 2023, 16:14 AroonPro, @.***> wrote:

Topic

https://stackoverflow.com/questions/31367270/exporting-sqlite-database-to-csv-file-in-android

Is giving handles to it. Android has async callback to give feedback back to the UI

Op ma 21 aug. 2023 15:27 schreef Graham Jones @.***>:

This is quite a serious bug as it prevents users obtaining their data easily, so I'll do a quick fix as V4.1.11 rather than waiting for V4.2.

— Reply to this email directly, view it on GitHub <

https://github.com/OpenSeizureDetector/Android_Pebble_SD/issues/120#issuecomment-1686330221>,

or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AV6J745443LP63P2PKVP3TTXWNO37ANCNFSM6AAAAAA3YMFKK4>

. You are receiving this because you are subscribed to this thread.Message ID: @.*** com>

— Reply to this email directly, view it on GitHub < https://github.com/OpenSeizureDetector/Android_Pebble_SD/issues/120#issuecomment-1686523667>,

or unsubscribe < https://github.com/notifications/unsubscribe-auth/AACLSY652NAE5R7EF43A5QDXWN3NZANCNFSM6AAAAAA3YMFKK4>

. You are receiving this because you were assigned.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/OpenSeizureDetector/Android_Pebble_SD/issues/120#issuecomment-1686556708, or unsubscribe https://github.com/notifications/unsubscribe-auth/AV6J745US463XDO66KT2FATXWN5STANCNFSM6AAAAAA3YMFKK4 . You are receiving this because you commented.Message ID: @.***>

AroonPro commented 10 months ago

The same counts for https://github.com/OpenSeizureDetector/Android_Pebble_SD/blob/V4.2.x/app/src/main/java/uk/org/openseizuredetector/ExportDataActivity.java#L283-L287

Creating variables inside lead to memory build-up. Easy fix

Op di 22 aug. 2023 06:08 schreef Bram Regtien @.***>:

https://github.com/OpenSeizureDetector/Android_Pebble_SD/blob/V4.2.x/app/src/main/java/uk/org/openseizuredetector/LogManager.java#L503

HashMap is recreated inside an active loop. This will only be cleared when leaving the context.

Op ma 21 aug. 2023 17:32 schreef Graham Jones @.***>:

Thanks, I'll have a look at that. Ours will be a bit more complicated because I'd like to parse the dataJSON string into the csv, but hopefully that solution might avoid the memory consumption problem.

Graham

On Mon, 21 Aug 2023, 16:14 AroonPro, @.***> wrote:

Topic

https://stackoverflow.com/questions/31367270/exporting-sqlite-database-to-csv-file-in-android

Is giving handles to it. Android has async callback to give feedback back to the UI

Op ma 21 aug. 2023 15:27 schreef Graham Jones @.***>:

This is quite a serious bug as it prevents users obtaining their data easily, so I'll do a quick fix as V4.1.11 rather than waiting for V4.2.

— Reply to this email directly, view it on GitHub <

https://github.com/OpenSeizureDetector/Android_Pebble_SD/issues/120#issuecomment-1686330221>,

or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AV6J745443LP63P2PKVP3TTXWNO37ANCNFSM6AAAAAA3YMFKK4>

. You are receiving this because you are subscribed to this thread.Message ID: @.*** com>

— Reply to this email directly, view it on GitHub < https://github.com/OpenSeizureDetector/Android_Pebble_SD/issues/120#issuecomment-1686523667>,

or unsubscribe < https://github.com/notifications/unsubscribe-auth/AACLSY652NAE5R7EF43A5QDXWN3NZANCNFSM6AAAAAA3YMFKK4>

. You are receiving this because you were assigned.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/OpenSeizureDetector/Android_Pebble_SD/issues/120#issuecomment-1686556708, or unsubscribe https://github.com/notifications/unsubscribe-auth/AV6J745US463XDO66KT2FATXWN5STANCNFSM6AAAAAA3YMFKK4 . You are receiving this because you commented.Message ID: @.***>

jones139 commented 10 months ago

Ah, that was probably it then. I get too careless with Java by trusting the garbage collector. If it was C I'd be much more careful! I don't use that now- we go straight from the DB cursor to the csv file: https://github.com/OpenSeizureDetector/Android_Pebble_SD/compare/master...V4.1.x#diff-e10be5e2def6ca2fd5f4ad55b8a98b82485ad04706f9531644a0c0b6cc831b89R495

On Tue, 22 Aug 2023, 05:08 AroonPro, @.***> wrote:

https://github.com/OpenSeizureDetector/Android_Pebble_SD/blob/V4.2.x/app/src/main/java/uk/org/openseizuredetector/LogManager.java#L503

HashMap is recreated inside an active loop. This will only be cleared when leaving the context.

Op ma 21 aug. 2023 17:32 schreef Graham Jones @.***>:

Thanks, I'll have a look at that. Ours will be a bit more complicated because I'd like to parse the dataJSON string into the csv, but hopefully that solution might avoid the memory consumption problem.

Graham

On Mon, 21 Aug 2023, 16:14 AroonPro, @.***> wrote:

Topic

https://stackoverflow.com/questions/31367270/exporting-sqlite-database-to-csv-file-in-android

Is giving handles to it. Android has async callback to give feedback back to the UI

Op ma 21 aug. 2023 15:27 schreef Graham Jones @.***>:

This is quite a serious bug as it prevents users obtaining their data easily, so I'll do a quick fix as V4.1.11 rather than waiting for V4.2.

— Reply to this email directly, view it on GitHub <

https://github.com/OpenSeizureDetector/Android_Pebble_SD/issues/120#issuecomment-1686330221>,

or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AV6J745443LP63P2PKVP3TTXWNO37ANCNFSM6AAAAAA3YMFKK4>

. You are receiving this because you are subscribed to this thread.Message ID: @.*** com>

— Reply to this email directly, view it on GitHub <

https://github.com/OpenSeizureDetector/Android_Pebble_SD/issues/120#issuecomment-1686523667>,

or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AACLSY652NAE5R7EF43A5QDXWN3NZANCNFSM6AAAAAA3YMFKK4>

. You are receiving this because you were assigned.Message ID: @.***>

— Reply to this email directly, view it on GitHub < https://github.com/OpenSeizureDetector/Android_Pebble_SD/issues/120#issuecomment-1686556708>,

or unsubscribe < https://github.com/notifications/unsubscribe-auth/AV6J745US463XDO66KT2FATXWN5STANCNFSM6AAAAAA3YMFKK4>

. You are receiving this because you commented.Message ID: @.***>

— Reply to this email directly, view it on GitHub https://github.com/OpenSeizureDetector/Android_Pebble_SD/issues/120#issuecomment-1687385665, or unsubscribe https://github.com/notifications/unsubscribe-auth/AACLSY52CMDUZSUFNOF2723XWQWD3ANCNFSM6AAAAAA3YMFKK4 . You are receiving this because you modified the open/close state.Message ID: @.*** com>

AroonPro commented 10 months ago

Graham,

That looks great.

Can you also take care of timeconversions?

https://github.com/OpenSeizureDetector/Android_Pebble_SD/compare/master...V4.1.x#diff-e10be5e2def6ca2fd5f4ad55b8a98b82485ad04706f9531644a0c0b6cc831b89R495

We can use my new osdutil.convertTimeUnit

Op di 22 aug. 2023 08:18 schreef Graham Jones @.***>:

Ah, that was probably it then. I get too careless with Java by trusting the garbage collector. If it was C I'd be much more careful! I don't use that now- we go straight from the DB cursor to the csv file:

https://github.com/OpenSeizureDetector/Android_Pebble_SD/compare/master...V4.1.x#diff-e10be5e2def6ca2fd5f4ad55b8a98b82485ad04706f9531644a0c0b6cc831b89R495

On Tue, 22 Aug 2023, 05:08 AroonPro, @.***> wrote:

https://github.com/OpenSeizureDetector/Android_Pebble_SD/blob/V4.2.x/app/src/main/java/uk/org/openseizuredetector/LogManager.java#L503

HashMap is recreated inside an active loop. This will only be cleared when leaving the context.

Op ma 21 aug. 2023 17:32 schreef Graham Jones @.***>:

Thanks, I'll have a look at that. Ours will be a bit more complicated because I'd like to parse the dataJSON string into the csv, but hopefully that solution might avoid the memory consumption problem.

Graham

On Mon, 21 Aug 2023, 16:14 AroonPro, @.***> wrote:

Topic

https://stackoverflow.com/questions/31367270/exporting-sqlite-database-to-csv-file-in-android

Is giving handles to it. Android has async callback to give feedback back to the UI

Op ma 21 aug. 2023 15:27 schreef Graham Jones @.***>:

This is quite a serious bug as it prevents users obtaining their data easily, so I'll do a quick fix as V4.1.11 rather than waiting for V4.2.

— Reply to this email directly, view it on GitHub <

https://github.com/OpenSeizureDetector/Android_Pebble_SD/issues/120#issuecomment-1686330221>,

or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AV6J745443LP63P2PKVP3TTXWNO37ANCNFSM6AAAAAA3YMFKK4>

. You are receiving this because you are subscribed to this thread.Message ID: @.*** com>

— Reply to this email directly, view it on GitHub <

https://github.com/OpenSeizureDetector/Android_Pebble_SD/issues/120#issuecomment-1686523667>,

or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AACLSY652NAE5R7EF43A5QDXWN3NZANCNFSM6AAAAAA3YMFKK4>

. You are receiving this because you were assigned.Message ID: @.***>

— Reply to this email directly, view it on GitHub <

https://github.com/OpenSeizureDetector/Android_Pebble_SD/issues/120#issuecomment-1686556708>,

or unsubscribe <

https://github.com/notifications/unsubscribe-auth/AV6J745US463XDO66KT2FATXWN5STANCNFSM6AAAAAA3YMFKK4>

. You are receiving this because you commented.Message ID: @.***>

— Reply to this email directly, view it on GitHub < https://github.com/OpenSeizureDetector/Android_Pebble_SD/issues/120#issuecomment-1687385665>,

or unsubscribe < https://github.com/notifications/unsubscribe-auth/AACLSY52CMDUZSUFNOF2723XWQWD3ANCNFSM6AAAAAA3YMFKK4>

. You are receiving this because you modified the open/close state.Message ID: @.*** com>

— Reply to this email directly, view it on GitHub https://github.com/OpenSeizureDetector/Android_Pebble_SD/issues/120#issuecomment-1687523651, or unsubscribe https://github.com/notifications/unsubscribe-auth/AV6J7437DKJWH3COJDMO26TXWRFKHANCNFSM6AAAAAA3YMFKK4 . You are receiving this because you commented.Message ID: @.***>