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

lower new() to reference the called object instead of re-creating it with apply_type #44664

Merged
merged 1 commit into from
Apr 26, 2022

Conversation

JeffBezanson
Copy link
Member

addresses #36384

This makes the lowering change suggested there, but I don't see any performance differences yet. However this should go nicely together with #44656.

@JeffBezanson JeffBezanson added the compiler:lowering Syntax lowering (compiler front end, 2nd stage) label Mar 18, 2022
@Keno
Copy link
Member

Keno commented Mar 18, 2022

but I don't see any performance differences yet

That's because you're lowering to:

(T::Type{Val{X}})() where {X} = Expr(:new, :T)

when the fast version is

(T::Type{Val{X}} where X)() = Expr(:new, :T)

Of course ideally the system would see that the typevar is unused and move it for us.

@JeffBezanson
Copy link
Member Author

JeffBezanson commented Mar 18, 2022

Found this bug along the way:

julia> struct X{T}
           X()::Int = new{Int}()
       end

julia> X()
X{Int64}()

Processing constructors is hard enough that we were just dropping the type declaration!

@Keno
Copy link
Member

Keno commented Mar 18, 2022

Whoops.

@JeffBezanson JeffBezanson force-pushed the jb/innerctorlowering branch from c3ca854 to 7c36bb5 Compare March 21, 2022 21:32
@JeffBezanson
Copy link
Member Author

Ok hopefully works now, and I see a speedup.

@Keno
Copy link
Member

Keno commented Mar 21, 2022

Can confirm that this addresses the performance issue.

@Keno Keno closed this Mar 21, 2022
@Keno Keno reopened this Mar 21, 2022
@vtjnash vtjnash added the needs nanosoldier run This PR should have benchmarks run on it label Mar 22, 2022
@JeffBezanson
Copy link
Member Author

The subarray test failure on linux64 is weird; can't reproduce locally.

@Keno
Copy link
Member

Keno commented Mar 22, 2022

The subarray test failure on linux64 is weird; can't reproduce locally.

#44635

src/julia-syntax.scm Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
compiler:lowering Syntax lowering (compiler front end, 2nd stage) needs nanosoldier run This PR should have benchmarks run on it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants