Bug 421469 - [Regression] Slow search in the contents pane
Summary: [Regression] Slow search in the contents pane
Status: RESOLVED FIXED
Alias: None
Product: okular
Classification: Applications
Component: general (show other bugs)
Version: 20.04.0
Platform: Other Linux
: NOR normal
Target Milestone: ---
Assignee: Okular developers
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2020-05-13 15:19 UTC by Frank Mehnert
Modified: 2020-05-24 15:39 UTC (History)
4 users (show)

See Also:
Latest Commit:
Version Fixed In: 1.10.2
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Frank Mehnert 2020-05-13 15:19:01 UTC
SUMMARY
Searching in the Contents is very slow. This is a regression: 17.12.2-2.2 (Debian/Buster) is reasonable fast while version 20.04.0-1  (Debian/Sid) is much slower. 

STEPS TO REPRODUCE
1. Download https://static.docs.arm.com/ddi0487/fb/DDI0487F_b_armv8_arm.pdf
2. Open the file and go to the 'Contents' search bar.
3. Enter 'esr'

OBSERVED RESULT
It takes more than 20 seconds until the few search results are presented. During that time the application is more or less not responsible.

EXPECTED RESULT
With the old version from Buster the same search takes less than 3 seconds. It's possible to put search strings there character by character and the incremental search function is reasonable fast. That's not possible with the current 20.04.0 version.

SOFTWARE/OS VERSIONS
Windows: 
macOS: 
Linux/KDE Plasma: Debian/Sid, 5.17.5, Linux 5.6.0
(available in About System)
KDE Plasma Version: 5.17.5
KDE Frameworks Version: 5.62.0
Qt Version: 5.12.5

ADDITIONAL INFORMATION
My host is reasonable fast (Skylake i7-7700 HQ, T470p, 24G RAM, SSD)
Comment 1 Martin Steigerwald 2020-05-13 17:07:14 UTC
Thank you for your report. I can confirm that search is really slow with that document on Debian Sid with Okular 20.04.0-1 using a Sandybridge based laptop (ThinkPad T520).
Comment 2 Albert Astals Cid 2020-05-13 20:10:52 UTC
Are you convinced it's a regression or is it just that this file is particularly slow?

I.e. are you sure this file was fast with the old Okular you were using?
Comment 3 Frank Mehnert 2020-05-13 20:19:58 UTC
100% sure that this was fast with the old okular from Debian/Buster (okular version 1.3.2). The difference is easy to recognize.
Comment 4 Albert Astals Cid 2020-05-13 21:44:59 UTC
ok, need someone to have a look
Comment 5 Albert Astals Cid 2020-05-13 21:49:06 UTC
Ok, recent regression, works fine on release/19.12, that's "good"
Comment 6 Albert Astals Cid 2020-05-13 22:13:21 UTC
Ahmad it seems this regresion was introduced by you in commit 4a4456abd794d05074dd13aec10ffe5990b2b47d

    Port QRegExp to QRegularExpression in a couple of locations
    
    Some instances of QRegExp are still left:
    generators/mobipocket and generators/epub


Before it i can search "esr" in the contents pane and it takes like 2 seconds, after it takes like 22 seconds.

Could you please try to fix this?
Comment 7 Ahmad Samir 2020-05-13 23:34:01 UTC
Indeed it is slower... reverting the changes in KTreeViewSearchLine::itemMatches() restores the old behaviour. I am looking at it, so far I don't see anything obvious, other than that QRegExp::FixedString has some kind of optimisation that is much faster than what can be achieved by using QRegularExpression::escape() on the pattern (QRegExp docs say that using FixedString is equivalent to searching by using a pattern escaped by using QRegExp::escape()).

I could just use plain string comparison when the "regular expression" search option is disabled; string comparison is always faster than regular expression; that doesn't fix the issue for "regular expression"... I'll test some more (but it'll have to wait until tomorrow, it's getting late and I am almost asleep...).

Sorry about the trouble :)
Comment 8 Ahmad Samir 2020-05-14 19:22:12 UTC
It turns out that the issue was in how itemMatches() was being called; it was being called too many times.

A proposed fix: https://invent.kde.org/kde/okular/-/merge_requests/166
Comment 9 Frank Mehnert 2020-05-14 20:37:51 UTC
Fix works for me.
Comment 10 Albert Astals Cid 2020-05-23 10:11:51 UTC
Git commit 793f1692db244621a0a603a2a7c404cc1757b9ab by Albert Astals Cid, on behalf of Ahmad Samir.
Committed on 23/05/2020 at 10:11.
Pushed by aacid into branch 'release/20.04'.

Fix filtering items in the contents treeview

When filtering the items in the contents panel, we start iterating over
them using rootIndex() to get all the children, don't do the same loop
for each and every item, one go is enough.

Also only use a regular expression if the regularExpression option is
enabled, otherwise just use string operations, the latter is always
faster. With this change, both regex and non-regex search are faster.

M  +13   -12   ui/ktreeviewsearchline.cpp

https://invent.kde.org/graphics/okular/commit/793f1692db244621a0a603a2a7c404cc1757b9ab