SnoopPrecompile + pipe handling

[timholy]

This switches to SnoopPrecompile and uses more careful handling of pipes to ensure tasks run to completion (see comment in #343).

Would be grateful if you could check if this is now correct, @vtjnash.

On Julia 1.9+, switching to SnoopPrecompile has an unexpectedly-large benefit for a compile=min package: descend(gcd, (Int, Int)) now feels practically instantaneous.

[codecov-commenter]

Codecov Report

Merging #344 (ab7a6a9) into master (362a3ae) will decrease coverage by 0.19%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #344      +/-   ##
==========================================
- Coverage   32.65%   32.47%   -0.19%     
==========================================
  Files          10       10              
  Lines        1084     1084              
==========================================
- Hits          354      352       -2     
- Misses        730      732       +2     

Impacted Files	Coverage Δ
src/Cthulhu.jl	33.68% <ø> (ø)
src/ui.jl	45.65% <0.00%> (-4.35%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[timholy]

Hmm, seems to hang (presumably during precompilation) on nightly. Doesn't do that locally though, unsure what's going on.

[vtjnash]

You still need an async read, not just the flag indicating support for it

[vtjnash]

This line needs to be listed before you start writing to the pipe. I can't suggest on a large enough region in github, but roughly:

input, output, err = linked_pipe(), linked_pipe(), linked_pipe()
# set up reader async support
tout = @async write(devnull, output)
terr = @async write(devnull, err) # or read(devnull, String) if you want error messages
# do work here
write(input.in, 'q')
@precompile_all_calls begin
    descend(gcd, (Int, Int); terminal=term)
    # declare we are done with streams
    close(input.in)
    close(output.in)
    close(err.in)
    # pause for the REPL to finish precompiling
    wait(tout)
    wait(terr)
end

[vtjnash]

(you could also still use @sync instead of the explicit variables with wait—the macros expand to the same thing, but it seems it would be harder to nest precompile_all_calls in that case)

[vtjnash]

(very strictly, the write(input.in, 'q') should also be @async too, but kernel buffering doesn't care that much about a single byte. The rules guidelines are that (a) each Task can manage any number of in objects (such as stdout/stderr), but should only access exactly one out (such as stdin) and (b) each end of an IO object should be accessed only from one Task. The second rule means that if we moved the 'q' into a separate task, the close(input.in) call must go with it too into the same task.)

Sorry, I don't know any way currently to help simplify this so that IO is harder to mess up. It is just part of the reality of writing async-event-driven programs. For something like linked_pipe, perhaps Base could have a callback interface for this, so that there is a visible separation between the object behaviors like this:

in, t = linked_pipe() do out
    # what to do with output here
    @show read(out, String)
end
# what to do with input end here
write(in, "Hello\n")
close(in)
wait(t)

Where linked_pipe is pretty simple:

function linked_pipe(reader::Callable)
    (; in, out) = linked_pipe()
    in, @async reader(out)
end

[vtjnash]

Alternatively, define that linked_pipe() = Base.BufferStream(). Since that is an immediate Julia object without kernel backing and with back-pressure disabled, it lets you write pretty straightforward code that would be incorrect for a general IO object, but works as desired without to manage Tasks.

[timholy]

That sounds like a nice idea, except

julia> descend(gcd, (Int, Int); terminal=term)
┌ Warning: TerminalMenus: Unable to enter raw mode:
│   exception =
│    type BufferStream has no field handle
│    Stacktrace:
│      [1] getproperty(stream::Base.BufferStream, name::Symbol)
│        @ Base ./stream.jl:60
│      [2] raw!(t::REPL.Terminals.TTYTerminal, raw::Bool)
│        @ REPL.Terminals ~/.julia/juliaup/julia-1.9.0-beta3+0.x64.linux.gnu/share/julia/stdlib/v1.9/REPL/src/Terminals.jl:139
│      [3] request(term::REPL.Terminals.TTYTerminal, m::Cthulhu.CthulhuMenu; cursor::Int64, suppress_output::Bool)
│        @ REPL.TerminalMenus ~/.julia/juliaup/julia-1.9.0-beta3+0.x64.linux.gnu/share/julia/stdlib/v1.9/REPL/src/TerminalMenus/AbstractMenu.jl:192
│      [4] request
│        @ ~/.julia/juliaup/julia-1.9.0-beta3+0.x64.linux.gnu/share/julia/stdlib/v1.9/REPL/src/TerminalMenus/AbstractMenu.jl:181 [inlined]
│      [5] request(term::REPL.Terminals.TTYTerminal, msg::String, m::Cthulhu.CthulhuMenu; kwargs::Base.Pairs{Symbol, Union{}, Tuple{}, NamedTuple{(), Tuple{}}})
│        @ REPL.TerminalMenus ~/.julia/juliaup/julia-1.9.0-beta3+0.x64.linux.gnu/share/julia/stdlib/v1.9/REPL/src/TerminalMenus/AbstractMenu.jl:259
│      [6] request(term::REPL.Terminals.TTYTerminal, msg::String, m::Cthulhu.CthulhuMenu)
│        @ REPL.TerminalMenus ~/.julia/juliaup/julia-1.9.0-beta3+0.x64.linux.gnu/share/julia/stdlib/v1.9/REPL/src/TerminalMenus/AbstractMenu.jl:257
│      [7] _descend(term::REPL.Terminals.TTYTerminal, interp::Cthulhu.CthulhuInterpreter, curs::Cthulhu.CthulhuCursor; override::Nothing, debuginfo::Symbol, optimize::Bool, interruptexc::Bool, iswarn::Bool, hide_type_stable::Bool, verbose::Nothing, remarks::Bool, with_effects::Bool, inline_cost::Bool, type_annotations::Bool)
│        @ Cthulhu ~/.julia/dev/Cthulhu/src/Cthulhu.jl:538

Would it be more appropriate to add a check to raw! or should we not use a BufferedStream?

[timholy]

Does this version pass muster? I think the errors are independent. (?)

[vtjnash]

LGTM