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

Get this working again, add betray! #10

Open
wants to merge 16 commits into
base: master
Choose a base branch
from

Conversation

MasonProtter
Copy link

@MasonProtter MasonProtter commented Feb 18, 2021

So I've gotten the package working on julia version 1.4+, and I've got a working implementation of the betray! functionality.

One big annoyance that's still present is worldage issues:

julia> using Traitor

julia> begin
           abstract type Size end
           struct Small  <: Size end
           Size(::Union{Type{Int32},Type{Int64}}) = Small
       end
Size

julia> @traitor f(x::::Small) = x + 1
_f{::Any::Tuple{Small}} (generic function with 1 method)

julia> f(1)
2

julia> Size(::Type{Float64}) = Small
Size

julia> f(1.0)
ERROR: MethodError: no method matching Size(::Type{Float64})
The applicable method may be too new: running in world age 29605, while current world is 29607.
Closest candidates are:
  Size(::Type{Float64}) at REPL[5]:1 (method too new to be called from this world context.)
  Size(::Union{Type{Int32}, Type{Int64}}) at REPL[2]:4
Stacktrace:
[...]

However, I believe this can be solved through the injection of backedges into the generated function body. I think the above worldage issue would be solved by just copying the backedges from Traitor.trait_dispatch into the code info for the generated function returned by @traitor.

Another approach that might help us get away from generated functions would be to use https://github.com/RelationalAI-oss/Salsa.jl rather than generated functions to cache, memoize, and reactively update the dispatch lookup tables.

Yet another option would be to use the AbstractInterpreter machinery. I'm really excited about this machinery and would like to learn to use it, but it's still pretty intimidating to me and looks quite difficult to learn to use so I haven't gotten around to it yet.

@MasonProtter
Copy link
Author

CC @timholy if case you have any thoughts or comments.

Copy link
Owner

@andyferris andyferris left a comment

Choose a reason for hiding this comment

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

Wow, thanks for doing this, that is really cool!

However, I believe this can be solved through the injection of backedges into the generated function body. I think the above worldage issue would be solved by just copying the backedges from Traitor.trait_dispatch into the code info for the generated function returned by @traitor.

Yes, I think this is the approach we should take. The idea here was to have fully-compiled code (rather than memoization like Salsa.jl) and also to make a proof-of-principle that could potentially (if the core devs were interested) be integrated into Julia proper.


(See also `Traitor` and `@traitor`.)
Betraying functions is not a very kind thing to do, and can cause unintended side effects.
Copy link
Owner

Choose a reason for hiding this comment

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

Do you have examples of these unintended side effects?

Copy link
Author

@MasonProtter MasonProtter Feb 19, 2021

Choose a reason for hiding this comment

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

One example would be if you betray!'d Traitor.get_trait_table, you would delete the trait methods for a given signature because it'd have to recompile so the dispatch dict would get emptied.

Presumably, that'd be a pretty unfortunate unintended consequence.

them to be compatible with traits-based dispatch, so that new submethods
defined by `@traitor` do not overwrite existing default methods (and used where
no more specific trait match is found).
Et tu?
Copy link
Owner

Choose a reason for hiding this comment

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

Lol.

end
# TODO: make this work with TT of the form (Tuple{T, T} where T)

m = (last ∘ collect ∘ methods)(f, TT)
Copy link
Owner

Choose a reason for hiding this comment

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

Why last? Should it be only?

Also, purely out of curiousity, do you find this easier to read than last(collect(methods(f, TT)))?

Copy link
Author

Choose a reason for hiding this comment

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

I use last because methods will give all the more specific methods as well as the one you ask for. For example,

julia> methods(sin, Tuple{Real})
# 4 methods for generic function "sin":
[1] sin(a::Float16) in Base.Math at math.jl:1159
[2] sin(x::T) where T<:Union{Float32, Float64} in Base.Math at special/trig.jl:29
[3] sin(x::BigFloat) in Base.MPFR at mpfr.jl:728
[4] sin(x::Real) in Base.Math at math.jl:404

Here, we just wanted the last one.

Also, purely out of curiousity, do you find this easier to read than last(collect(methods(f, TT)))?

Slightly yeah. Even with coloured parens, I find it hard for my eye to quickly verify that they're all matched once you have that many nested calls. It's not a huge deal either way for me though.

@MasonProtter
Copy link
Author

MasonProtter commented Feb 19, 2021

However, I believe this can be solved through the injection of backedges into the generated function body. I think the above worldage issue would be solved by just copying the backedges from Traitor.trait_dispatch into the code info for the generated function returned by @traitor.

Yes, I think this is the approach we should take. The idea here was to have fully-compiled code (rather than memoization like Salsa.jl) and also to make a proof-of-principle that could potentially (if the core devs were interested) be integrated into Julia proper.

Hm, from that POV then I think the AbstractInterpreter machinery would be the way to go because it'd be using the same API that julia already uses for compilation and deciding dispatch. I can't imagine tje Compiler team being at all enthusiastic about a PR to Core.Compiler that uses generated functions to decide dispatch.

Either way though, I'll get the backedge injection route working first because I understand it better.

@AriMKatz
Copy link

AriMKatz commented Feb 19, 2021

There are a couple of packages that subtype AbstractInterpreter to good effect and your interest in the same had me ask Keno how they'd play together. Turns out they don't. Subtyping AI is apparently a temporary hack until the higher level compiler interfaces can get figured out. Obviously the ideal route is through compiler passes, but given that information (this may clash or have undefined behavior with GPUCompiler etc) not sure if that changes how you want to proceed.

@0x0f0f0f 's metatheory.jl is one candidate for this higher level compiler infra, which might shift to integrate with SSA IR.

@AriMKatz
Copy link

relevant JuliaLang/julia#39697

@MasonProtter
Copy link
Author

MasonProtter commented Feb 19, 2021

Okay, so the good news is that I managed to get the invalidation machinery to cooperate and recompile stuff as needed:

julia> using Traitor

julia> begin
           abstract type Size end
           struct Small  <: Size end
           Size(::Union{Type{Int32},Type{Int64}}) = Small
       end
Size

julia> @traitor f(x::::Small) = x + 1
f (generic function with 1 method)

julia> f(1)
2

julia> Size(::Type{Float64}) = Small
Size

julia> f(1.0)
2.0

The bad new is that I can't get it to work without a Cassette overdub pass through trait_dispatch that doesn't actually do anything. I was debugging stuff with a cassette pass using some code from https://github.com/NHDaly/StagedFunctions.jl/, and suddenly I got it to work. Without the pass, it wouldn't work. Then I made the pass literally do nothing and it still worked.

I suspect that maybe the pass is forcing some specializations that the compiler was giving up on or something. I'm not sure and I've given up for now on figuring it out. So for now, my branch depends on Cassette.jl.

This is weird.

@NHDaly, you wouldn't happen to know what's going on here, would you? The code in

Traitor.jl/src/Traitor.jl

Lines 279 to 356 in 98ded52

$Traitor.@generated function $funcname($(args...))
dict = Traitor.get_trait_table($funcname, $(Expr(:curly, :Tuple, argtypes...)))
thunk = () -> Traitor.trait_dispatch(dict, $(Expr(:curly, :Tuple, argnames...)))
# This cassette overdub pass literally does nothing other than normal evaluation.
# However, if I don't use it, the backedge attachment later on doesn't appear
# to have the desired effect.
$dispatchname = $Cassette.overdub($DoNothingCtx(), thunk)
#$dispatchname = thunk()
ex = (Expr(:call, $dispatchname, $(QuoteNode.(argnames)...)))
# Create a codeinfo
ci = $expr_to_codeinfo($(__module__), [Symbol("#self#"), $(quotednames...)], [], (), ex)
# Attached edges from MethodInstrances of the `supertrait` function to to this CodeInfo.
# This should make it so that adding members to a trait relevant to this function
# triggers recompilation, fixing the #265 equivalent for trait methods.
ci.edges = $Core.MethodInstance[]
for TT in [$(traits...)]
for T in TT.parameters
for mi in $Core.Compiler.method_instances($supertrait, Tuple{Type{T}})
push!(ci.edges, mi)
end
end
end
return ci
end
$internalname($(args...)) = $body
local d = $Traitor.get_trait_table($funcname, $(Expr(:curly, :Tuple, argtypes...)))
d[Tuple{$(traits...)}] = $internalname
$funcname
end
esc(ex)
end
"""
expr_to_codeinfo(m::Module, argnames, spnames, sp, e::Expr)
Take an expr (usually a generated function generator) and convert it into a CodeInfo object
(Julia's internal, linear representation of code).
`m` is the module that the CodeInfo should be generated from (used for name resolution)
`argnames` must be an iterable container of symbols describing the CodeInfo's input arguments.
NOTE: the first argument should be given as `Symbol("#self#")`. So if the function is `f(x) = x + 1`,
then `argnames = [Symbol("#self#"), :x]`
`spnames` should be an iterable container of the names of the static parameters to the CodeInfo body
(e.g.) in `f(x::T) where {T <: Int} = ...`, `T` is a static parameter, so `spnames` should be `[:T]`
`sp` should be an iterable container of the static parameters to the CodeInfo body themselves (as
opposed to their names) (e.g.) in `f(x::T) where {T <: Int} = ...`, `T` is a static parameter,
so `sp` should be `[T]`
`e` is the actual expression to lower to CodeInfo. This must be 'pure' in the same sense as generated
function bodies.
"""
function expr_to_codeinfo(m::Module, argnames, spnames, sp, e::Expr)
lam = Expr(:lambda, argnames,
Expr(Symbol("scope-block"),
Expr(:block,
Expr(:return,
Expr(:block,
e,
)))))
ex = if spnames === nothing || isempty(spnames)
lam
else
Expr(Symbol("with-static-parameters"), lam, spnames...)
end
# Get the code-info for the generatorbody in order to use it for generating a dummy
# code info object.
ci = ccall(:jl_expand_and_resolve, Any, (Any, Any, Core.SimpleVector), ex, m, Core.svec(sp...))
@assert ci isa Core.CodeInfo "Failed to create a CodeInfo from the given expression. This might mean it contains a closure or comprehension?\n Offending expression: $e"
ci
end
is essentially a pruned version of what's in StagedFunctions.jl

@timholy
Copy link
Collaborator

timholy commented Feb 19, 2021

Kinda swamped but just chiming in to say exciting stuff! @andyferris is more than capable of handling this but I will look sometime (before or after merging).

@MasonProtter
Copy link
Author

Not a problem Tim, I know you're a busy guy!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants