REPL becomes unresponsive during completion inference #52763
Labels
compiler:inference
Type inference
kind:bug
Indicates an unexpected problem or unintended behavior
stdlib:REPL
Julia's REPL (Read Eval Print Loop)
Comments
Liozou
added
stdlib:REPL
DilumAluthge
added
the
kind:bug
|
Dup of #52765? |
I don't think so - Looks like a similar kind of issue to what @aviatesk fixed up in #52186 (comment) That PR fixed this up for array operations, but we still seem to have bad hangs when inferring through larger packages. |
aviatesk
added a commit
that referenced
this issue
Jan 10, 2024
Investigating into #52763, I've found that `AbstractInterpreters` which enables the `aggressive_constprop` option, such as `REPLInterpreter`, might perform const-prop' even if the result of a non-const call is `LimitedAccuracy'. This can lead to an unintended infinite loop with a custom aggressive const-prop' implementation. This commit restricts const-prop' for such cases where the non-const call result is limited to avoid the issue. This fix is conservative, but given that accurate inference is mostly impossible when there are unresolvable cycles (which is indicated by limited result), aggressive const-prop' isn't necessary for such cases, and I don't anticipate this leading to any obvious regression. fix #52763
aviatesk
added a commit
that referenced
this issue
Jan 10, 2024
Investigating into #52763, I've found that `AbstractInterpreters` which enables the `aggressive_constprop` option, such as `REPLInterpreter`, might perform const-prop' even if the result of a non-const call is `LimitedAccuracy'. This can lead to an unintended infinite loop with a custom aggressive const-prop' implementation. This commit restricts const-prop' for such cases where the non-const call result is limited to avoid the issue. This fix is conservative, but given that accurate inference is mostly impossible when there are unresolvable cycles (which is indicated by limited result), aggressive const-prop' isn't necessary for such cases, and I don't anticipate this leading to any obvious regression. fix #52763
aviatesk
added a commit
that referenced
this issue
Jan 10, 2024
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`.
aviatesk
added a commit
that referenced
this issue
Jan 11, 2024
Investigating into #52763, I've found that `AbstractInterpreters` which enables the `aggressive_constprop` option, such as `REPLInterpreter`, might perform const-prop' even if the result of a non-const call is `LimitedAccuracy'. This can lead to an unintended infinite loop with a custom aggressive const-prop' implementation. This commit restricts const-prop' for such cases where the non-const call result is limited to avoid the issue. This fix is conservative, but given that accurate inference is mostly impossible when there are unresolvable cycles (which is indicated by limited result), aggressive const-prop' isn't necessary for such cases, and I don't anticipate this leading to any obvious regression. fix #52763
aviatesk
added a commit
that referenced
this issue
Jan 11, 2024
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`.
|
Simpler MWE that I hit today: julia> muladd(t) = 0.0 * t + 10.0
muladd (generic function with 1 method)
julia> (@code_typed muladd(0.0)).<tab>Pause is >10 seconds |
|
Indeed, since the code in question internally invokes the whole Julia level compilation pipeline, it's reasoable for the REPL inference (by |
aviatesk
added a commit
that referenced
this issue
Jan 13, 2024
Investigating into #52763, I've found that `AbstractInterpreters` which enables the `aggressive_constprop` option, such as `REPLInterpreter`, might perform const-prop' even if the result of a non-const call is `LimitedAccuracy'. This can lead to an unintended infinite loop with a custom aggressive const-prop' implementation. This commit restricts const-prop' for such cases where the non-const call result is limited to avoid the issue. This fix is conservative, but given that accurate inference is mostly impossible when there are unresolvable cycles (which is indicated by limited result), aggressive const-prop' isn't necessary for such cases, and I don't anticipate this leading to any obvious regression. fix #52763
aviatesk
added a commit
that referenced
this issue
Jan 13, 2024
Investigating into #52763, I've found that `AbstractInterpreters` which enables the `aggressive_constprop` option, such as `REPLInterpreter`, might perform const-prop' even if the result of a non-const call is `LimitedAccuracy'. This can lead to an unintended infinite loop with a custom aggressive const-prop' implementation. This commit restricts const-prop' for such cases where the non-const call result is limited to avoid the issue. This fix is conservative, but given that accurate inference is mostly impossible when there are unresolvable cycles (which is indicated by limited result), aggressive const-prop' isn't necessary for such cases, and I don't anticipate this leading to any obvious regression. fix #52763
aviatesk
added a commit
that referenced
this issue
Jan 14, 2024
Investigating into #52763, I've found that `AbstractInterpreters` which enables the `aggressive_constprop` option, such as `REPLInterpreter`, might perform const-prop' even if the result of a non-const call is `LimitedAccuracy'. This can lead to an unintended infinite loop with a custom aggressive const-prop' implementation. This commit restricts const-prop' for such cases where the non-const call result is limited to avoid the issue. This fix is conservative, but given that accurate inference is mostly impossible when there are unresolvable cycles (which is indicated by limited result), aggressive const-prop' isn't necessary for such cases, and I don't anticipate this leading to any obvious regression. fix #52763
aviatesk
added a commit
that referenced
this issue
Jan 14, 2024
Investigating into #52763, I've found that `AbstractInterpreters` which enables the `aggressive_constprop` option, such as `REPLInterpreter`, might perform const-prop' even if the result of a non-const call is `LimitedAccuracy'. This can lead to an unintended infinite loop with a custom aggressive const-prop' implementation. This commit restricts const-prop' for such cases where the non-const call result is limited to avoid the issue. This fix is conservative, but given that accurate inference is mostly impossible when there are unresolvable cycles (which is indicated by limited result), aggressive const-prop' isn't necessary for such cases, and I don't anticipate this leading to any obvious regression. fix #52763
aviatesk
added a commit
that referenced
this issue
Jan 14, 2024
Investigating into #52763, I've found that `AbstractInterpreters` which enables the `aggressive_constprop` option, such as `REPLInterpreter`, might perform const-prop' even if the result of a non-const call is `LimitedAccuracy'. This can lead to an unintended infinite loop with a custom aggressive const-prop' implementation. This commit restricts const-prop' for such cases where the non-const call result is limited to avoid the issue. This fix is conservative, but given that accurate inference is mostly impossible when there are unresolvable cycles (which is indicated by limited result), aggressive const-prop' isn't necessary for such cases, and I don't anticipate this leading to any obvious regression. fix #52763
aviatesk
added a commit
that referenced
this issue
Jan 15, 2024
Investigating into #52763, I've found that `AbstractInterpreters` which enables the `aggressive_constprop` option, such as `REPLInterpreter`, might perform const-prop' even if the result of a non-const call is `LimitedAccuracy'. This can lead to an unintended infinite loop with a custom aggressive const-prop' implementation. This commit restricts const-prop' for such cases where the non-const call result is limited to avoid the issue. This fix is conservative, but given that accurate inference is mostly impossible when there are unresolvable cycles (which is indicated by limited result), aggressive const-prop' isn't necessary for such cases, and I don't anticipate this leading to any obvious regression. fix #52763
aviatesk
added a commit
that referenced
this issue
Jan 15, 2024
Investigating into #52763, I've found that `AbstractInterpreters` which enables the `aggressive_constprop` option, such as `REPLInterpreter`, might perform const-prop' even if the result of a non-const call is `LimitedAccuracy'. This can lead to an unintended infinite loop with a custom aggressive const-prop' implementation. This commit restricts const-prop' for such cases where the non-const call result is limited to avoid the issue. This fix is conservative, but given that accurate inference is mostly impossible when there are unresolvable cycles (which is indicated by limited result), aggressive const-prop' isn't necessary for such cases, and I don't anticipate this leading to any obvious regression. fix #52763
aviatesk
added a commit
that referenced
this issue
Jan 15, 2024
Investigating into #52763, I've found that `AbstractInterpreters` which enables the `aggressive_constprop` option, such as `REPLInterpreter`, might perform const-prop' even if the result of a non-const call is `LimitedAccuracy'. This can lead to an unintended infinite loop with a custom aggressive const-prop' implementation. This commit restricts const-prop' for such cases where the non-const call result is limited to avoid the issue. This fix is conservative, but given that accurate inference is mostly impossible when there are unresolvable cycles (which is indicated by limited result), aggressive const-prop' isn't necessary for such cases, and I don't anticipate this leading to any obvious regression. fix #52763
aviatesk
added a commit
that referenced
this issue
Jan 16, 2024
…52836) Investigating into #52763, I've found that `AbstractInterpreters` which enables the `aggressive_constprop` option, such as `REPLInterpreter`, might perform const-prop' even if the result of a non-const call is `LimitedAccuracy`. This can lead to an unintended infinite loop with a custom aggressive const-prop' implementation. This commit restricts const-prop' for such cases where the non-const call result is limited to avoid the issue. This fix is conservative, but given that accurate inference is mostly impossible when there are unresolvable cycles (which is indicated by limited result), aggressive const-prop' isn't necessary for such cases, and I don't anticipate this leading to any obvious regression. fix #52763
aviatesk
added a commit
that referenced
this issue
Jan 16, 2024
…52836) Investigating into #52763, I've found that `AbstractInterpreters` which enables the `aggressive_constprop` option, such as `REPLInterpreter`, might perform const-prop' even if the result of a non-const call is `LimitedAccuracy`. This can lead to an unintended infinite loop with a custom aggressive const-prop' implementation. This commit restricts const-prop' for such cases where the non-const call result is limited to avoid the issue. This fix is conservative, but given that accurate inference is mostly impossible when there are unresolvable cycles (which is indicated by limited result), aggressive const-prop' isn't necessary for such cases, and I don't anticipate this leading to any obvious regression. fix #52763
Drvi
pushed a commit
to RelationalAI/julia
that referenced
this issue
Jun 7, 2024
…uliaLang#52836) Investigating into JuliaLang#52763, I've found that `AbstractInterpreters` which enables the `aggressive_constprop` option, such as `REPLInterpreter`, might perform const-prop' even if the result of a non-const call is `LimitedAccuracy`. This can lead to an unintended infinite loop with a custom aggressive const-prop' implementation. This commit restricts const-prop' for such cases where the non-const call result is limited to avoid the issue. This fix is conservative, but given that accurate inference is mostly impossible when there are unresolvable cycles (which is indicated by limited result), aggressive const-prop' isn't necessary for such cases, and I don't anticipate this leading to any obvious regression. fix JuliaLang#52763
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
On master, typing the following makes the REPL completely unresponsive with 100% CPU usage for more than an hour:
(the last comma must be typed, not copy-pasted, or you have to type something after the comma in order to kick completion into action)
It does not respond to Ctrl+C either, but does print a stacktrace when sending a SIGUSR1:
SIGUSR1 stacktrace from julia-debug (first example)
SIGUSR1 stacktrace from julia-debug (example 2)
Two comparison points:
Clapeyron.GERG2008(["carbon dioxide"])object first removes the issue:To avoid REPL freezes due to inference in general, a speculative solution could be to launch REPL completion inference from a separate (non-
interactive) dedicated thread, and make it easily interruptible from a signal without killing the REPL? At least we'll need a way to interrupt inference without crashing the session if it's stuck.For this particular issue though, I would assume there is an inference heuristic incorrectly set for REPL completion because actually executing the following code (by copy-pasting it into the REPL, or from a file) takes less than a minute to compile and execute:
versioninfo and Clapeyron version
The text was updated successfully, but these errors were encountered: