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

Limit the maximum number of highlight markers #193

Conversation

wbinnssmith
Copy link
Contributor

@wbinnssmith wbinnssmith commented Nov 28, 2018

This places a configurable limit on the number of highlights shown across all editors.
In my tests, after creating a selection, scanning a large file (50K+ lines) with a match on each line causes Atom's UI thread to lock up for several seconds at a time.

It doesn't, however, lock up when there are only a few matches across files of that size, as a lot of time is spent creating and managing the many markers that result. Likewise, it seems that editor.scan takes considerably longer when there are matches compared to when there aren't on documents of equal size.

By default, the maximum number of results is limited to 500. On my machine (a 2017-era MacBook Pro), scanning a 50K line document with matches on each line with this limit applied reliably completes in under 80ms, which is well under the threshold for a perceptible delay. I'm definitely open to changing this default, especially if it's smaller :)

This is all implemented by throwing an exception once we've hit our result threshold. It's gross, and
I've tried an early return in the scan callback, but the scan runs to completion either way and we end up doing nothing with results that we spent work on. AFAICT, this is the only way we can terminate the scan early without upstreaming support for this in Atom's native C++ text buffer.

cc @matthewwithanm @captbaritone @sunz7

This places a configurable limit on the number of highlights shown across all editors.
In my tests, scanning a large file (50K+ lines) with a match on each line causes Atom's UI thread
to lock up for several seconds at a time. It doesn't, however, lock up when there are only a few
matches across files of that size, as a lot of time is spent creating and managing the many markers
that result. Likewise, it seems that `editor.scan` takes considerably longer when there are matches
compared to when there aren't on documents of equal size.

By default, the number of markers is limited to 500. On my machine (a 2017-era MacBook Pro), scanning
a 50K line document with matches on each line with this limit applied reliably completes in under 80ms,
which is [well under the threshold for a perceptible delay](https://developers.google.com/web/fundamentals/performance/rail).
I'm definitely open to changing this default, especially if it's smaller :)

This is all implemented by throwing an exception once we've hit our result threshold. It's gross, and
I've tried an early `return` in the `scan` callback, but the scan runs to completion either way and we end up
doing nothing with results that we spent work on. AFAICT, this is the only way we can terminate the scan early
without upstreaming support for this in Atom's native C++ text buffer.

cc @matthewwithanm @captbaritone @sunz7
@danielocdh
Copy link

danielocdh commented Dec 29, 2018

Not sure why this didn't work for me at first, I had to add an empty constructor to EarlyTerminationSignal and then it worked, I'm using windows.

Thank you

@richrace richrace merged commit 881e456 into Pulsar-Edit-Highlights:master Mar 17, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants