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

regression with getfield access on global objects #50870

Closed
rfourquet opened this issue Aug 10, 2023 · 2 comments
Closed

regression with getfield access on global objects #50870

rfourquet opened this issue Aug 10, 2023 · 2 comments
Labels
performance Must go faster regression Regression in behavior compared to a previous version regression 1.9 Regression in the 1.9 release

Comments

@rfourquet
Copy link
Member

rfourquet commented Aug 10, 2023

Compared to v1.8:

  • v1.9.2 has a regression where a.xx allocates (whereas getfield(a, :xx) doesn't)
  • v1.10.0-beta1 has a regression where both a[1].xx and getfield(a[1], :xx) allocate when a is a a tuple

The following script should show the problem:

using BenchmarkTools

struct XX
    xx::Vector{Int}
end

const XXs = XX(Int[])

get1() = length(getfield(XXs, :xx))
get2() = length(XXs.xx)

const YYs = (XX(Int[]),)

get3() = length(getfield(YYs[1], :xx))
get4() = length(YYs[1].xx)

@btime get1()
@btime get2()
@btime get3()
@btime get4()
nothing

Then:

julia> include("tmp/julia/getfield.jl") # v1.8.5
  1.923 ns (0 allocations: 0 bytes)
  1.924 ns (0 allocations: 0 bytes)
  1.923 ns (0 allocations: 0 bytes)
  1.923 ns (0 allocations: 0 bytes)

julia> include("tmp/julia/getfield.jl") # v1.9.2
  1.923 ns (0 allocations: 0 bytes)
  12.877 ns (1 allocation: 16 bytes)
  2.134 ns (0 allocations: 0 bytes)
  12.927 ns (1 allocation: 16 bytes)

julia> include("tmp/julia/getfield.jl") # v1.10.0-beta1
  1.953 ns (0 allocations: 0 bytes)
  2.815 ns (0 allocations: 0 bytes)
  15.269 ns (1 allocation: 16 bytes)
  17.970 ns (1 allocation: 16 bytes)
@rfourquet rfourquet added performance Must go faster regression Regression in behavior compared to a previous version regression 1.9 Regression in the 1.9 release labels Aug 10, 2023
@gbaraldi
Copy link
Member

This is fixed on master and the fix is getting backported to both 1.9 and 1.10 ;)
#50523

@rfourquet
Copy link
Member Author

This is fixed on master and the fix is getting backported to both 1.9 and 1.10 ;)

Ah cool! I was compiling master to try it out but then forgot about it ;-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Must go faster regression Regression in behavior compared to a previous version regression 1.9 Regression in the 1.9 release
Projects
None yet
Development

No branches or pull requests

2 participants