Bug 477978 - List of reviews takes 20 seconds or more to open
Summary: List of reviews takes 20 seconds or more to open
Status: RESOLVED FIXED
Alias: None
Product: Discover
Classification: Applications
Component: discover (show other bugs)
Version: master
Platform: Neon Linux
: NOR normal
Target Milestone: ---
Assignee: Plasma Bugs List
URL:
Keywords: qt6
Depends on:
Blocks:
 
Reported: 2023-12-03 10:46 UTC by Patrick Silva
Modified: 2024-03-30 04:43 UTC (History)
5 users (show)

See Also:
Latest Commit:
Version Fixed In: 6.0.3


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Silva 2023-12-03 10:46:51 UTC
SUMMARY
Can reproduce on Arch Linux running Plasma 6 beta and neon unstable.

STEPS TO REPRODUCE
1. open Discover
2. click on VLC player
3. click on "Show all reviews" button

OBSERVED RESULT
the list of reviews takes 20 seconds or more to open

EXPECTED RESULT
the list of reviews opens immediately

SOFTWARE/OS VERSIONS
Operating System: KDE neon Unstable Edition
KDE Plasma Version: 5.81.80
KDE Frameworks Version: 5.245.0
Qt Version: 6.6.0
Graphics Platform: Wayland
Comment 1 Nate Graham 2023-12-05 19:37:47 UTC
Can reproduce with those steps. After I close it, the next time I open it, it opens immediately.
Comment 2 Harald Sitter 2024-03-20 13:38:51 UTC
Is this still reproducible on master? I 've tried wiping my ~/.cache/discover/ but it loads in about 2-3 seconds even after that.
Comment 3 Patrick Silva 2024-03-20 15:22:02 UTC
The bug persists on neon unstable.
Comment 4 Harald Sitter 2024-03-20 22:33:42 UTC
Hm. I boot up latest neon unstable iso, open discover, search for vlc, click on it, scroll down to view all comments, click that, loads in 2-3 seconds :\
Comment 5 Nate Graham 2024-03-21 15:21:40 UTC
I just reproduced it on my system... kind of. The OverlaySheet of reviews didn't take 20 seconds to open, but it take about 5.

Steps:
0. Have it be over 12 hours since you last opened Discover
1. Launch Discover
2. Immediately click on VLC on the home page
3. Scroll down to the reviews section, which is still loading
4. The moment the "Show all reviews" button appears, click it

At this point Discover hung for about 5 seconds. After that, the OverlaySheet opened.
Comment 6 Harald Sitter 2024-03-22 17:34:18 UTC
I have a theory on this. The delegate instantiation via cacheBuffer is way too aggressive and creates some 600+ delegates when all we want is two pages worth. This happens because the height/implicitHeight of the delegate is 0 initially. I am not quite sure what to do about it though. We could set height: Math.max(implicitHeight, someRandomMinimalFixedInt) but that seems iffy.  At the same time I think that implicitHeight should somehow be a more sane value. There are multiple controls involved each with their own padding and decorations, surely those should at least amount to some height that cacheBuffer can use, even if it is height without text 🤷‍♂️

The possibly least awkward solution is to simply set cacheBuffer to 0. With reuseItems:true the delegate recycling should be plenty fast.
Comment 7 Bug Janitor Service 2024-03-23 00:04:32 UTC
A possibly relevant merge request was started @ https://invent.kde.org/frameworks/kirigami/-/merge_requests/1491
Comment 8 Harald Sitter 2024-03-24 23:59:36 UTC
Git commit 75bf9ef12482eba9a7f496af3a10bf4f2f6f32eb by Harald Sitter.
Committed on 24/03/2024 at 23:56.
Pushed by sitter into branch 'master'.

padding: make sure to polish on completion

this is incredibly important as without polish run we'll not have an
implicit size and without that our parents will not know what to do with
us. this is doubly problematic when used as a listview delegate (such as
via AbstractCard) because the cacheBuffer determines how many delegates
to create based on the size of the first delegate right after creation.
if we have no size, the listview will create hundreds of delegates,
potentially needlessly.

M  +8    -0    src/padding.cpp
M  +2    -0    src/padding.h

https://invent.kde.org/frameworks/kirigami/-/commit/75bf9ef12482eba9a7f496af3a10bf4f2f6f32eb