Skip to content

Compiler: avoid type instability in access to PartialStruct field#57553

Merged
giordano merged 2 commits intoJuliaLang:masterfrom
nsajko:Compiler_PartialStruct_undef_field_access
Feb 28, 2025
Merged

Compiler: avoid type instability in access to PartialStruct field#57553
giordano merged 2 commits intoJuliaLang:masterfrom
nsajko:Compiler_PartialStruct_undef_field_access

Conversation

@nsajko
Copy link
Copy Markdown
Member

@nsajko nsajko commented Feb 27, 2025

The Base.getproperty method for PartialStruct has a type assert to ensure type stable access of the field undef. However Compiler.jl has Compiler.getproperty === Core.getfield.

Introduce a getter for this field of PartialStruct into Compiler.jl and use it.

I guess this should improve compiler performance, and it should help avoid spurious invalidation.

@nsajko nsajko requested a review from aviatesk February 27, 2025 15:49
@nsajko nsajko force-pushed the Compiler_PartialStruct_undef_field_access branch from f73a76a to ab72c5f Compare February 27, 2025 15:50
@nsajko nsajko added the compiler:inference Type inference label Feb 27, 2025
@nsajko
Copy link
Copy Markdown
Member Author

nsajko commented Feb 27, 2025

help avoid spurious invalidation

using SnoopCompileCore
invs = @snoop_invalidations begin
    struct I <: Integer end
    Base.Int64(::I) = 7
end
display(length(invs))  # before: 5451, after: 2131

Copy link
Copy Markdown
Member

@aviatesk aviatesk left a comment

Choose a reason for hiding this comment

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

Good catch, do you want to merge this before #57541?

The `Base.getproperty` method for `PartialStruct` has a type assert to
ensure type stable access of the field `undef`. However Compiler.jl
has `Compiler.getproperty === Core.getfield`.

Introduce a getter for this field of `PartialStruct` into Compiler.jl
and use it.

I guess this should improve compiler performance, and it should help
avoid spurious invalidation.
@nsajko nsajko force-pushed the Compiler_PartialStruct_undef_field_access branch from 4c45fa3 to 105ffb9 Compare February 27, 2025 17:31
@nsajko
Copy link
Copy Markdown
Member Author

nsajko commented Feb 27, 2025

Good catch

Thanks!

do you want to merge this before #57541?

That would be nice 😃

@nsajko nsajko added the merge me PR is reviewed. Merge when all tests are passing label Feb 27, 2025
@giordano giordano merged commit f5ce249 into JuliaLang:master Feb 28, 2025
8 checks passed
@giordano giordano removed the merge me PR is reviewed. Merge when all tests are passing label Feb 28, 2025
@nsajko nsajko deleted the Compiler_PartialStruct_undef_field_access branch February 28, 2025 10:36
serenity4 pushed a commit to serenity4/julia that referenced this pull request May 1, 2025
…uliaLang#57553)

The `Base.getproperty` method for `PartialStruct` has a type assert to
ensure type stable access of the field `undef`. However Compiler.jl has
`Compiler.getproperty === Core.getfield`.

Introduce a getter for this field of `PartialStruct` into Compiler.jl
and use it.

I guess this should improve compiler performance, and it should help
avoid spurious invalidation.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants