From 351b73082023f0c07df1f537fb9d4e59f3ac825c Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Thu, 3 Feb 2022 09:41:39 -0600 Subject: [PATCH 1/4] Simplify precompiles This switches to "do a little work" for precompilation, rather than explicitly spelling out the specific signatures. This should be more maintainable and potentially more exhaustive (or could be made to be so) since it precompiles across runtime dispatch boundaries. This also uses `Base.inferencebarrier` in several places to prevent an argument from being specialized. --- src/callbacks.jl | 6 +-- src/lowered.jl | 6 +-- src/packagedef.jl | 9 ++-- src/precompile.jl | 114 +++++++++++++--------------------------------- test/runtests.jl | 2 +- 5 files changed, 44 insertions(+), 93 deletions(-) diff --git a/src/callbacks.jl b/src/callbacks.jl index b40e5ad6..6ca65a28 100644 --- a/src/callbacks.jl +++ b/src/callbacks.jl @@ -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) @@ -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 @@ -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 diff --git a/src/lowered.jl b/src/lowered.jl index 180f3d3b..5f0535d8 100644 --- a/src/lowered.jl +++ b/src/lowered.jl @@ -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) @@ -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...) return methodinfo, docexprs, frame end @@ -233,7 +233,7 @@ 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 diff --git a/src/packagedef.jl b/src/packagedef.jl index 193f90db..af031609 100644 --- a/src/packagedef.jl +++ b/src/packagedef.jl @@ -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) @@ -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 @@ -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) @@ -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)) diff --git a/src/precompile.jl b/src/precompile.jl index 45ffdd60..f923500f 100644 --- a/src/precompile.jl +++ b/src/precompile.jl @@ -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 __RInternal__ +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(__RInternal__, :(f() = 1)) + eval_with_signatures(__RInternal__, :(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, :(__RInternal__.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 diff --git a/test/runtests.jl b/test/runtests.jl index cfcf275d..da9cd3e1 100644 --- a/test/runtests.jl +++ b/test/runtests.jl @@ -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" From 0c4f0f83103ea54f8fc1d59ff612ef3b8c335d39 Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Fri, 4 Feb 2022 07:04:13 -0600 Subject: [PATCH 2/4] lowered: improve inference of lnn and add_include --- src/lowered.jl | 22 ++++++++++++---------- 1 file changed, 12 insertions(+), 10 deletions(-) diff --git a/src/lowered.jl b/src/lowered.jl index 5f0535d8..810ee5b7 100644 --- a/src/lowered.jl +++ b/src/lowered.jl @@ -237,6 +237,7 @@ methods_by_execution!(methodinfo, docexprs, mod::Module, ex::Expr; 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: @@ -289,9 +290,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 @@ -300,9 +301,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 @@ -335,7 +337,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 @@ -345,7 +347,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 @@ -354,7 +356,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 @@ -391,7 +393,7 @@ function methods_by_execution!(@nospecialize(recurse), methodinfo, docexprs, fra 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 From d49f829cf34054ab9f2b10f63b4fd47ec84f5266 Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Fri, 4 Feb 2022 07:05:14 -0600 Subject: [PATCH 3/4] Fix LimitedAccuracy --- src/lowered.jl | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/src/lowered.jl b/src/lowered.jl index 810ee5b7..22946735 100644 --- a/src/lowered.jl +++ b/src/lowered.jl @@ -255,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]) @@ -385,7 +381,7 @@ 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) @@ -443,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) From cf42ad18e4ac8262387216c5c3153ebd5a7d8249 Mon Sep 17 00:00:00 2001 From: Tim Holy Date: Mon, 7 Feb 2022 15:24:08 -0600 Subject: [PATCH 4/4] Rename private module --- src/precompile.jl | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/precompile.jl b/src/precompile.jl index f923500f..387e0794 100644 --- a/src/precompile.jl +++ b/src/precompile.jl @@ -1,4 +1,4 @@ -module __RInternal__ +module var"#Internal" ftmp() = 1 end @@ -23,12 +23,12 @@ function _precompile_() add_revise_deps(true) revise() revise_first(:(1+1)) - eval_with_signatures(__RInternal__, :(f() = 1)) - eval_with_signatures(__RInternal__, :(f2() = 1); skip_include=true) + 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, :(__RInternal__.ftmp(::Int) = 0)) + 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)