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

Simplify precompiles #669

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/callbacks.jl
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ You can use the return value `key` to remove the callback later
(`Revise.remove_callback`) or to update it using another call
to `Revise.add_callback` with `key=key`.
"""
function add_callback(f, files, modules=nothing; all=false, key=gensym())
function add_callback(@nospecialize(f), files, modules=nothing; all=false, key=gensym())
fix_trailing(path) = isdir(path) ? joinpath(path, "") : path # insert a trailing '/' if missing, see https://github.com/timholy/Revise.jl/issues/470#issuecomment-633298553

remove_callback(key)
Expand Down Expand Up @@ -91,7 +91,7 @@ end
Remove a callback previously installed by a call to `Revise.add_callback(...)`.
See its docstring for details.
"""
function remove_callback(key)
function remove_callback(@nospecialize(key))
for cbs in values(user_callbacks_by_file)
delete!(cbs, key)
end
Expand Down Expand Up @@ -152,7 +152,7 @@ end
This will print "update" every time `"/tmp/watched.txt"` or any of the code defining
`Pkg1` or `Pkg2` gets updated.
"""
function entr(f::Function, files, modules=nothing; all=false, postpone=false, pause=0.02)
function entr(@nospecialize(f), files, modules=nothing; all=false, postpone=false, pause=0.02)
yield()
postpone || f()
key = add_callback(files, modules; all=all) do
Expand Down
51 changes: 32 additions & 19 deletions src/lowered.jl
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ add_dependencies!(methodinfo::MethodInfo, be::CodeEdges, src, isrequired) = meth
add_includes!(methodinfo::MethodInfo, mod::Module, filename) = methodinfo

# This is not generally used, see `is_method_or_eval` instead
function hastrackedexpr(stmt; heads=LoweredCodeUtils.trackedheads)
function hastrackedexpr(@nospecialize(stmt); heads=LoweredCodeUtils.trackedheads)
haseval = false
if isa(stmt, Expr)
haseval = matches_eval(stmt)
Expand Down Expand Up @@ -122,7 +122,7 @@ end
function methods_by_execution(mod::Module, ex::Expr; kwargs...)
methodinfo = MethodInfo()
docexprs = DocExprs()
value, frame = methods_by_execution!(JuliaInterpreter.Compiled(), methodinfo, docexprs, mod, ex; kwargs...)
value, frame = methods_by_execution!(Base.inferencebarrier(JuliaInterpreter.Compiled()), methodinfo, docexprs, mod, ex; kwargs...)
Copy link
Collaborator

Choose a reason for hiding this comment

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

This seems to be an actual usecase example of JuliaLang/julia#41931. As far as I understand, does it turn out to be effective to turn off inference of recurse(...) within a callstack of methods_by_execution!? If recurse(...) is less-frequently called actually, then that sounds a reasonable hack to improve latency.

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes, this is exactly a @noinfer. All the execution time specific to a particular recurse is in JuliaInterpreter anyway.

return methodinfo, docexprs, frame
end

Expand Down Expand Up @@ -233,10 +233,11 @@ function methods_by_execution!(@nospecialize(recurse), methodinfo, docexprs, mod
return ret, lwr
end
methods_by_execution!(methodinfo, docexprs, mod::Module, ex::Expr; kwargs...) =
methods_by_execution!(JuliaInterpreter.Compiled(), methodinfo, docexprs, mod, ex; kwargs...)
methods_by_execution!(Base.inferencebarrier(JuliaInterpreter.Compiled()), methodinfo, docexprs, mod, ex; kwargs...)

function methods_by_execution!(@nospecialize(recurse), methodinfo, docexprs, frame::Frame, isrequired::AbstractVector{Bool}; mode::Symbol=:eval, skip_include::Bool=true)
isok(lnn::LineTypes) = !iszero(lnn.line) || lnn.file !== :none # might fail either one, but accept anything
lnnstd(lnn::LineTypes) = LineNumberNode(lnn.line, lnn.file)

mod = moduleof(frame)
# Hoist this lookup for performance. Don't throw even when `mod` is a baremodule:
Expand All @@ -254,11 +255,7 @@ function methods_by_execution!(@nospecialize(recurse), methodinfo, docexprs, fra
if isa(stmt, Expr)
head = stmt.head
if head === :toplevel
local value
for ex in stmt.args
ex isa Expr || continue
value = methods_by_execution!(recurse, methodinfo, docexprs, mod, ex; mode=mode, disablebp=false, skip_include=skip_include)
end
value = handle_toplevel(recurse, methodinfo, docexprs, mod, stmt, mode, skip_include)
isassign(frame, pc) && assign_this!(frame, value)
pc = next_or_nothing!(frame)
# elseif head === :thunk && isanonymous_typedef(stmt.args[1])
Expand Down Expand Up @@ -289,9 +286,9 @@ function methods_by_execution!(@nospecialize(recurse), methodinfo, docexprs, fra
lnn = nothing
if line_is_decl
sigcode = @lookup(frame, stmt3.args[2])::Core.SimpleVector
lnn = sigcode[end]
if !isa(lnn, LineNumberNode)
lnn = nothing
lnntmp = sigcode[end]
if isa(lnntmp, LineNumberNode)
lnn = lnntmp
end
end
if lnn === nothing
Expand All @@ -300,9 +297,10 @@ function methods_by_execution!(@nospecialize(recurse), methodinfo, docexprs, fra
bodycode = @lookup(frame, bodycode)
end
if isa(bodycode, CodeInfo)
lnn = linetable(bodycode, 1)
if !isok(lnn)
lnn = nothing
lnn_ = linetable(bodycode, 1)
if isok(lnn_)
lnn = lnnstd(lnn_)
else
if length(bodycode.code) > 1
# This may be a kwarg method. Mimic LoweredCodeUtils.bodymethod,
# except without having a method
Expand Down Expand Up @@ -335,7 +333,7 @@ function methods_by_execution!(@nospecialize(recurse), methodinfo, docexprs, fra
for lnntmp in linetable(bodycode)
lnntmp = lnntmp::LineTypes
if isok(lnntmp)
lnn = lnntmp
lnn = lnnstd(lnntmp)
break
end
end
Expand All @@ -345,7 +343,7 @@ function methods_by_execution!(@nospecialize(recurse), methodinfo, docexprs, fra
bodycode = bodycode::Expr
lnntmp = bodycode.args[end][1]::LineTypes
if isok(lnntmp)
lnn = lnntmp
lnn = lnnstd(lnntmp)
end
end
end
Expand All @@ -354,7 +352,7 @@ function methods_by_execution!(@nospecialize(recurse), methodinfo, docexprs, fra
while i > 0
lnntmp = linetable(frame, i)
if isok(lnntmp)
lnn = lnntmp
lnn = lnnstd(lnntmp)
break
end
i -= 1
Expand Down Expand Up @@ -383,15 +381,15 @@ function methods_by_execution!(@nospecialize(recurse), methodinfo, docexprs, fra
end
newex = unwrap(newex)
push_expr!(methodinfo, newmod, newex)
value = methods_by_execution!(recurse, methodinfo, docexprs, newmod, newex; mode=mode, skip_include=skip_include, disablebp=false)
value = handle_eval(recurse, methodinfo, docexprs, newmod, newex, mode, skip_include)
pop_expr!(methodinfo)
end
assign_this!(frame, value)
pc = next_or_nothing!(frame)
elseif skip_include && (f === modinclude || f === Base.include || f === Core.include)
# include calls need to be managed carefully from several standpoints, including
# path management and parsing new expressions
add_includes!(methodinfo, mod, @lookup(frame, stmt.args[2]))
add_includes!(methodinfo, mod, @lookup(frame, stmt.args[2])::String)
assign_this!(frame, nothing) # FIXME: the file might return something different from `nothing`
pc = next_or_nothing!(frame)
elseif f === Base.Docs.doc! # && mode !== :eval
Expand Down Expand Up @@ -441,3 +439,18 @@ function methods_by_execution!(@nospecialize(recurse), methodinfo, docexprs, fra
end
return isrequired[frame.pc] ? get_return(frame) : nothing
end

# These are separated out to limit the impact of LimitedAccuracy inference problems
# (moving them into short method bodies) which prevent serialization of the inferred code

@noinline function handle_toplevel(@nospecialize(recurse), methodinfo, docexprs, mod, stmt::Expr, mode, skip_include)
local value
for ex in stmt.args
ex isa Expr || continue
value = Base.invoke_in_world(Base.get_world_counter(), methods_by_execution!, recurse, methodinfo, docexprs, mod, ex; mode, skip_include, disablebp=false)
end
return value
end

@noinline handle_eval(@nospecialize(recurse), methodinfo, docexprs, newmod, newex::Expr, mode, skip_include) =
Base.invoke_in_world(Base.get_world_counter(), methods_by_execution!, recurse, methodinfo, docexprs, newmod, newex; mode, skip_include, disablebp=false)
9 changes: 6 additions & 3 deletions src/packagedef.jl
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ and julia objects, and allows re-evaluation of code in the proper module scope.
It is a dictionary indexed by PkgId:
`pkgdatas[id]` returns a value of type [`Revise.PkgData`](@ref).
"""
const pkgdatas = Dict{PkgId,PkgData}(NOPACKAGE => PkgData(NOPACKAGE))
const pkgdatas = Dict{PkgId,PkgData}()

const moduledeps = Dict{Module,DepDict}()
function get_depdict(mod::Module)
Expand Down Expand Up @@ -461,7 +461,7 @@ end
function eval_with_signatures(mod, ex::Expr; mode=:eval, kwargs...)
methodinfo = CodeTrackingMethodInfo(ex)
docexprs = DocExprs()
frame = methods_by_execution!(finish_and_return!, methodinfo, docexprs, mod, ex; mode=mode, kwargs...)[2]
frame = methods_by_execution!(Base.inferencebarrier(finish_and_return!), methodinfo, docexprs, mod, ex; mode=mode, kwargs...)[2]
return methodinfo.allsigs, methodinfo.deps, methodinfo.includes, frame
end

Expand Down Expand Up @@ -1281,6 +1281,8 @@ function __init__()
for m in methods(includet)
push!(JuliaInterpreter.compiled_methods, m)
end
# Add the dummy package for user callbacks
pkgdatas[NOPACKAGE] = PkgData(NOPACKAGE)
# Set up a repository for methods defined at the REPL
id = PkgId(nothing, "@REPL")
pkgdatas[id] = pkgdata = PkgData(id, nothing)
Expand Down Expand Up @@ -1359,9 +1361,10 @@ function setup_atom(atommod::Module)::Nothing
return nothing
end

function add_revise_deps()
function add_revise_deps(skip_revise::Bool=false)
# Populate CodeTracking data for dependencies and initialize watching on code that Revise depends on
for mod in (CodeTracking, OrderedCollections, JuliaInterpreter, LoweredCodeUtils, Revise)
skip_revise && mod === Revise && continue
id = PkgId(mod)
pkgdata = parse_pkg_files(id)
init_watching(pkgdata, srcfiles(pkgdata))
Expand Down
114 changes: 31 additions & 83 deletions src/precompile.jl
Original file line number Diff line number Diff line change
@@ -1,90 +1,38 @@
macro warnpcfail(ex::Expr)
modl = __module__
file = __source__.file === nothing ? "?" : String(__source__.file)
line = __source__.line
quote
$(esc(ex)) || @warn """precompile directive
$($(Expr(:quote, ex)))
failed. Please report an issue in $($modl) (after checking for duplicates) or remove this directive.""" _file=$file _line=$line
end
module var"#Internal"
ftmp() = 1
end

function _precompile_()
ccall(:jl_generating_output, Cint, ()) == 1 || return nothing
# These are blocking so don't actually call them
precompile(Tuple{TaskThunk})
precompile(Tuple{typeof(wait_changed), String})
precompile(Tuple{typeof(revise_dir_queued), String})
precompile(Tuple{typeof(revise_file_queued), PkgData, String})
precompile(Tuple{typeof(watch_manifest), String})
# This excludes Revise itself
precompile(Tuple{typeof(watch_package_callback), PkgId})
# Too complicated to bother
precompile(Tuple{typeof(includet), String})
precompile(Tuple{typeof(track), Module, String})
precompile(Tuple{typeof(get_def), Method})
precompile(Tuple{typeof(entr), Any, Vector{String}})

@warnpcfail precompile(Tuple{TaskThunk})
@warnpcfail precompile(Tuple{typeof(wait_changed), String})
@warnpcfail precompile(Tuple{typeof(watch_package), PkgId})
@warnpcfail precompile(Tuple{typeof(watch_includes), Module, String})
@warnpcfail precompile(Tuple{typeof(watch_manifest), String})
@warnpcfail precompile(Tuple{typeof(revise_dir_queued), String})
@warnpcfail precompile(Tuple{typeof(revise_file_queued), PkgData, String})
@warnpcfail precompile(Tuple{typeof(init_watching), PkgData, Vector{String}})
@warnpcfail precompile(Tuple{typeof(add_revise_deps)})
@warnpcfail precompile(Tuple{typeof(watch_package_callback), PkgId})

@warnpcfail precompile(Tuple{typeof(revise)})
@warnpcfail precompile(Tuple{typeof(revise_first), Expr})
@warnpcfail precompile(Tuple{typeof(includet), String})
@warnpcfail precompile(Tuple{typeof(track), Module, String})
# setindex! doesn't fully precompile, but it's still beneficial to do it
# (it shaves off a bit of the time)
# See https://github.com/JuliaLang/julia/pull/31466
@warnpcfail precompile(Tuple{typeof(setindex!), ExprsSigs, Nothing, RelocatableExpr})
@warnpcfail precompile(Tuple{typeof(setindex!), ExprsSigs, Vector{Any}, RelocatableExpr})
@warnpcfail precompile(Tuple{typeof(setindex!), ModuleExprsSigs, ExprsSigs, Module})
@warnpcfail precompile(Tuple{typeof(setindex!), Dict{PkgId,PkgData}, PkgData, PkgId})
@warnpcfail precompile(Tuple{Type{WatchList}})
@warnpcfail precompile(Tuple{typeof(setindex!), Dict{String,WatchList}, WatchList, String})

MI = CodeTrackingMethodInfo
@warnpcfail precompile(Tuple{typeof(minimal_evaluation!), MI, Core.CodeInfo, Symbol})
@warnpcfail precompile(Tuple{typeof(minimal_evaluation!), Any, MI, Core.CodeInfo, Symbol})
@warnpcfail precompile(Tuple{typeof(methods_by_execution!), Any, MI, DocExprs, Module, Expr})
@warnpcfail precompile(Tuple{typeof(methods_by_execution!), Any, MI, DocExprs, JuliaInterpreter.Frame, Vector{Bool}})
@warnpcfail precompile(Tuple{typeof(Core.kwfunc(methods_by_execution!)),
NamedTuple{(:mode,),Tuple{Symbol}},
typeof(methods_by_execution!), Function, MI, DocExprs, Module, Expr})
@warnpcfail precompile(Tuple{typeof(Core.kwfunc(methods_by_execution!)),
NamedTuple{(:skip_include,),Tuple{Bool}},
typeof(methods_by_execution!), Function, MI, DocExprs, Module, Expr})
@warnpcfail precompile(Tuple{typeof(Core.kwfunc(methods_by_execution!)),
NamedTuple{(:mode, :skip_include),Tuple{Symbol,Bool}},
typeof(methods_by_execution!), Function, MI, DocExprs, Module, Expr})
@warnpcfail precompile(Tuple{typeof(Core.kwfunc(methods_by_execution!)),
NamedTuple{(:mode,),Tuple{Symbol}},
typeof(methods_by_execution!), Function, MI, DocExprs, Frame, Vector{Bool}})
@warnpcfail precompile(Tuple{typeof(Core.kwfunc(methods_by_execution!)),
NamedTuple{(:mode, :skip_include),Tuple{Symbol,Bool}},
typeof(methods_by_execution!), Function, MI, DocExprs, Frame, Vector{Bool}})

mex = which(methods_by_execution!, (Function, MI, DocExprs, Module, Expr))
mbody = bodymethod(mex)
# use `typeof(pairs(NamedTuple()))` here since it actually differs between Julia versions
@warnpcfail precompile(Tuple{mbody.sig.parameters[1], Symbol, Bool, Bool, typeof(pairs(NamedTuple())), typeof(methods_by_execution!), Any, MI, DocExprs, Module, Expr})
@warnpcfail precompile(Tuple{mbody.sig.parameters[1], Symbol, Bool, Bool, Iterators.Pairs{Symbol,Bool,Tuple{Symbol},NamedTuple{(:skip_include,),Tuple{Bool}}}, typeof(methods_by_execution!), Any, MI, DocExprs, Module, Expr})
mfr = which(methods_by_execution!, (Function, MI, DocExprs, Frame, Vector{Bool}))
mbody = bodymethod(mfr)
@warnpcfail precompile(Tuple{mbody.sig.parameters[1], Symbol, Bool, typeof(methods_by_execution!), Any, MI, DocExprs, Frame, Vector{Bool}})

