Bug 158773 - FILEOPEN PPTX Slow loading of file with lots of unused master pages
Summary: FILEOPEN PPTX Slow loading of file with lots of unused master pages
Status: NEW
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Impress (show other bugs)
Version:
(earliest affected)
7.6.3.2 release
Hardware: All All
: medium normal
Assignee: Not Assigned
URL:
Whiteboard: target:24.8.0 target:24.2.3
Keywords: bibisected, bisected, filter:pptx, perf
Depends on:
Blocks: PPTX-MasterSlide
  Show dependency treegraph
 
Reported: 2023-12-19 11:34 UTC by Gabor Kelemen (allotropia)
Modified: 2024-06-05 22:09 UTC (History)
9 users (show)

See Also:
Crash report or crash signature:


Attachments
Flamegraph (394.85 KB, application/x-bzip2)
2024-03-05 19:40 UTC, Julien Nabet
Details
Flamegraph 2 (245.64 KB, application/x-bzip2)
2024-03-10 13:24 UTC, Julien Nabet
Details
Flamegraph 3 (229.63 KB, application/x-bzip2)
2024-03-14 09:06 UTC, Julien Nabet
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Gabor Kelemen (allotropia) 2023-12-19 11:34:55 UTC
Opening attachment 188496 [details] from bug 156400 in 7.6 took about 2-3 seconds on my virtual machine.
In current master it takes about 50 seconds.

1. Open  attachment 188496 [details] 
-> long load time

Version: 24.8.0.0.alpha0+ (X86_64) / LibreOffice Community
Build ID: bcbc0857bf4bc24b5ea36e445a367cce0a382da4
CPU threads: 15; OS: Windows 10.0 Build 19045; UI render: default; VCL: win
Locale: hu-HU (hu_HU); UI: en-US
Calc: threaded

Seems to have been like this since:

https://git.libreoffice.org/core/+/f2ae8b934aaac7c444e8493ed5e8189c6ce63328

author	Henry Castro <hcastro@collabora.com>	Mon Oct 09 07:34:02 2023 -0400
committer	Henry Castro <hcastro@collabora.com>	Tue Oct 31 21:09:19 2023 +0100

tdf#155512: oox: ppt: fix import master slides, follow up

Before this commit load time was some 10-12 seconds, so not the only culprit there.
Comment 1 Gabor Kelemen (allotropia) 2023-12-19 11:47:45 UTC
First change from 2 seconds to 12 was:

https://git.libreoffice.org/core/+/adcde78935fb8ca2b93322aa3a558d0b3ccdbfad

author	Henry Castro <hcastro@collabora.com>	Thu Sep 28 15:01:43 2023 -0400
committer	Henry Castro <hcastro@collabora.com>	Wed Oct 04 21:07:14 2023 +0200

tdf#155512: oox: ppt: fix import master slides

Adding CC to: Henry Castro
Comment 2 Stéphane Guillou (stragu) 2024-01-02 16:38:19 UTC
Slow in:

Version: 7.6.4.1 (X86_64) / LibreOffice Community
Build ID: e19e193f88cd6c0525a17fb7a176ed8e6a3e2aa1
CPU threads: 8; OS: Linux 5.15; UI render: default; VCL: gtk3
Locale: en-AU (en_AU.UTF-8); UI: en-US
Calc: threaded

Near-instant in:

Version: 7.5.9.2 (X86_64) / LibreOffice Community
Build ID: cdeefe45c17511d326101eed8008ac4092f278a9
CPU threads: 8; OS: Linux 5.15; UI render: default; VCL: gtk3
Locale: en-AU (en_AU.UTF-8); UI: en-US
Calc: threaded
Comment 3 Justin L 2024-01-10 14:29:43 UTC
I saw the same problem in bug 137691 comment 0's attachment 166646 [details] (chartlegendmissing.pptx). In bug 137691 comment 10's attachment 191778 [details] (chartlegendmissing-min.pptx) I minimized by removing the unused master/layout pages which immensely improved the load time.

I really noticed it in my development build with debugging turned on.
Comment 4 Timur 2024-02-22 17:00:48 UTC
Prior to these changes, master slides were not imported and they are many in this file. Now they load and LO is slow to load with them.
I remove regression here. 
OTOH, MSO loads that fast, and for some PPTX with many unused slides it even asks if a cleanup should be done.
Comment 5 Xisco Faulí 2024-03-05 15:44:40 UTC
Hi Julien,
Would it be possible to have a perf graph for this issue ?
Comment 6 Julien Nabet 2024-03-05 19:40:18 UTC
Created attachment 192982 [details]
Flamegraph

Flamegraph retrieved on pc Debian x86-64 with master sources updated today.
Comment 7 Commit Notification 2024-03-06 12:01:15 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/6f8073caf0d6b331232f6edb5f18d14ddefdb465

tdf#158773 reduce cost of ContentInfo::GetText

It will be available in 24.8.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 8 Commit Notification 2024-03-06 18:28:53 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

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

tdf#158773 reduce dynamic_cast'ing in CustomShapeProperties::Notify

It will be available in 24.8.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 9 Commit Notification 2024-03-06 18:58:58 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/3b784236d7c3bf386deeeadcf79d9e9b289bf991

tdf#158773 reduce cost of TextProperties::Notify

It will be available in 24.8.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 Commit Notification 2024-03-07 05:52:58 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/9c5fda14fff397d5d503f749ad019791d7e4ef83

tdf#158773 reduce dynamic_cast'ing in CustomShapeProperties::Notify

It will be available in 24.8.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 11 Xisco Faulí 2024-03-07 10:43:57 UTC
Using 'time OOO_EXIT_POST_STARTUP=1 instdir/program/soffice /home/xisco/Descargas/rogueimage.pptx'

it takes

real	0m37,018s
user	0m36,065s
sys	0m0,927s

in LibreOfficeDev 24.8.0.0.alpha0 e66f5cb353bc0ea1020061bab4981380f98499b9

while it takes

real	0m18,879s
user	0m18,051s
sys	0m0,952s

in LibreOfficeDev 24.8.0.0.alpha0 825dde03999a55d02e4d5bc88a4d5beacb65e67f
Comment 12 Commit Notification 2024-03-07 15:50:28 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/06d444e9a102569aa6cf429079036fc95482cc7f

tdf#158773 reduce dynamic_cast'ing in TextProperties::Notify

It will be available in 24.8.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 13 Commit Notification 2024-03-07 15:50:31 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/34017b14d866e99aff466ff200b60b3e2d509a30

tdf#158773 reduce time spent in IndexedStyleSheets::Reindex

It will be available in 24.8.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 Commit Notification 2024-03-07 15:50:33 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/4a30c919373940025024442d9607da06258bb552

tdf#158773 reduce dynamic_cast in AttributeProperties::Notify

It will be available in 24.8.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 15 Commit Notification 2024-03-08 12:36:53 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/1406e7b2f01a88377649c763cd65d20803c3c3a6

tdf#158773 flatten data of IndexedStyleSheets

It will be available in 24.8.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 16 Commit Notification 2024-03-08 16:57:24 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/5cd557a512551ae5033957e22ea5c5f285518be3

tdf#158773 GetNumberOfStyleSheets can be an inline method

It will be available in 24.8.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 17 Commit Notification 2024-03-08 16:57:27 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/02f88f45e681beacf611f7c20f7903916b7e7374

tdf#158773 reduce cost of stylesheet lookup

It will be available in 24.8.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 18 Commit Notification 2024-03-08 16:57:29 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/6441778b61d1c6737947551e55d2c27e79260142

tdf#158773 reduce size of IndexedStyleSheets

It will be available in 24.8.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 19 Commit Notification 2024-03-08 20:39:03 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/6a3b20b14d32922c99df4ad65271499d0a8d663e

tdf#158773 do the cheap checks first

It will be available in 24.8.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 20 Commit Notification 2024-03-09 06:09:54 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/632fc28fda03e312b2eb2d5843459b3ceee43534

