Bug 148854 - Android Viewer runs out of memory when scrolling up and down in Calc doc in experimental editing mode
Summary: Android Viewer runs out of memory when scrolling up and down in Calc doc in e...
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Android Viewer (show other bugs)
Version:
(earliest affected)
7.4.0.0 alpha0+
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard:
Keywords:
: 158300 (view as bug list)
Depends on:
Blocks: Crash
  Show dependency treegraph
 
Reported: 2022-04-29 15:24 UTC by Michael Weghorn
Modified: 2023-11-23 14:13 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments
Screenshot of Perfetto memory profile (web UI) (238.44 KB, image/png)
2022-05-17 04:21 UTC, Michael Weghorn
Details

Note You need to log in before you can comment on or make changes to this bug.
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. ***