Bug 126663 - XLSX: LibreOffice is freezed when I try open Style list in Sidebar
Summary: XLSX: LibreOffice is freezed when I try open Style list in Sidebar
Status: VERIFIED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:6.4.0 target:6.3.1 target:6.3.2
Keywords: perf
: 118523 (view as bug list)
Depends on:
Blocks: Sidebar-Styles
  Show dependency treegraph
 
Reported: 2019-08-01 14:51 UTC by Roman Kuznetsov
Modified: 2019-08-19 07:37 UTC (History)
7 users (show)

See Also:
Crash report or crash signature:


Attachments
Sample XLSX file (968.76 KB, application/vnd.openxmlformats-officedocument.spreadsheetml.sheet)
2019-08-01 14:52 UTC, Roman Kuznetsov
Details
Flamegraph (1.74 MB, image/svg+xml)
2019-08-04 16:00 UTC, Julien Nabet
Details
perf flamegraph (2.55 MB, image/svg+xml)
2019-08-06 21:14 UTC, Julien Nabet
Details
perf flamegraph (2.21 MB, image/svg+xml)
2019-08-10 07:22 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Roman Kuznetsov 2019-08-01 14:51:57 UTC
Description:
XLSX: LibreOffice is freezed when I try open Style list in Sidebar

Steps to Reproduce:
1. Open XLSX file from attach
2. Try open Style section in Sidebar
3. LO is freeze but after 30-40 min it shows style list

Actual Results:
LibreOffice is freezed when I try open Style list in Sidebar

Expected Results:
LIbreOffice shows Style list at once


Reproducible: Always


User Profile Reset: No



Additional Info:
Version: 6.4.0.0.alpha0+ (x86)
Build ID: ca6df519a78e5bfc96030c916f242b86306194e5
CPU threads: 4; OS: Windows 6.1; UI render: default; VCL: win; 
Locale: ru-RU (ru_RU); UI-Language: en-US
Calc: threaded

the same problem in 6.2.4
Comment 1 Roman Kuznetsov 2019-08-01 14:52:30 UTC
Created attachment 153093 [details]
Sample XLSX file
Comment 2 m.a.riosv 2019-08-02 19:54:21 UTC
Take about 20 seconds for me with
Version: 6.2.5.2 (x64)
Build ID: 1ec314fa52f458adc18c4f025c545a4e8b22c159
CPU threads: 4; OS: Windows 10.0; UI render: GL; VCL: win; 
Locale: es-ES (es_ES); UI-Language: en-US
Calc: CL

There are a thousands of styles.
Comment 3 Joel Madero 2019-08-03 16:20:59 UTC
@Roman - can you test an older version of LibreOffice to see if this is a regression or not? https://downloadarchive.documentfoundation.org/libreoffice/old/

It helps to pinpoint if it's a new issue or if it's just how it was build (maybe faulty). Regressions are easier to diagnose and fix than things that were implemented maybe poorly from the beginning.

Please test if possible. In particular 5.0 and 4.0.
Comment 4 Julien Nabet 2019-08-04 16:00:01 UTC
Created attachment 153129 [details]
Flamegraph

Here is a Flamgraph made from a pc with Debian x86-64 with master sources updated today.
Comment 5 Julien Nabet 2019-08-04 18:12:08 UTC
This part seems fishy:
https://opengrok.libreoffice.org/xref/core/sfx2/source/dialog/templdlg.cxx?r=45839115#1185
1185      while( pStyle )
1186      {
1187          //Bubblesort
1188          size_t nPos;
1189          for(nPos = aStrings.size(); nPos && aSorter.compare(aStrings[nPos-1], pStyle->GetName()) > 0; --nPos)
1190          {};
1191          aStrings.insert(aStrings.begin() + nPos, pStyle->GetName());
1192          pStyle = pStyleSheetPool->Next();
1193      }

Wouldn't it be better to push_back every pStyle names in aStrings with the loop, then sort once afterwards with std::sort ?

Eg:
diff --git a/sfx2/source/dialog/templdlg.cxx b/sfx2/source/dialog/templdlg.cxx
index 8cc0240f5f6f..c9e654afed45 100644
--- a/sfx2/source/dialog/templdlg.cxx
+++ b/sfx2/source/dialog/templdlg.cxx
@@ -1184,13 +1184,13 @@ void SfxCommonTemplateDialog_Impl::UpdateStyles_Impl(StyleFlags nFlags)
 
     while( pStyle )
     {
-        //Bubblesort
-        size_t nPos;
-        for(nPos = aStrings.size(); nPos && aSorter.compare(aStrings[nPos-1], pStyle->GetName()) > 0; --nPos)
-        {};
-        aStrings.insert(aStrings.begin() + nPos, pStyle->GetName());
+        aStrings.push_back(pStyle->GetName());
         pStyle = pStyleSheetPool->Next();
     }
+    std::sort(aStrings.begin(), aStrings.end(),
+       [&aSorter](const OUString& rLHS, const OUString& rRHS) {
+       return aSorter.compare(rLHS, rRHS) < 0;
+       });
Comment 6 Julien Nabet 2019-08-04 18:13:05 UTC
Noel: thought you might be interested in this one since it concerns perf.
I proposed a small patch in my previous comment.
Comment 7 Xisco Faulí 2019-08-05 08:00:18 UTC
With a clean profile, it takes 145 seconds in

Version: 6.4.0.0.alpha0+
Build ID: 620fff54ca9cd04459cc5d963ef94d4438129fe4
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); UI-Language: en-US
Calc: threaded
Comment 8 Xisco Faulí 2019-08-05 08:06:34 UTC
@Julien, you patch reduces the time to 15 seconds for me. Could you please submit a patch to gerrit so it can be easily review by others ?
Comment 9 Julien Nabet 2019-08-05 08:10:30 UTC
(In reply to Xisco Faulí from comment #8)
> @Julien, you patch reduces the time to 15 seconds for me. Could you please
> submit a patch to gerrit so it can be easily review by others ?

I'll be able to do this only when get back home after my day time job.
If you don't want to wait, don't hesitate to submit it on gerrit.
Comment 10 Xisco Faulí 2019-08-05 15:08:08 UTC
(In reply to Julien Nabet from comment #9)
> (In reply to Xisco Faulí from comment #8)
> > @Julien, you patch reduces the time to 15 seconds for me. Could you please
> > submit a patch to gerrit so it can be easily review by others ?
> 
> I'll be able to do this only when get back home after my day time job.
> If you don't want to wait, don't hesitate to submit it on gerrit.

This issue has been around for quite a while, we can wait a few more hours ;-)
I'm wondering if this code could be optimized as well? -> https://opengrok.libreoffice.org/xref/core/svx/source/tbxctrls/tbcontrl.cxx?r=06daea73#3018
Comment 11 Xisco Faulí 2019-08-05 15:16:17 UTC
Also reproduced in

Version: 4.5.0.0.alpha0+
Build ID: 2851ce5afd0f37764cbbc2c2a9a63c7adc844311
Locale: ca_ES
Comment 12 Xisco Faulí 2019-08-05 19:27:47 UTC
the initial code was introduced in https://cgit.freedesktop.org/libreoffice/core/commit/?id=fd069bee7e57ad529c3c0974559fd2d84ec3151a back in 2000
Another bottleneck killed...
Comment 13 Commit Notification 2019-08-05 19:29:14 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/86ab2b2fcd95c6d14d0fd91dd1adaee5e7fd5713%5E%21

tdf#126663: optimize style list display in sidebar

It will be available in 6.4.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 14 Xisco Faulí 2019-08-05 19:40:10 UTC
*** Bug 118523 has been marked as a duplicate of this bug. ***
Comment 15 Commit Notification 2019-08-06 08:22:54 UTC
Julien Nabet committed a patch related to this issue.
It has been pushed to "libreoffice-6-3":

https://git.libreoffice.org/core/+/0702541b3d4661846dccde26efa9c2fb5ed65f35%5E%21

tdf#126663: optimize style list display in sidebar

It will be available in 6.3.1.

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 16 Xisco Faulí 2019-08-06 09:03:48 UTC
So these are my results using the bisect repositories ( which seems much slower than my own build )

it takes 1 minute and 15 seconds in

Version: 6.4.0.0.alpha0+
Build ID: 967644f09b8b7abe3b86d1647820f14e0196f8b4
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); UI-Language: en-US
Calc: threaded

( 55 seconds with gen instead of gtk3 )

and 3 minutes and 20 seconds in

Version: 6.3.1.0.0+
Build ID: 8b81a453b22611f25674f5e44ae411d78c2fcada
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); UI-Language: en-US
Calc: threaded

@Noel, @Julien, do you know if it's possible to optimize it further ?

@Roman, @Gabor, what time do you get ?
Comment 17 Julien Nabet 2019-08-06 09:41:27 UTC
Xisco: I can do another Flamegraph (which includes the commit) after my daytime job.
Comment 18 Jean-Baptiste Faure 2019-08-06 10:51:20 UTC
Less than 10 seconds to open Styles list in the sidebar with

Version: 6.3.1.0.0+
Build ID: 0702541b3d4661846dccde26efa9c2fb5ed65f35
Threads CPU : 4; OS : Linux 4.15; UI Render : par défaut; VCL: gtk3; 
Ubuntu_18.04_x86-64
Locale : fr-FR (fr_FR.UTF-8); UI-Language: fr-FR
Calc: threaded

built at home under Ubuntu 18.04 x86-64

Best regards. JBF
Comment 19 Julien Nabet 2019-08-06 21:14:08 UTC
Created attachment 153182 [details]
perf flamegraph

Here's a new Flamegraph perf with the quoted commit included.
Comment 20 Commit Notification 2019-08-07 18:14:05 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/66661417ff019831cbe7e647be2df1a4328ec2e6%5E%21

tdf#126663 speed up styles display

It will be available in 6.4.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 21 Xisco Faulí 2019-08-07 19:20:11 UTC
it takes 12 seconds for me now

Version: 6.4.0.0.alpha0+
Build ID: 6d024a69164716f7856ec936a72d01a6630d2a7c
CPU threads: 4; OS: Linux 4.15; UI render: default; VCL: gtk3; 
Locale: ca-ES (ca_ES.UTF-8); UI-Language: en-US
Calc: threaded