tdf#158773 add a set to sd::ShapeList

It will be available in 24.8.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 Commit Notification 2024-03-09 17:42:03 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/959b30841ae9c5cc1f43928f5e7780abaab4a087

tdf#158773 avoid some OUString construction

It will be available in 24.8.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 22 Commit Notification 2024-03-10 09:12:22 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/5f62629af6d12d505b029db85c276daedf5017b1

tdf#158773 avoid Reindex() until we are done renaming

It will be available in 24.8.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 23 Noel Grandin 2024-03-10 10:33:26 UTC
I'm done with this for now. I don't see any paths to further progress, but someone else might have more luck,
Comment 24 Julien Nabet 2024-03-10 13:24:47 UTC
Created attachment 193044 [details]
Flamegraph 2
Comment 25 Julien Nabet 2024-03-10 13:26:21 UTC
Thank you Noel for all the optimizations.
I read your last comment about the fact you saw no other paths to optimize, but, just in case, I've submitted a new Flamegraph.
Thank you again! :-)
Comment 26 Commit Notification 2024-03-11 05:32:21 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

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

tdf#158773 reduce cost of importing binary data

It will be available in 24.8.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 27 Timur 2024-03-13 12:41:46 UTC
All this improved load time from 18 to 8 seconds for me. Meaning that improvemnt is very tangible. I suggest this be closed as Fixed.
Comment 28 Gerald Pfeifer 2024-03-13 14:13:47 UTC
That's been a great job, chiselling at it piece by piece, Noel - kudos
and thank you!

Some timings (user, not system) on a 13th Gen Intel(R) Core(TM) i5-1345U:

  20240301:  19,008s
  20240307:  13,427s
  20240308:  11,790s
  20240309:  7,767s
  20240310:  7,568s
  20240311:  7,087s
  20240312:  7,127s

Once nice "side effect" of this - which you (or we, as in LibreOffice)
should promote - is that this also helps simpler documents. It is an
improvement across the board!
Comment 29 Julien Nabet 2024-03-14 09:06:38 UTC
Created attachment 193108 [details]
Flamegraph 3

Here's an updated Flamegraph retrieved on pc Debian x86-64 with master sources updated today.
Just in case if someone has some idea to improve perf even more.

Of course, perfs already improved greatly, thank you Noel! :-)
Comment 30 Gerald Pfeifer 2024-03-18 08:23:30 UTC
(In reply to Timur from comment #27)
> All this improved load time from 18 to 8 seconds for me. Meaning that
> improvemnt is very tangible. I suggest this be closed as Fixed.

The improvement is great, and I am very grateful to Noel and his
meticulous persistence.

As for closing this issue - isn't it still to flow from a user 
perspective?
Comment 31 Xisco Faulí 2024-03-21 09:23:56 UTC
I backported 3b784236d7c3bf386deeeadcf79d9e9b289bf991 to libreoffice-24-2 -> https://gerrit.libreoffice.org/c/core/+/165112 which for me reduces the import time from 30 seconds to  22 seconds. I'm hesitant to backport all the commits...
Comment 32 Commit Notification 2024-03-21 20:22:44 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "libreoffice-24-2":

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

tdf#158773 reduce cost of TextProperties::Notify

It will be available in 24.2.3.

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 33 Gerald Pfeifer 2024-03-24 20:01:16 UTC
(In reply to Gerald Pfeifer from comment #30)
> As for closing this issue - isn't it still to flow from a user 
> perspective?

...too slow... <oops>
Comment 34 Timur 2024-03-25 10:31:57 UTC
I do not think it is realistic to expect more speed up here. Not it is useful to keep this open for a long time. 
Should someone want to speed up more, ticket remains here for a reference. IMO way to go is warning to remove excessive unused slides, bug 159860.
Comment 35 Commit Notification 2024-04-04 11:41:29 UTC
Noel Grandin committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/41581fe7d782f261c19594c89875666f99c91af1

fix 'tdf#158773 reduce dynamic_cast'ing in CustomShapeProperties::Notify'

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