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

Replace operation limit API with a clock-based timeout API #301

Merged
merged 6 commits into from
Mar 14, 2019

Conversation

maxbrunsfeld
Copy link
Contributor

Background

When I initially implemented the multi-threaded parsing support in Atom, I hacked in a feature called a "parsing operation limit" in order to avoid unnecessary threading overhead in the case of fast parsing. At the time, I avoided using actual time as the limiting "unit", because I wanted to avoid introducing any system call overhead during parsing.

Improvements

In this PR, I've replaced the ts_parser_set_operation_limit function with a new function called ts_parser_set_timeout_micros. This new fuction allows you to provide a timeout value in microseconds instead of an abstract "operation limit".

I've also added a --timeout flag to the tree-sitter parse command so that we can easily test the behavior of timeouts at the command line.

Implementation

The new timeout functionality relies on the clock(3) POSIX API and the CLOCKS_PER_SEC constant. I initially found that calling clock() on every parse action imposed a huge amount of overhead, so I added a counter to ensure that clock() is only called every 100 parse actions. This change eliminated any observable performance overhead.

@maxbrunsfeld maxbrunsfeld force-pushed the clock-based-timeouts branch 3 times, most recently from 859c1d3 to 070a3ce Compare March 14, 2019 20:32
@maxbrunsfeld maxbrunsfeld force-pushed the clock-based-timeouts branch 3 times, most recently from df266fa to 18e825c Compare March 14, 2019 22:38
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.

1 participant