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

[REPLCompletions] async tab/hint completion #52847

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

Conversation

aviatesk
Copy link
Sponsor Member

REPL utilizes inference for smarter completions, but since its inference engine is independent from the native cache and issues like #52763 in REPLInterpreter's inference, sometimes delays can be noticeable during tab/hint-completion, and REPL can even freeze in the worst cases.

To address this, the commit allows tab/hint-completion to run asynchronously on a :default thread. This approach removes delays in multi-threaded process and prevents REPL hang in extreme cases like #52763. In detail, for tab-completion, completion task simply runs on a separate thread since completion itself does not block anything. For hint-completion however, considering that a stuttering display of hints can be bothersome, when the completion task doesn't conclude within 0.01 sec, we cancel the hint-completion to keep the input in the REPL smooth.

There are still some points left open for discussion:

  • Is it better to allocate a separate thread specifically for REPL completion? I'm not sure if this is even possible though.
  • Should we make this an optional feature, considering the concern some users might have about reduced thread resources?
  • For situations where completion hangs or is slow, like in REPL becomes unresponsive during completion inference #52763, canceling the completion task to release resources would be ideal. However, my understanding is that there's no safe method to cancel a task, is it correct?
  • At present, this commit Threads.@spawns at a relatively high level in the completion pipeline. But it might be more effective to Threads.@spawn closer to the point where inference is actually invoked, such as in complete_line or repl_eval_ex.

@aviatesk aviatesk added the stdlib:REPL Julia's REPL (Read Eval Print Loop) label Jan 10, 2024
@IanButterworth
Copy link
Sponsor Member

Related: #52833

REPL utilizes inference for smarter completions, but since its inference
engine is independent from the native cache and issues like #52763 in
`REPLInterpreter`'s inference, sometimes delays can be noticeable during
tab/hint-completion, and REPL can even freeze in the worst cases.

To address this, the commit allows tab/hint-completion to run
asynchronously on a `:default` thread. This approach removes delays in
multi-threaded process and prevents REPL hang in extreme cases like #52763.
In detail, for tab-completion, completion task simply runs on a separate
thread since completion itself does not block anything. For
hint-completion however, considering that a stuttering display of hints
can be bothersome, when the completion task doesn't conclude within
0.01 sec, we cancel the hint-completion to keep the input in the REPL
smooth.

There are still some points left open for discussion:
- Is it better to allocate a separate thread specifically for REPL
  completion? I'm not sure if this is even possible though.
- Should we make this an optional feature, considering the concern some
  users might have about reduced thread resources?
- For situations where completion hangs or is slow, like in #52763,
  canceling the completion task to release resources would be ideal.
  However, my understanding is that there's no safe method to cancel a
  task, is it correct?
- At present, this commit `Threads.@spawn`s at a relatively high level
  in the completion pipeline. But it might be more effective to
  `Threads.@spawn` closer to the point where inference is actually
  invoked, such as in `complete_line` or `repl_eval_ex`.
@Keno
Copy link
Member

Keno commented Jan 12, 2024

Does the 0.01 sec also apply to the implicit completion? If so that doesn't make much sense to me, it feels like that should just run with however much time it has until the user next types. On explicit tab, a timeout makes sense to me, although we could maybe give the thread a little more time to populate the cache and make it more likely to succeed on the next tab invocation.

@IanButterworth
Copy link
Sponsor Member

I think for both hint and tab it makes sense to freely run until the next key press.

Also note that a big common reason for slow completions can be fixed by #52833 but I think these PRs are complimentary.

@vtjnash
Copy link
Sponsor Member

vtjnash commented Jan 12, 2024

canceling the completion task to release resources would be ideal. However, my understanding is that there's no safe method to cancel a task, is it correct?

You could have this safely: this is likely just an extra absint feature to (asynchronously) cause all future calls to return Limited@topmost (and clear that bit upon return)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stdlib:REPL Julia's REPL (Read Eval Print Loop)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants