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

use jl_gf_invoke_lookup in invoke_tfunc to lookup the method to be called (invoke improvement No. 2) #11005

Merged
merged 1 commit into from
May 28, 2015

Conversation

yuyichao
Copy link
Contributor

This is the second one from the break down of my old PR (#9642) to improve the performance of invoke. The original commit was here.

This particular commit might not have much impact on the performance but IMHO, since we already have jl_gf_invoke_lookup to find the method to be called by invoke, we might as well use it to simplify the code.

@yuyichao yuyichao force-pushed the invoke-tfunc-lookup branch 3 times, most recently from 7db6e43 to f4c40e9 Compare April 26, 2015 18:54
@yuyichao
Copy link
Contributor Author

Not sure how to come up with a practical/reasonable benchmark for this but here's a comparason of the original and patched version.
The test shows a performance improve of 2-10x and a allocation reduction of 4-15x depending on how many methods there are. (It's true though that this is only compile time performance so it will not give a similar speed up at runtime).

Script

#!/usr/bin/julia -f

# Original version
function invoke_tfunc1(f, types, argtype)
    argtype = typeintersect(types, Base.limit_tuple_type(argtype))
    if is(argtype, Base.Bottom)
        return Base.Bottom
    end
    applicable = Base._methods(f, types, -1)
    if isempty(applicable)
        return Any
    end
    for (m::SimpleVector) in applicable
        local linfo
        try
            linfo = Base.func_for_method(m[3],types,m[2])
        catch
            return Any
        end
        if Base.typeseq(m[1], types)
            tvars = m[2][1:2:end]
            (ti, env) = ccall(:jl_match_method, Any, (Any, Any, Any),
                              argtype, m[1], tvars)::SimpleVector
            (_tree, rt) = Base.typeinf(linfo, ti, env, linfo)
            return rt
        end
    end
    return Any
end

# New version
function invoke_tfunc2(f, types, argtype)
    argtype = typeintersect(types, Base.limit_tuple_type(argtype))
    if is(argtype, Base.Bottom)
        return Base.Bottom
    end
    meth = ccall(:jl_gf_invoke_lookup, Any, (Any, Any), f, types)
    if is(meth, nothing)
        return Any
    end
    (ti, env) = ccall(:jl_match_method, Any, (Any, Any, Any),
                      argtype, meth.sig, meth.tvars)::SimpleVector
    local linfo
    try
        linfo = Base.func_for_method(meth, types, env)
    catch
        return Any
    end
    (_tree, rt) = Base.typeinf(linfo, ti, env, linfo)
    return rt
end

function timing(f, args...)
    println(f, args)
    f(args...)
    gc()
    @time for i in 1:1000000
        f(args...)
    end
    gc()
end

function test_invoke_tfunc(f, types, argtypes)
    println("methods number of $f: $(length(methods(f)))")
    @assert invoke_tfunc1(f, types, argtypes) == invoke_tfunc2(f, types,
                                                               argtypes)
    timing(invoke_tfunc1, f, types, argtypes)
    timing(invoke_tfunc2, f, types, argtypes)
    println()
end

f() = nothing
test_invoke_tfunc(f, Tuple{}, Tuple{})
test_invoke_tfunc(sin, Tuple{Real}, Tuple{Int})
test_invoke_tfunc(+, Tuple{Number, Number}, Tuple{Int, Int})

Output

methods number of f: 1
invoke_tfunc1(f,Tuple{},Tuple{})
elapsed time: 2.799114387 seconds (564 MB allocated, 1.27% gc time in 25 pauses with 0 full sweep)
invoke_tfunc2(f,Tuple{},Tuple{})
elapsed time: 1.202795036 seconds (122 MB allocated, 0.27% gc time in 5 pauses with 0 full sweep)

methods number of sin: 11
invoke_tfunc1(sin,Tuple{Real},Tuple{Int64})
elapsed time: 6.125865963 seconds (1220 MB allocated, 0.72% gc time in 55 pauses with 0 full sweep)
invoke_tfunc2(sin,Tuple{Real},Tuple{Int64})
elapsed time: 1.456254345 seconds (122 MB allocated, 0.15% gc time in 5 pauses with 0 full sweep)

methods number of +: 139
invoke_tfunc1(+,Tuple{Number,Number},Tuple{Int64,Int64})
elapsed time: 31.479080662 seconds (5615 MB allocated, 0.58% gc time in 256 pauses with 0 full sweep)
invoke_tfunc2(+,Tuple{Number,Number},Tuple{Int64,Int64})
elapsed time: 3.173698599 seconds (381 MB allocated, 0.29% gc time in 17 pauses with 0 full sweep)

Edit: change the code in the PR to a slightly shorter version.
Edit2: actually I'm a little surprised that it makes such a big difference and my main goal was only to remove duplicated logic and make the code shorter........

@yuyichao yuyichao force-pushed the invoke-tfunc-lookup branch 4 times, most recently from 07842c8 to 0296515 Compare April 28, 2015 04:31
@yuyichao yuyichao changed the title use jl_gf_invoke_lookup in invoke_tfunc to lookup the method to be called use jl_gf_invoke_lookup in invoke_tfunc to lookup the method to be called (invoke improvement No. 2) May 6, 2015
@yuyichao
Copy link
Contributor Author

Ping. Can anyone review this please?

For performance, this one should be much faster.
For correctness, the new one should by definition be more correct since it is using the same function the real invoke uses for method lookup.

@tkelman
Copy link
Contributor

tkelman commented May 19, 2015

@vtjnash maybe?

@yuyichao
Copy link
Contributor Author

Ping again? @vtjnash

Hopefully weekly is not too much.

@vtjnash
Copy link
Sponsor Member

vtjnash commented May 25, 2015

Lgtm. @JeffBezanson

JeffBezanson added a commit that referenced this pull request May 28, 2015
use `jl_gf_invoke_lookup` in `invoke_tfunc` to lookup the method to be called (`invoke` improvement No. 2)
@JeffBezanson JeffBezanson merged commit 6d9b50f into JuliaLang:master May 28, 2015
@yuyichao yuyichao deleted the invoke-tfunc-lookup branch May 28, 2015 11:19
@yuyichao yuyichao added the performance Must go faster label Jun 30, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Must go faster
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants