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

Add specialized * for Rational and Integer #35483

Merged
merged 3 commits into from
Apr 15, 2020

Conversation

garborg
Copy link
Contributor

@garborg garborg commented Apr 14, 2020

Piggybacking off of #35444. Seeing that PR, I realized I have code that would be helped if multiplication got the same treatment. Trying following benchmark:

function mul_spec(x::Rational, y::Integer)
    xd,yn = Base.divgcd(x.den,y)
    Base.checked_mul(x.num,yn) // xd
end
mul_spec(x::Integer, y::Rational) = mul_spec(y, x)

function mk_rats(T, n)
    r = one(T):T(1000)
    Rational.(rand(r, n), rand(r, n))
end

function bench(f, rats)
    r = 1:length(rats)
    maximum(
        f(i, f(i2, f(f(rat, i2), i))).den
        for (rat, i, i2)
        in zip(rats, r, r .+ 1)
    )
end

n = 1_000
for T in (UInt, Int, BigInt)
    rats = mk_rats(T, n)
    @info "Type" T
    @btime bench($(*), $rats)
    @btime bench($mul_spec, $rats)
end

I see the following speedups:

┌ Info: Type
└   T = UInt64
  717.044 μs (2 allocations: 64 bytes)
  480.512 μs (2 allocations: 64 bytes)
┌ Info: Type
└   T = Int64
  753.683 μs (2 allocations: 64 bytes)
  485.841 μs (2 allocations: 64 bytes)
┌ Info: Type
└   T = BigInt
  4.653 ms (145006 allocations: 2.78 MiB)
  2.555 ms (80002 allocations: 1.53 MiB)

There's still a redundant divgcd in Rational's inner constructor.

base/rational.jl Outdated Show resolved Hide resolved
base/rational.jl Outdated Show resolved Hide resolved
@fredrikekre fredrikekre added performance Must go faster rationals The Rational type and values thereof labels Apr 15, 2020
@fredrikekre fredrikekre merged commit 07b7237 into JuliaLang:master Apr 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Must go faster rationals The Rational type and values thereof
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants