Skip to content
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

Rework QPlainEditSearchWidget #151

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from
Open

Rework QPlainEditSearchWidget #151

wants to merge 1 commit into from

Conversation

fnkhz
Copy link
Contributor

@fnkhz fnkhz commented Jan 7, 2022

Work in progress.
This should address some performance issues and some minor issues in QPlainEditSearchWidget.

  • runs search across the entire document only when required (when the search term is changed and when the QPlainTextEdit is modified and the search is invalidated); otherwise, for the up/down search and replacements, it uses the QTextCursor list fetched in the first search.

Missing / required changes:

  • further testing
  • could be simplified
  • benchmark to compare with previous version
  • further improvements by replacing deboucing with asynchronous search

@pbek pbek added the WIP work in progress label Jan 7, 2022
@Waqar144
Copy link
Contributor

Waqar144 commented Jan 7, 2022

FWIW, this isn't going to make the search very fast. I tried this change and a few other things and ran some benchmarks and it seems the biggest slow down is setExtraSelections. As long as we are using that, things will be slow. You can't do this async. Profile below shows the amount of time doing the search (far right) vs the amount of time showing the results.

image

So besides the bug fixes etc, imo the added complexity is worth it. To make the search fast we need to avoid setExtraSelections and do the search highlighting outselves for only the region that is visible to the user for a start. To be clear by fast I mean ~100ms max for files around ~5MB on a cpu from 2015 or so.

@Waqar144
Copy link
Contributor

Waqar144 commented Jan 7, 2022

Good news is that the search highlighting code is already there in QtCreator's code. One just needs to to adapt it to our text editor.

@Waqar144
Copy link
Contributor

Waqar144 commented Jan 8, 2022

We can also use our syntax highlighter to highlight search results

@fnkhz
Copy link
Contributor Author

fnkhz commented Jan 15, 2022

Thanks for the suggestions @Waqar144 . Will get back to this as soon as I get some spare time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WIP work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants