Bug 148854

Summary: Android Viewer runs out of memory when scrolling up and down in Calc doc in experimental editing mode
Product: LibreOffice Reporter: Michael Weghorn <m.weghorn>
Component: Android ViewerAssignee: Not Assigned <libreoffice-bugs>
Status: NEW ---    
Severity: normal CC: m.weghorn, sophie.sipasseuth, stephane.guillou
Priority: medium    
Version: 7.4.0.0 alpha0+   
Hardware: All   
OS: All   
See Also: https://bugs.documentfoundation.org/show_bug.cgi?id=148851
Whiteboard:
Crash report or crash signature: Regression By:
Bug Depends on:    
Bug Blocks: 133092    
Attachments: Screenshot of Perfetto memory profile (web UI)

Description Michael Weghorn 2022-04-29 15:24:40 UTC
This is a follow-up to tdf#148851 which fixed one memory leak that is also present with experimental editing mode disabled, but there is another problem that only shows up with experimental mode enabled.

Steps to reproduce:

1) enable the experimental editing mode in Android Viewer
2) open attachment 179845 [details] (from tdf#148851) in Android Viewer
3) switch to the first table in the spreadsheet, which has more rows than the second one
4) scroll down the document, then up again
5) repeat step 2 over and over again

Actual result:

At some point in time (in my case, I usually had to repeat step 2 about 2-10 times, depending on the device), the app crashes because it runs out of memory.

This is with git master as of commit 6c724e18c2af227c2ad865342531ff35b0d511ac + the pending fix for tdf#148851 in place.
Comment 1 Michael Weghorn 2022-04-29 15:30:35 UTC
Result of a first analysis:

This is caused by the way that Calc row and column headers (the "A", "B", "C", ... labels for columns and "1", "2", "3", ... for rows) are currently handled.

Android Viewer currently gets the headers for the whole current "document size", s. `LOKitTileProvider#getCalcHeaders`. However, this has the side effect of increasing the document size, triggered by this in  `ScTabView::getRowColumnHeaders`:

>
>     // 2) if we are approaching current max tiled row, signal a size changed event
>     // and invalidate the involved area
>     lcl_ExtendTiledDimension(/* bColumn */ false, nEndRow, nVisibleRows, *this, aViewData);

Properly fixing this would probably include reworking the way that Calc headers are handled in Android Viewer more fundamentally.

With this workaround/hack to no longer retrieve headers for the whole doc size, I no longer see that problem (but Calc headers are missing):

diff --git a/android/source/src/java/org/libreoffice/LOKitTileProvider.java b/android/source/src/java/org/libreoffice/LOKitTileProvider.java
index 0c7931763571..03e6f1a7abb7 100644
--- a/android/source/src/java/org/libreoffice/LOKitTileProvider.java
+++ b/android/source/src/java/org/libreoffice/LOKitTileProvider.java
@@ -396,7 +396,7 @@ class LOKitTileProvider implements TileProvider {
         long nWidth = mDocument.getDocumentWidth();
         long nHeight = mDocument.getDocumentHeight();
         return mDocument.getCommandValues(".uno:ViewRowColumnHeaders?x=" + nX + "&y=" + nY
-                + "&width=" + nWidth + "&height=" + nHeight);
+                + "&width=" + 10 + "&height=" + 10);
     }
 
     /**
Comment 2 disco 2022-05-13 06:19:53 UTC
I found if call method('getCalcHeaders') frequently it will be blocked.Such as frequently pause and resume the DocumentActivity
Comment 3 Michael Weghorn 2022-05-17 04:21:19 UTC
Created attachment 180141 [details]
Screenshot of Perfetto memory profile (web UI)

Attached is a screenshot of the web UI showing the memory profile created with Perfetto's "heap_profile" tool, which shows that updating formulas is causing high memory usage (and the delays also).

One potential solution (besides looking more closely into what exactly happens there) would probably be to only request the headers relevant for the portion of the doc that is currently shown (i.e. pass corresponding x and y values instead of the whole doc size), s.a. how gtktiledviewer does it.

I probably won't find the time to look into this in the coming weeks, but *might* return to it at some point in time later.

If anybody else wants to look into this, please feel free. :-)
Comment 4 Stéphane Guillou (stragu) 2023-08-29 12:50:58 UTC
Reproduced with current APK from F-Droid (7.6 alpha1+): scrolling up and down a for a while on the first "part" of the file suddenly closes the document.
Tested on a Samsung Galaxy A20e.
Comment 5 Stéphane Guillou (stragu) 2023-11-23 14:13:31 UTC
*** Bug 158300 has been marked as a duplicate of this bug. ***