Bug 133557 - Rejecting track & changes in Calc slow
Summary: Rejecting track & changes in Calc slow
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
4.4.0.3 release
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:26.2.0
Keywords: bibisected, bisected, perf, regression
Depends on:
Blocks: Performance Calc-Track-Changes CPU-AT-100%
  Show dependency treegraph
 
Reported: 2020-05-31 16:10 UTC by Telesto
Modified: 2025-10-13 06:46 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments
Bibisect log (3.62 KB, text/plain)
2020-05-31 16:11 UTC, Telesto
Details
Flamegraph (14.85 KB, application/x-bzip)
2022-06-21 20:32 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Telesto 2020-05-31 16:10:42 UTC
Description:
Rejecting track & changes in Calc slow

Steps to Reproduce:
1. Open attachment 161462 [details]
2. Enable tracking changes
3. Delete column D
4. Delete column E (ref error or 0)
5. Edit -> track and changes -> Show track and changes
6. Reject all

Actual Results:
Restore to original state in maybe 30 seconds

Expected Results:
Restore to original state in 8 seconds


Reproducible: Always


User Profile Reset: No



Additional Info:
Version: 7.1.0.0.alpha0+ (x64)
Build ID: 83c4f86f22dc37269ac6a038fe7de053c42aad6e
CPU threads: 4; OS: Windows 6.3 Build 9600; UI render: Skia/Raster; VCL: win
Locale: en-US (nl_NL); UI: en-US
Calc: CL

also found in
4.4.7.2

but not in
4.3.7.2
Comment 1 Telesto 2020-05-31 16:11:44 UTC
Created attachment 161463 [details]
Bibisect log

Bibisect attempt, but likely wrong
Comment 2 Buovjaga 2020-06-14 18:47:20 UTC
Bibisected with Linux 44max to https://git.libreoffice.org/core/+/797db638870f5c6192e72c0b4669b471788e16ea%5E!/
Use group area listener when entering a new single formula cell.

Seems to be the real deal as I tested flipping between good/bad.

Step 5. is "Manage changes" - not about showing changes.
Comment 3 Roman Kuznetsov 2022-06-20 14:39:40 UTC
Still repro in

Version: 7.5.0.0.alpha0+ / LibreOffice Community
Build ID: e4d23c27288b99c3ed3cfa332ff308b31c01f97d
CPU threads: 4; OS: Linux 5.14; UI render: default; VCL: gtk3
Locale: ru-RU (ru_RU.UTF-8); UI: en-US
Calc: threaded Jumbo

It took over 2 minutes for me and I killed the LO

I saw 100% CPU load btw, so I added the 100% CPU META

Julien, could you please create a perfgraph here?
Comment 4 Julien Nabet 2022-06-21 20:32:33 UTC
Created attachment 180883 [details]
Flamegraph

Here's a Flamegraph retrieved on pc Debian x86-64 with master sources updated today.
I started the trace when choosing "Reject all".
Comment 5 Julien Nabet 2022-06-21 20:35:30 UTC
László: I noticed patches from you but on sw part. Do you also know sc part or do you know whom to ping?
Comment 6 László Németh 2022-06-22 07:11:55 UTC
(In reply to Julien Nabet from comment #5)
> László: I noticed patches from you but on sw part. Do you also know sc part
> or do you know whom to ping?

@Julien: I've added Kohei to the CC list, maybe he could check this or forward it. Thanks, László
Comment 7 Buovjaga 2025-05-10 16:29:53 UTC
Still repro

Arch Linux 64-bit
Version: 25.8.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: 0d62ab61e982a0967386e40f21829fc95870b969
CPU threads: 8; OS: Linux 6.14; UI render: default; VCL: kf6 (cairo+wayland)
Locale: fi-FI (fi_FI.UTF-8); UI: en-US
Calc: CL threaded
Built on 10 May 2025
Comment 8 Noel Grandin 2025-10-01 09:25:05 UTC
Kohei, looking at the profile for this, we spend most of our time hooking up the listeners in SharedFormulaUtil::startListeningAsGroup.

What is odd in that particular function,
is that for svSingleRef formula, we are doing a slow thing (hooking up individual listeners), 
while for svDoubleRef, we are using the BASM machinery, which shares listeners.

Any suggestions?
Comment 9 Commit Notification 2025-10-03 11:35:30 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/e63e494d56c83c93db819532cd17574844366f02

tdf#133557 Rejecting track & changes in Calc slow

It will be available in 26.2.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 10 Kohei Yoshida 2025-10-11 21:25:37 UTC
(In reply to Noel Grandin from comment #8)

> What is odd in that particular function,
> is that for svSingleRef formula, we are doing a slow thing (hooking up
> individual listeners), 
> while for svDoubleRef, we are using the BASM machinery, which shares
> listeners.

That's how Calc has traditionally handled cell dependencies i.e. for single cell reference, store the broadcaster in the column and for range reference, us BASM.  This design decision even predates my involvement in this code base.

Prior to the cell storage rework in 4.2, the single cell listener was stored directly in ScBaseCell.  After the rework it got switched to multi_type_vector-based listener store, primarily to preserve the original design.

Or, perhaps by asking this question if you are implying that we should perhaps only use BASM to handle both single cell reference as well as range reference *everywhere*, and just drop this single cell listener thing in the column for good, then I do see your point, and I don't disagree with that.  In fact, my experimental alternative dependency tracker implementation in ixion, I did do exactly that, using mdds::rtree to store cell and cell range positions.

But making such change in Calc would be quite large undertaking. And whether or not the performance would be better is (though it potentially may be) still unknown.  That said, if I were to design a new mechanism from scratch I would go in that direction for sure, using mdds::rtree to store broadcaster position data.
Comment 11 Noel Grandin 2025-10-13 06:46:05 UTC
(In reply to Kohei Yoshida from comment #10)
> 
> That's how Calc has traditionally handled cell dependencies i.e. for single
> cell reference, store the broadcaster in the column and for range reference,
> us BASM.  This design decision even predates my involvement in this code
> base.
> 

Aha, so that is normal then.

> 
> Or, perhaps by asking this question if you are implying that we should
> perhaps only use BASM to handle both single cell reference as well as range

No, I was more hoping that this corner of code had been overlooked during the
BASM work and there was something simple I could do to speed things up.

Thanks for your feedback Kohei.