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

optimize field access when field types don't depend on parameters #21620

Merged
merged 1 commit into from
May 2, 2017

Conversation

JeffBezanson
Copy link
Member

Example:

mutable struct RemoteChannel{T<:AbstractChannel} <: AbstractRemoteRef
    where::Int
    whence::Int
    id::Int
end

This change generates better code when we only know the type of something is RemoteChannel. It also computes a layout for the wrapper of such types, and shares it for all instances. We just need to make sure we never assume t->layout != NULL means leaftype, but I think we are pretty good about that. In any case this should be checked over carefully before merging this.

@nanosoldier runbenchmarks(ALL, vs = ":master")

@JeffBezanson JeffBezanson added compiler:codegen Generation of LLVM IR and native code performance Must go faster labels Apr 28, 2017
@ararslan ararslan requested a review from yuyichao April 28, 2017 22:01
@nanosoldier
Copy link
Collaborator

Your benchmark job has completed - possible performance regressions were detected. A full report can be found here. cc @jrevels

@JeffBezanson JeffBezanson requested a review from vtjnash April 30, 2017 19:24
Copy link
Member

@vtjnash vtjnash left a comment

Choose a reason for hiding this comment

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

We just need to make sure we never assume t->layout != NULL means leaftype

I think we should be good at this, since we already will fill in layout for other non-leaftypes such as bitstypes and singletons. I think we just need to be careful that we are aware that now T->layout will apply to all subtypes of T. Perhaps worth putting a comment to that effect somewhere? (Since it means we are now depending on the type-system always returning correct answers for isleaftype)

This may need support from dump.c to avoid making copies of them in the sysimg, but this shouldn't be any worse then we do now.

src/datatype.c Outdated
w->layout) {
// if layout doesn't depend on type parameters, reuse it
st->layout = w->layout;
st->size =w->size;
Copy link
Member

Choose a reason for hiding this comment

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

formatting

also computes layout and shares it for all instantiations for such types
@JeffBezanson JeffBezanson merged commit 3a181fe into master May 2, 2017
@JeffBezanson JeffBezanson deleted the jb/nonleaf_fields branch May 2, 2017 02:52
vtjnash added a commit that referenced this pull request May 19, 2017
would segfault when reaching the jl_datatype_nfields call
vtjnash added a commit that referenced this pull request May 22, 2017
would segfault when reaching the jl_datatype_nfields call
vtjnash added a commit that referenced this pull request May 22, 2017
would segfault when reaching the jl_datatype_nfields call
vtjnash added a commit that referenced this pull request May 22, 2017
would segfault when reaching the jl_datatype_nfields call
vtjnash added a commit that referenced this pull request Jun 8, 2017
vtjnash added a commit that referenced this pull request Jun 9, 2017
…n field types don't depend on parameters

fixes the bug in PR #21620 correctly
vtjnash added a commit that referenced this pull request Jun 10, 2017
tkelman pushed a commit that referenced this pull request Jun 17, 2017
would segfault when reaching the jl_datatype_nfields call

(cherry picked from commit 8cc5e2b)
small piece of #22019
tkelman pushed a commit that referenced this pull request Jun 17, 2017
…n field types don't depend on parameters

fixes the bug in PR #21620 correctly

(cherry picked from commit 6ab0f0a)
ref #22282

fix test case to use 4 space indent instead of 3
(cherry picked from commit ff2ad4e)
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

Successfully merging this pull request may close these issues.

3 participants