Bug 87101 - EDITING 100x performance regression inserting rows/columns into sheet
Summary: EDITING 100x performance regression inserting rows/columns into sheet
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
4.3.4.1 release
Hardware: All All
: medium major
Assignee: Eike Rathke
URL:
Whiteboard: target:5.3.0
Keywords: bibisected, bisected, perf, regression
Depends on:
Blocks:
 
Reported: 2014-12-08 14:25 UTC by rlk
Modified: 2016-09-28 11:52 UTC (History)
7 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description rlk 2014-12-08 14:25:35 UTC
Reference https://drive.google.com/file/d/0B-4rl96vSQGGenhwd0wzdFpOMFk/view?usp=sharing

Inserting rows into the attached spreadsheet suffers a critical performance regression going from 4.2.7 to 4.3.4.  To reproduce:

1) Go to sheet "T Rank"

2) Select rows 420-519

3) Insert Rows Above (note that 4.2.7 and 4.3.4 are subject to bug 77592).

On my laptop (Core i7-920XM, 16 GB RAM), I measured the following CPU times (and memory consumption) for opening the file and inserting rows as stated.  Wall clock time is subjectively very close to CPU time and the system shows one core is 100% occupied.  Very little else was taking place, so hyperthreading or turbo mode is unlikely to be responsible for any significant difference.  The VSZ and RSS columns are the memory consumption following the load and insert operations; the VSZ (close) column is the virtual size after closing the spreadsheet.

Inserting columns shows similar behavior, but I didn't measure it.  I'm less certain about removing sheets, but that also seems much slower in 4.3.4.

This is in all cases using the freedesktop RPM's, not the distro RPMs (for openSUSE 13.2).

4.2.7 is a nice improvement over 4.1.5.  4.3.4 is unusable.

Version		Load		Insert Rows	VSZ	RSS	VSZ (close)
4.1.5		1:33		 0:33  		9659796	8200896 11195680
4.2.7		2:00		 0:14		2158056  681396  2147184
4.3.4		2:07		23:15		2818208 1317840  3962356
Comment 1 Jean-Baptiste Faure 2014-12-08 17:36:29 UTC
In version 4.4.0.0.beta2+, the sheet "T Rank" looks empty ...

Best regards. JBF
Comment 2 rlk 2014-12-08 17:41:52 UTC
That happened to me too.  I believe it's a bug in 4.4, but I haven't had a chance to file it yet.  It works up through at least 4.3.4.1.
Comment 3 rlk 2014-12-08 18:02:23 UTC
It's a rather complex spreadsheet that makes heavy use of names as unparameterized macros.  There's probably something squirrely in 4.4 right now that's breaking it, but that's obviously a separate issue.
Comment 4 raal 2014-12-08 20:00:35 UTC
I can see data with LO 4.3.3, but doesn't see data on the sheet Trank with version Version: 4.5.0.0.alpha0+
Build ID: a26222f5af55e8ffe9784dd411485d3c5c72e983
TinderBox: Linux-rpm_deb-x86_64@46-TDF, Branch:master, Time: 2014-12-07_01:55:26
I can see data with Version: 4.4.0.0.alpha0+
Build ID: dea4a3b9d7182700abeb4dc756a24a9e8dea8474

No data with Version: 4.3.6.0.0+
Build ID: 9ac54387bcee173e0af448d71b8890e5e255285c
TinderBox: Linux-rpm_deb-x86_64@46-TDF, Branch:libreoffice-4-3, Time: 2014-12-06_04:06:54

 -->  Fill new bug for this.


with Version: 4.4.0.0.alpha0+
Build ID: dea4a3b9d7182700abeb4dc756a24a9e8dea8474
Inserting rows -> few seconds

LO 4.3.3.2 ID build: 430m0(Build:2)
Inserting rows -> I killed LO after 2 minutes of 100%CPU
Comment 5 rlk 2014-12-08 20:27:33 UTC
Done, bug 87119.
Comment 6 Joel Madero 2014-12-12 06:27:14 UTC
I can confirm it's horribly slow on:

Ubuntu 14.04 x64
LibreOffice 4.5 master built a few days ago
LibreOffice 4.3.4.1 release

What I can't confirm is that it's a regression as I don't have an older version installed right now and this file crashes my system while trying to bibisect.

A bibisect would be fantastic

New
Major - can't open a particular file
Medium - seems appropriate, it's an incredibly complex file, seems like this is a corner case issue given the vast improvements in calc over the last few releases.
Comment 7 Matthew Francis 2014-12-13 12:39:13 UTC
Bibisect results from 44:
 daf852f47d4f0d3e531138611c2d35d64a90460a is the first bad commit
commit daf852f47d4f0d3e531138611c2d35d64a90460a
Author: Bjoern Michaelsen <bjoern.michaelsen@canonical.com>
Date:   Sun Oct 19 01:33:23 2014 +0000

    source-hash-fe9f8144c7a1a5e51544b4dacb8a2d7e70b321ee
    
    commit fe9f8144c7a1a5e51544b4dacb8a2d7e70b321ee
    Author:     Markus Mohrhard <markus.mohrhard@googlemail.com>
    AuthorDate: Mon Aug 25 16:11:54 2014 +0200
    Commit:     Markus Mohrhard <markus.mohrhard@googlemail.com>
    CommitDate: Tue Aug 26 13:34:03 2014 +0200
    
        move method documentation to the header file
    
        Change-Id: I7d4f77c50a8b6b2b0d7c0868c73b0cb13f952421


# bad: [6a4073277f529f28970e2b7d8dbd4a6b3b844773] source-hash-d6a83d3f91336e23b51bfc3b3d58da799760829f
# good: [812c4a492375ac47b3557fbb32f5637fc89d60d9] source-hash-dea4a3b9d7182700abeb4dc756a24a9e8dea8474
git bisect start '6a40732' '812c4a4'
# bad: [acb494028b446679c6f90400e84cb14b15474a50] source-hash-f6001fb30c516ec6da39b0ca4058f8b28058df1f
git bisect bad acb494028b446679c6f90400e84cb14b15474a50
# good: [89c2417b2972072396b05e705a2cadec46c269b1] source-hash-5ab1098d5fbc1ba092da73af2b92e5bcdd7a3f8d
git bisect good 89c2417b2972072396b05e705a2cadec46c269b1
# bad: [6830e03f961e3dcedc959cfc382fb19050a529b7] source-hash-7d5b30b419fb49232f3476d9fb8899fe3754d96d
git bisect bad 6830e03f961e3dcedc959cfc382fb19050a529b7
# bad: [daf852f47d4f0d3e531138611c2d35d64a90460a] source-hash-fe9f8144c7a1a5e51544b4dacb8a2d7e70b321ee
git bisect bad daf852f47d4f0d3e531138611c2d35d64a90460a
# good: [9794d750c0eeb345369b0a6c1c7a26e82b438d06] source-hash-d4a8fa7db0ed4faae00408fbda2352379774cfc0
git bisect good 9794d750c0eeb345369b0a6c1c7a26e82b438d06
# good: [871807e2ae6637a19f7c96e16dd6c0a563eae5b6] source-hash-a6c5f2ba6bca8ad95a3731e2770a1d216c9925a0
git bisect good 871807e2ae6637a19f7c96e16dd6c0a563eae5b6
# good: [f817802ec63276a72a7711648ce471ca7ff7e45a] source-hash-bf640ba048704220292411e4f2bcc0d3c62caa32
git bisect good f817802ec63276a72a7711648ce471ca7ff7e45a
# good: [3e965dd5a8232692695a640531e51d25aa5fa06b] source-hash-27d79b12e17a93f6e4eab2a3c09d423a4a04a63d
git bisect good 3e965dd5a8232692695a640531e51d25aa5fa06b
# first bad commit: [daf852f47d4f0d3e531138611c2d35d64a90460a] source-hash-fe9f8144c7a1a5e51544b4dacb8a2d7e70b321ee
Comment 8 Matthew Francis 2014-12-13 16:25:35 UTC
A follow-up source bisect suggests the following commit:

commit 0792aef9010007d5738723d8930990028bef2f9e
Author: Eike Rathke <erack@redhat.com>
Date:   Mon Aug 25 21:54:42 2014 +0200

    fdo#83067 also volatile cells need to listen to all references
    
    As we now broadcast also cell moves it is not sufficient anymore to add
    volatile cells only to the BCA_LISTEN_ALWAYS broadcaster, add them as
    listener to all referenced cells and ranges as usual.
    
    Change-Id: I7901b73db7e0c82c4bac302ae746810cbc16ea44
Comment 9 rlk 2014-12-13 17:26:42 UTC
I tried eliminating all calls to TODAY().  Didn't help.
Comment 10 Luke 2014-12-16 05:31:03 UTC
Eike,
Commit 0792aef9010007d5738723d8930990028bef2f9e may be responsible for this performance regression. Could you please look into this?
Comment 11 rlk 2015-01-01 18:16:04 UTC
I did find one more call to TODAY(), which I eliminated.  I also checked that there are no calls to INFO(), RAND(), or NOW() by inspecting the XML files in the .ods container.  Didn't appear to help.  This one's basically a blocker for me to move beyond 4.2.7.
Comment 12 rlk 2015-01-01 18:25:03 UTC
I also tried turning off auto-recalculation; that too did not help.

It appears from https://superuser.com/questions/852763/how-to-nail-libreoffice-references-in-place that OFFSET() is a volatile function.  That's one I have no hope of eliminating.

(BTW, I notice that references inside names are NOT adjusted when rows/columns are added/removed.  That's another one I have to either find or file.)
Comment 13 Robinson Tryon (qubit) 2015-12-13 11:11:08 UTC Comment hidden (obsolete)
Comment 14 Commit Notification 2016-09-22 08:07:34 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

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

tdf#87101 Revert "fdo#83067 also volatile cells need to listen to all refe"...

It will be available in 5.3.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 15 Commit Notification 2016-09-22 08:07:52 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

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

sc-perf: tdf#87101 add bulk scope for BroadcastRecalcOnRefMove() calls

It will be available in 5.3.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 16 Commit Notification 2016-09-23 15:16:47 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

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

perf: eliminate SfxSimpleHint and move to SfxHint, tdf#87101 related

It will be available in 5.3.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 17 Xisco Faulí 2016-09-26 15:53:29 UTC
Adding Cc: to Eike Rathke
Comment 18 Commit Notification 2016-09-27 18:50:18 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

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

sc-perf: avoid repeated TrackFormulas() during bulk broadcast, tdf#87101 rel.

It will be available in 5.3.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 19 Commit Notification 2016-09-28 11:30:39 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

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

sc-perf: avoid second call to ScAttrArray::Search(), tdf#87101 related

It will be available in 5.3.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.