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

Constructor with trivial assert much slower than without #21323

Closed
cstjean opened this issue Apr 8, 2017 · 5 comments
Closed

Constructor with trivial assert much slower than without #21323

cstjean opened this issue Apr 8, 2017 · 5 comments
Labels
compiler:codegen Generation of LLVM IR and native code performance Must go faster

Comments

@cstjean
Copy link
Contributor

cstjean commented Apr 8, 2017

This code takes ~2μs

struct Myo{C}
    c::C
    function (::Type{Myo{C}}){C}(c)
        if 2 == 2   # expansion of @assert 2==2
            nothing
        else 
            throw(Base.AssertionError("2 == 2"))
        end
        return new{C}(c)
    end
end
foo() = Myo{Type{Int}}(Int)
@benchmark foo()

It's 200X slower than the same code with

  • Myo{Int}(2)
  • Taking out the if
  • Replacing the if with the equivalent if 2 != 2 throw(...) end

Meta-question: I know compiler code is full of heuristics, so is it useful to the Julia team to file an issue like this?

@cstjean cstjean changed the title Constructor with trivial assert 200X slower than without Constructor with trivial assert much slower than without Apr 8, 2017
@JeffBezanson
Copy link
Member

Yes, keep the issues coming! :) Always better to know about a possible problem.

@JeffBezanson
Copy link
Member

In Myo{Type{Int}}(Int), the field that points to Int is a reference, so the object is heap allocated. Myo{Int}(2) is stack allocated. However it looks like we are also failing to remove the type check that Int isa Type{Int}, which seems to be a real codegen issue.

@JeffBezanson JeffBezanson added compiler:codegen Generation of LLVM IR and native code performance Must go faster labels Apr 10, 2017
@JeffBezanson
Copy link
Member

JeffBezanson commented Apr 10, 2017

Another thing happening here: the constructor is being specialized for Myo{Type{Int}}(::Type), causing a dynamically-dispatched convert call that takes most of the time. You can work around this by adding a static parameter for the argument (function (::Type{Myo{C}}){C,T}(c::T)).

Internally, we should be able to handle this by speculatively inlining the matching identity conversion method, along with the needed type assertion.

JeffBezanson added a commit that referenced this issue Apr 11, 2017
prompted by #21323

This removes the special case for the first field.
@JeffBezanson
Copy link
Member

This is probably the same dispatch performance issue as #21370, except for convert instead of constructors. convert also has a large number of methods specialized on different types.

@KristofferC
Copy link
Member

julia> @btime foo()
  6.809 ns (1 allocation: 16 bytes)

looks fixed to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:codegen Generation of LLVM IR and native code performance Must go faster
Projects
None yet
Development

No branches or pull requests

3 participants