Bug 120436 - Setting cell's Formula property is an order of magnitude slower than setting its FormulaLocal
Summary: Setting cell's Formula property is an order of magnitude slower than setting ...
Status: RESOLVED WORKSFORME
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: BASIC (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard:
Keywords: perf
Depends on:
Blocks:
 
Reported: 2018-10-09 08:06 UTC by Mike Kaganski
Modified: 2022-07-11 13:11 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments
A test ODS with 2 macros to test Formula vs FormulaLocal performance (1.52 MB, application/vnd.oasis.opendocument.spreadsheet)
2018-10-09 08:06 UTC, Mike Kaganski
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Mike Kaganski 2018-10-09 08:06:32 UTC
Created attachment 145503 [details]
A test ODS with 2 macros to test Formula vs FormulaLocal performance

In the attached file, the two macros (testformulalocal and testformula) only differ in using FormulaLocal vs Formula. The performance of the latter is an order of magnitude slower than that of the former (testformulalocal executes in about 90 s on my system, while testformula takes ~1400 s).
Comment 1 Mike Kaganski 2018-10-09 08:59:37 UTC
It is necessary to run the testformulalocal macro with "Use English function names" enabled when using a non-English UI.
Comment 2 Buovjaga 2018-10-27 16:05:42 UTC
Repro the same proportion of time.
Do you want a callgrind trace?

Arch Linux 64-bit
Version: 6.2.0.0.alpha0+
Build ID: 9a373521d7a328197a4bf9abeb0a981b7acba896
CPU threads: 8; OS: Linux 4.18; UI render: default; VCL: gtk3_kde5; 
Locale: fi-FI (fi_FI.UTF-8); Calc: threaded
Built on 19 October 2018
Comment 3 QA Administrators 2019-10-28 03:29:49 UTC Comment hidden (obsolete)
Comment 4 QA Administrators 2021-10-28 04:15:06 UTC Comment hidden (obsolete)
Comment 5 Andreas Heinisch 2022-07-09 14:00:26 UTC
FormulaLocal parses every given text (formula) in the users native UI language in bool ScDocFunc::SetCellText and this process needs time, since it will be done every time.

Could we extend ScInputStringType ScStringUtil::parseInputString to get the correct local format to avoid scanning it over and over again?

Or we could avoid the parsing of the inputr string if it is a formula?
Comment 6 Mike Kaganski 2022-07-11 06:54:06 UTC
(In reply to Andreas Heinisch from comment #5)
> FormulaLocal parses every given text (formula) in the users native UI
> language in bool ScDocFunc::SetCellText and this process needs time, since
> it will be done every time.

Just to make sure we are on the same page: this issue is about *Formula* property being much slower than FormulaLocal, not the other way round.
Comment 7 Andreas Heinisch 2022-07-11 07:42:00 UTC
from 5 to 1500:

without the patch
FormulaLocal: 107s
Formula: 15s

with the patch:
FormulaLocal: 17s
Formula: 16s


The first run always takes a little bit longer on both Formula/FormulaLocal calls.
Comment 8 Mike Kaganski 2022-07-11 10:00:49 UTC
Likely, something has changed since then: *both* testformula started to work much faster, *and* testformulalocal started to work much slower.

And also it seems that I wasn't clear enough in STR. Let me explain.

To check, I opened the file in 6.2.0.3, and made changes according to comment 1 (I use Russian UI, and Use English function names).
Version: 6.2.0.3 (x64)
Build ID: 98c6a8a1c6c7b144ce3cc729e34964b47ce25d62
CPU threads: 12; OS: Windows 10.0; UI render: GL; VCL: win; 
Locale: ru-RU (ru_RU); UI-Language: ru-RU
Calc: CL

First I run testformulalocal (the first in the module). It took less than 10s on my system.
Then I run testformula, and it took very long. Just as I explained in the report.

Now I decided to double-check running testformulalocal again, and this time, testformulalocal took longer than 90 s. Not nearly as log as testformula, but it made it clear that the repro steps should had a step like "run testformulalocal once before further testing" to get the numbers reported back then. However, testformula worked very slowly even with clear column L. So in the end: the report was valid for version 6.2.

But let me clarify the problematic numbers from comment 0 using v.6.2:

testformulalocal took ~7s the first time on clean column L, and ~100s when run the second time on already filled column L.
testformula took ~860s the first time on clean column L, and ~1400s when run the second time on already filled column L.

Now I re-test using 7.3.5.1 (again, Russian UI, and Use English function names).
Version: 7.3.5.1 (x64) / LibreOffice Community
Build ID: d56c1c78db15939340c3db8ee3b6667832313d23
CPU threads: 12; OS: Windows 10.0 Build 19044; UI render: Skia/Raster; VCL: win
Locale: ru-RU (ru_RU); UI: ru-RU
Calc: CL

testformulalocal took ~13s first time (with clear column L). Re-running it again, with already filled column L, takes now ~650s. A regression in the second case.

testformula took ~11s first time (with clear column L). Re-running it again, with already filled column L, takes now ~24s - great improvement over v.6.2 in both cases.

I close this one as WORKSFORME (because Formula now works at least no longer than FormulaLocal now, and much better than in 6.2, and also faster than FormulaLocal took back then in the worst case). But the regression in FormulaLocal needs a separate issue, bisection and fixing. Andreas: I assume, this is what you do in https://gerrit.libreoffice.org/c/core/+/136942 - but I still see different figures compared to comment 7, so there's some additional aspect here that I don't see.
Comment 9 Andreas Heinisch 2022-07-11 12:34:44 UTC
My observations on:

5 to 15000
FormulaLocal
18s not filled
634s filled

Formula
314s not filled
35s filled

Version: 7.3.4.2 (x64) / LibreOffice Community
Build ID: 728fec16bd5f605073805c3c9e7c4212a0120dc5
CPU threads: 16; OS: Windows 10.0 Build 19044; UI render: Skia/Vulkan; VCL: win
Locale: de-DE (de_DE); UI: de-DE
Calc: CL
Comment 10 Mike Kaganski 2022-07-11 12:39:38 UTC
(In reply to Andreas Heinisch from comment #9)
> Formula
> 314s not filled
> 35s filled

Sorry to re-check: is 314 correct, or maybe it was a typo, and was meant e.g. 14?
Comment 11 Andreas Heinisch 2022-07-11 12:46:46 UTC
Was no typo: 314s was correct.
Comment 12 Mike Kaganski 2022-07-11 12:50:19 UTC
(In reply to Andreas Heinisch from comment #11)

Thank you for clarifying!
This zoo of different timings is highly confusing; the 314 makes your timings differ from mine :-)
Comment 13 Andreas Heinisch 2022-07-11 13:11:54 UTC
I think this is something different here, because I note that the formulas are already set in the UI, and the UI does not respond nor the macro ends in with the patch:

Version: 7.5.0.0.alpha0+ (x64) / LibreOffice Community
Build ID: 8e19c9defc0f7ca6e8eabc36261336743f946276
CPU threads: 16; OS: Windows 10.0 Build 19044; UI render: Skia/Raster; VCL: win
Locale: de-DE (de_DE); UI: en-US
Calc: CL