@warnpcfail precompile(Tuple{typeof(hastrackedexpr), Expr})
@warnpcfail precompile(Tuple{typeof(get_def), Method})
@warnpcfail precompile(Tuple{typeof(parse_pkg_files), PkgId})
if isdefined(Revise, :filter_valid_cachefiles)
@warnpcfail precompile(Tuple{typeof(filter_valid_cachefiles), String, Vector{String}})
end
@warnpcfail precompile(Tuple{typeof(pkg_fileinfo), PkgId})
@warnpcfail precompile(Tuple{typeof(push!), WatchList, Pair{String,PkgId}})
@warnpcfail precompile(Tuple{typeof(pushex!), ExprsSigs, Expr})
@warnpcfail precompile(Tuple{Type{ModuleExprsSigs}, Module})
@warnpcfail precompile(Tuple{Type{FileInfo}, Module, String})
@warnpcfail precompile(Tuple{Type{PkgData}, PkgId})
@warnpcfail precompile(Tuple{typeof(Base._deleteat!), Vector{Tuple{Module,String,Float64}}, Vector{Int}})
@warnpcfail precompile(Tuple{typeof(add_require), String, Module, String, String, Expr})
@warnpcfail precompile(Tuple{Core.kwftype(typeof(maybe_add_includes_to_pkgdata!)),NamedTuple{(:eval_now,), Tuple{Bool}},typeof(maybe_add_includes_to_pkgdata!),PkgData,String,Vector{Pair{Module, String}}})

for TT in (Tuple{Module,Expr}, Tuple{DataType,MethodSummary})
@warnpcfail precompile(Tuple{Core.kwftype(typeof(Base.CoreLogging.handle_message)),NamedTuple{(:time, :deltainfo), Tuple{Float64, TT}},typeof(Base.CoreLogging.handle_message),ReviseLogger,LogLevel,String,Module,String,Symbol,String,Int})
end
watch_package(REVISE_ID)
watch_includes(Revise, "src/Revise.jl")
add_revise_deps(true)
revise()
revise_first(:(1+1))
eval_with_signatures(var"#Internal", :(f() = 1))
eval_with_signatures(var"#Internal", :(f2() = 1); skip_include=true)
add_require(pathof(LoweredCodeUtils), LoweredCodeUtils, "295af30f-e4ad-537b-8983-00126c2a3abe", "Revise", :(include("somefile.jl")))
add_require(pathof(JuliaInterpreter), JuliaInterpreter, "295af30f-e4ad-537b-8983-00126c2a3abe", "Revise", :(f(x) = 7))
pkgdata = pkgdatas[PkgId(LoweredCodeUtils)]
eval_require_now(pkgdata, length(pkgdata.info.files), last(pkgdata.info.files)*"__@require__", joinpath(basedir(pkgdata), last(pkgdata.info.files)), Revise, :(var"#Internal".ftmp(::Int) = 0))
# Now empty the stores to prevent them from being serialized
empty!(watched_files)
empty!(watched_manifests)
empty!(pkgdatas)
empty!(included_files)
return nothing
end
2 changes: 1 addition & 1 deletion test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -1200,7 +1200,7 @@ end
frame = Frame(ChangeDocstring, lwr.args[1])
methodinfo = Revise.MethodInfo()
docexprs = Revise.DocExprs()
ret = Revise.methods_by_execution!(JuliaInterpreter.finish_and_return!, methodinfo,
ret = Revise.methods_by_execution!(Base.inferencebarrier(JuliaInterpreter.finish_and_return!), methodinfo,
docexprs, frame, trues(length(frame.framecode.src.code)); mode=:sigs)
ds = @doc(ChangeDocstring.f)
@test get_docstring(ds) == "g"
Expand Down