@Julien, Noel, thanks for fixing this issue
Comment 22 Commit Notification 2019-08-07 20:14:12 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "libreoffice-6-3":

https://git.libreoffice.org/core/+/d2c728ea3fe1c657db500dcd114069705b4759d1%5E%21

tdf#126663 speed up styles display

It will be available in 6.3.1.

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 23 Roman Kuznetsov 2019-08-08 05:21:33 UTC
it takes only 4 sec for me now in

Version: 6.4.0.0.alpha0+
Build ID: 6d024a69164716f7856ec936a72d01a6630d2a7c
CPU threads: 4; OS: Linux 5.2; UI render: default; VCL: gtk3; 
Locale: ru-RU (ru_RU.UTF-8); UI-Language: en-US
Calc: threaded

brilliant work, Julien and Noel! Thank you!

I'll check it on Windows when we'll be have actual windows daily build.
Comment 24 Julien Nabet 2019-08-10 07:22:40 UTC
Created attachment 153278 [details]
perf flamegraph

On pc Debian x86-64 with master sources updated today, I wanted to retrieve another new Flamegraph perf following Noel's commit.

Just for information, I use -F 200, should I use -F 120 or other?
Comment 25 Noel Grandin 2019-08-10 07:48:41 UTC
The -F value is a judgment call, I typically have anywhere from -F 1 to -F 200, depending on how fast the thing I'm measuring already is. What I aim to get is roughly > 100 samples covering the time when the interesting thing is happening, so I get a decent chance of catching all the functions involved.
Comment 26 Roman Kuznetsov 2019-08-11 11:15:15 UTC
Test in Comment 23 was made on 4 core CPU 2,5GHz on new notebook => 4 sec and XFCE+vcl:gtk3

on another notebook (7 old year) with only 2 core CPU it takes about 20 sec (KDE)

Version: 6.4.0.0.alpha0+
Build ID: 8387a6db641b29e6ff3c2f4cdc4688f538cbe35f
CPU threads: 4; OS: Linux 5.2; UI render: default; VCL: kf5; 
TinderBox: Linux-rpm_deb-x86_64@86-TDF, Branch:master, Time: 2019-08-09_06:28:42
Locale: ru-RU (ru_RU.UTF-8); UI-Language: en-US
Calc: threaded

why is there big difference between several machines?
Comment 27 Roman Kuznetsov 2019-08-12 20:03:07 UTC
(In reply to Roman Kuznetsov from comment #23)
> it takes only 4 sec for me now in
> 
> Version: 6.4.0.0.alpha0+
> Build ID: 6d024a69164716f7856ec936a72d01a6630d2a7c
> CPU threads: 4; OS: Linux 5.2; UI render: default; VCL: gtk3; 
> Locale: ru-RU (ru_RU.UTF-8); UI-Language: en-US
> Calc: threaded
> 
> brilliant work, Julien and Noel! Thank you!
> 
> I'll check it on Windows when we'll be have actual windows daily build.

it takes about 30 sec for me in

Version: 6.4.0.0.alpha0+ (x64)
Build ID: ec3a14ba93ba0be49170afa979f299bbf9e24300
CPU threads: 4; OS: Windows 10.0; UI render: default; VCL: win; 
Locale: ru-RU (ru_RU); UI-Language: en-US
Calc: threaded

Linux rulezzzz!!! But it's really strange...
Comment 28 Commit Notification 2019-08-16 12:39:57 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/+/9a03308fafee8f424eba399c1aae56af02beafb0%5E%21

tdf#126663 speed up styles display by sorting twice

It will be available in 6.4.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 29 Roman Kuznetsov 2019-08-18 17:35:32 UTC
(In reply to Roman Kuznetsov from comment #27)
> 
> it takes about 30 sec for me in
> 
> Version: 6.4.0.0.alpha0+ (x64)
> Build ID: ec3a14ba93ba0be49170afa979f299bbf9e24300
> CPU threads: 4; OS: Windows 10.0; UI render: default; VCL: win; 
> Locale: ru-RU (ru_RU); UI-Language: en-US
> Calc: threaded
> 
> Linux rulezzzz!!! But it's really strange...

it takes about 20 sec in

Версия: 6.4.0.0.alpha0+ (x64)
ID сборки: 3e64065612acec2eb29aa21e2b515953422256d7
Потоков ЦП: 4; ОС:Windows 10.0; Отрисовка ИП: GL; VCL: win; 
TinderBox: Win-x86_64@62-TDF, Branch:master, Time: 2019-08-15_22:57:26
Локаль: ru-RU (ru_RU); Язык интерфейса: ru-RU
Calc: threaded

without latest patch from Noel. Windows build looks more slow than linux build in this case
Comment 30 Commit Notification 2019-08-19 07:37:36 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "libreoffice-6-3":

https://git.libreoffice.org/core/+/0789b179cea1c5fca3effc95bec169eeb5fd87cd%5E%21

tdf#126663 speed up styles display by sorting twice

It will be available in 6.3.2.

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.