-
-
Notifications
You must be signed in to change notification settings - Fork 5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Desktop: Resolves #11063: Improve the performance of GoToAnything #11064
Desktop: Resolves #11063: Improve the performance of GoToAnything #11064
Conversation
| valueRegex?: RegExp; | ||
| valueRegex?: string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Within SearchEngine.ts, valueRegex is currently set to the result of SearchEngine.queryTermToRegex, which returns a string and not a RegExp.
| export const pregQuote = stringUtilsCommon.pregQuote; | ||
| export const pregQuote = stringUtilsCommon.pregQuote as (str: string, delimiter?: string)=> string; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
pregQuote is imported from string-utils-common.js. As such, it previously had no type information. This line causes SearchEngine.queryTermToRegex to return string instead of any.
|
I wonder if we could skip very large notes, or not process them with removeDiacritics? By the way we have the note without diacritics already in the normalized_notes table - I wonder if that could help? |
I've updated
So far, this seems to prevent Joplin from freezing while searching with go-to-anything, but, at present, I'm testing mostly with a database of auto-generated notes with many repeated words. |
|
Looks good, thanks for fixing this! |
|
I'm using the webclipper to save scientific articles to my database. GoToAnything was not working correctly for some time because the search list was being updated randomly by incoming search results, rendering the search less useful for several seconds. |
The fix was for the desktop go-to-anything dialog only — see #11032 for the related mobile-specific issue. |
|
Thank you for the clarification! |
Summary
This pull request partially resolves #11063 by improving the performance of the GoToAnything dialog. In particular, it:
removeDiacriticsis called (removeDiacriticsis slow for large input texts).AsyncActionQueueinstead of asetTimeout.updateListisasync, this should prevent multipleupdateListcalls from running concurrently.Note: Although this pull request improves the performance of
GoToAnything, it is still slow. See the "performance comparison" section below.Performance comparison (old: from before e704fe8)
Note: The below tests were done before commit e704fe8.
I've manually tested this pull request by creating 20 long notes. Then, with and without the pull request applied, 1) applying a diff then 2) searching for "this is a test" with GoToAnything.
Diff used to generate the comparison
Before
With a slightly longer search
After
With a slightly longer search
Comparison
Effect of search length on

updateListrunning time:The above chart plots the data collected above — the length of the searched text (e.g.
this is= 7) is plotted on the x-axis and the time forupdateListto process that search on the y-axis. Observe that the search running time is significantly greater for length=14pre_pr(before applying the pull request) thanpost_pr(after applying the pull request).