Bug 117781 - Hang when copying a sheet to a new document
Summary: Hang when copying a sheet to a new document
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
6.1.0.0.alpha1+
Hardware: All All
: high major
Assignee: Mike Kaganski
URL:
Whiteboard: target:6.2.0 target:6.1.2
Keywords: bibisected, bisected, perf, regression
Depends on:
Blocks: CPU-AT-100%
  Show dependency treegraph
 
Reported: 2018-05-24 16:49 UTC by Telesto
Modified: 2018-09-03 17:59 UTC (History)
2 users (show)

See Also:
Crash report or crash signature:


Attachments
Dump from WinDbg (9.81 KB, text/plain)
2018-05-28 15:11 UTC, Timur
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Telesto 2018-05-24 16:49:57 UTC
Description:
Hang when copying a sheet to a new document

Steps to Reproduce:
1. Open attachment 129581 [details] (bug 104639)
2. Select the sheet "Opdrachten"
3. Sheet -> Move or Copy sheet
4. Select Copy & New document

Actual Results:  
It takes for every to finish 

Expected Results:
Should be done within 30 seconds


Reproducible: Always


User Profile Reset: No



Additional Info:
Found in
Version: 6.1.0.0.alpha1+
Build ID: b9ebabd675f916a0151ec3893a59131f3a8b2a05
CPU threads: 4; OS: Windows 6.3; UI render: default; 
TinderBox: Win-x86@42, Branch:master, Time: 2018-05-21_23:26:04
Locale: nl-NL (nl_NL); Calc: CL

but not in
Version: 6.0.0.1.0+
Build ID: 47cc374c0659fd3db74a1b184c870eaa56bc385b
CPU threads: 4; OS: Windows 6.3; UI render: default; 
Locale: nl-NL (nl_NL); Calc: CL


User-Agent: Mozilla/5.0 (Windows NT 6.3; WOW64; rv:52.0) Gecko/20100101 Firefox/52.0
Comment 1 Timur 2018-05-28 15:11:56 UTC
Created attachment 142347 [details]
Dump from WinDbg

sclo!ScPatternAttr::operator==+21
Comment 2 Xisco Faulí 2018-06-05 22:55:10 UTC
Regression introduced by:

author	Mike Kaganski <mike.kaganski@collabora.com>	2017-12-01 15:25:56 +0300
committer	Eike Rathke <erack@redhat.com>	2017-12-11 23:29:26 +0100
commit	3f614f431475e1bf3bb3bbeac59b0681309628b7 (patch)
tree	104fc56d968dd51802b658d97775c02e06424524
parent	7d202623b007979d9d0f93f6cd62c3c031d6d1d4 (diff)
tdf#95295: don't add duplicate conditional formats
This tries to make sure that on copy, any conditional formatting
that has same entries (conditions and formats) as another condition
in the table, is not appended to the conditions list, but updates
existing condition's range. This should prevent multiplication of
entries with duplicating conditions and formats, and fragmenting
ranges.

Existing unit tests had to be adjusted, because they had ensured
exactly that behaviour that is changed in this commit: that new
items are added to the list, and e.g. multiple elements are created
when a single cell is copied to a range.

Bisected with: bibisect-linux64-6.1

Adding Cc: to Mike Kaganski
Comment 3 Eike Rathke 2018-07-16 15:29:54 UTC
@Mike:
That sheet has 6440 conditional formats, mostly applied to single cells that look like quite a bunch could be merged (already on import?). However when copying the sheet we end up with an O(n^2) operation where each conditional format is tried through CheckAndDeduplicateCondFormat() and the existing ones removed by ScTable::RemoveCondFormatData(), likely in a sequence of

1. format for row 1, apply to row 1
2. format for row 2, identical to row 1
4. remove old from row 1
5. apply to row 1:2
6. format for row 3, identical to row 1:2
7. remove old from row 1:2
8. apply to row row 1:3
...

which results in super heavy operations on ScAttrArray.
This is even more than O(n^2), more like O(n^2*rangecolumns*rangerows), with ranges of where a conditional format is applied looped in ScTable::RemoveCondFormatData().
Comment 4 Xisco Faulí 2018-07-27 13:54:23 UTC
@Mike Kaganski, any change you could take a look at this bug after Eike's comments in comment 3 ?
Comment 5 Mike Kaganski 2018-07-27 22:12:27 UTC
(In reply to Xisco Faulí from comment #4)

:-) the https://gerrit.libreoffice.org/#/c/57725/ is there for some time already, waiting for review.
Comment 6 Xisco Faulí 2018-07-29 17:56:50 UTC
(In reply to Mike Kaganski from comment #5)
> (In reply to Xisco Faulí from comment #4)
> 
> :-) the https://gerrit.libreoffice.org/#/c/57725/ is there for some time
> already, waiting for review.

oh, nice, I didn't know... putting to assign then...
Comment 7 Commit Notification 2018-08-31 11:54:29 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=e56eb9cb7ee80215c05d06f25349d7ab7ad06640

tdf#117781: don't remove already applied conditional format data

It will be available in 6.2.0.

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

Affected users are encouraged to test the fix and report feedback.
Comment 8 Commit Notification 2018-08-31 15:48:32 UTC
Mike Kaganski committed a patch related to this issue.
It has been pushed to "libreoffice-6-1":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=2fffe4a50b6f541c1e43cf13a3a475caf94d203b&h=libreoffice-6-1

tdf#117781: don't remove already applied conditional format data

It will be available in 6.1.2.

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

Affected users are encouraged to test the fix and report feedback.
Comment 9 Xisco Faulí 2018-09-03 17:59:23 UTC
Verified in

Version: 6.2.0.0.alpha0+
Build ID: 4b5fcd417587cfb9e6d8b61ecb037ab165eeb5b9
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); Calc: threaded

@Mike Kaganski, Thanks for fixing this!!