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

Require rbs outside of thread to avoid deadlock #35

Merged
merged 1 commit into from
Sep 9, 2024

Conversation

tompng
Copy link
Member

@tompng tompng commented Sep 8, 2024

IRB's debug command stops all threads. When thread which is running require 'rbs' stops, all require will be blocked forever because require has exclusive control.
In IRB, pressing TAB will start require 'rdoc' but it will deadlock if debug mode started before require 'rbs' completes.

To avoid it, all require should be done in main thread.
This will make IRB startup time slow about 43%.

# Benchmarks

PTY.spawn('irb', '--regexp-completor').first.getc
# processing time: 0.168581s

PTY.spawn('irb', '--type-completor').first.getc # main branch
# processing time: 0.189315s

PTY.spawn('irb', '--type-completor').first.getc # this branch
# processing time: 0.271634s

Using irb's debug command, debug will stop all threads.
When thread running require is stopped, all require will be blocked forever.
Copy link
Member

@st0012 st0012 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this slowdown is acceptable. But I wonder if using autoload could be a better solution? AFAIK, debug doesn't interfere code autoloading.

@tompng
Copy link
Member Author

tompng commented Sep 9, 2024

Autoload will delay loading until its needed. But loading *.rbs should start as soon as possible to make the chance of type information ready before user enters 1.times.m which needs type to autocomplete.

@tompng tompng merged commit f2fc343 into main Sep 9, 2024
26 checks passed
@tompng tompng deleted the debugger_no_deadlock branch September 9, 2024 11:45
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