Skip to content

Throttle calls to await.delay() in some diagnostics #2680

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

Merged
merged 1 commit into from
May 27, 2024

Conversation

emmericp
Copy link
Contributor

These 5 diagnostics cause ~70% of all calls to await.delay() by diagnostics which in turn is about ~20% of the total runtime of diagnostics.

Out of these diagnostics only assign-type-mismatch commonly exceeds runtimes of 100ms (worst observed in my dataset was 7 seconds) and even then it still attempts to call await.delay() over 1500 times per second, so throttling by a factor of 15 is still fine.

There are likely more places where await() could be optimized, but the diagnostics are the low-hanging fruit because they are easy to find.

These 5 diagnostics cause ~70% of all calls to await.delay() by
diagnostics which in turn is about ~20% of the total runtime of
diagnostics.

Out of these diagnostics only assign-type-mismatch commonly exceeds
runtimes of 100ms (worst observed in my dataset was 7 seconds) and
even then it still attempts to call await.delay() over 1500 times
per second, so throttling by a factor of 15 is still fine.
@sumneko
Copy link
Collaborator

sumneko commented May 25, 2024

The purpose of calling delay is to give the program a chance to respond promptly to user actions. Perhaps using a time-based calculation would be better (but it might incur higher overhead for obtaining the time, which needs to be tested).

@emmericp
Copy link
Contributor Author

I understand the idea behind the rescheduling, but it just happens too often. Especially redundant-value and trailing-space are way too aggressive, there is no reason to reschedule after every single line or every single token. I considered using a time-based approach, but I was hesitant to use os.clock() because it is heavily system-dependent and not exactly accurate. Even if it was time-based then checking the time for every single line/token is a bit much

@sumneko sumneko merged commit e2ca72d into LuaLS:master May 27, 2024
10 checks passed
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.

2 participants