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

Dict setindex! run time dispatch #50837

Closed
nsajko opened this issue Aug 8, 2023 · 5 comments
Closed

Dict setindex! run time dispatch #50837

nsajko opened this issue Aug 8, 2023 · 5 comments
Labels
domain:types and dispatch Types, subtyping and method dispatch kind:regression Regression in behavior compared to a previous version performance Must go faster
Milestone

Comments

@nsajko
Copy link
Contributor

nsajko commented Aug 8, 2023

julia> using JET

julia> @report_opt setindex!(Dict{Nothing,Nothing}(), nothing, nothing)
═════ 2 possible errors found ═════
┌ setindex!(h::Dict{Nothing, Nothing}, v0::Nothing, key::Nothing) @ Base ./dict.jl:375
│┌ ht_keyindex2_shorthash!(h::Dict{Nothing, Nothing}, key::Nothing) @ Base ./dict.jl:333
││┌ rehash!(h::Dict{Nothing, Nothing}, newsz::Int64) @ Base ./dict.jl:208
│││┌ setproperty!(x::Dict{Nothing, Nothing}, f::Symbol, v::UInt64) @ Base ./Base.jl:40
││││ runtime dispatch detected: convert(%1::Type, v::UInt64)::Any
│││└────────────────────
││┌ rehash!(h::Dict{Nothing, Nothing}, newsz::Int64) @ Base ./dict.jl:213
│││┌ setproperty!(x::Dict{Nothing, Nothing}, f::Symbol, v::Int64) @ Base ./Base.jl:40
││││ runtime dispatch detected: convert(%1::Type, v::Int64)::Any
│││└────────────────────


julia> versioninfo()
Julia Version 1.11.0-DEV.239
Commit c40ecd3f03a (2023-08-08 03:06 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Linux (x86_64-linux-gnu)
  CPU: 8 × AMD Ryzen 3 5300U with Radeon Graphics
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, znver2)
  Threads: 1 on 8 virtual cores

The lines 208 and 213 are basically simple BitInteger assignments, so it's weird that they cause run time dispatch.

@nsajko
Copy link
Contributor Author

nsajko commented Aug 8, 2023

Note that the types in the assignment and increment don't match, Int vs UInt, however this still seems like it should be OK due to constant propagation 🤷

@brenhinkeller brenhinkeller added performance Must go faster domain:types and dispatch Types, subtyping and method dispatch labels Aug 8, 2023
@Seelengrab
Copy link
Contributor

Seelengrab commented Aug 8, 2023

FWIW, I cannot reproduce this on a slightly earlier version of master, so that's a recent regression.

julia> @report_opt setindex!(Dict{Nothing,Nothing}(), nothing, nothing)
No errors detected


julia> versioninfo()
Julia Version 1.11.0-DEV.122
Commit 0f56da89883 (2023-07-19 09:09 UTC)
Platform Info:
  OS: Linux (x86_64-pc-linux-gnu)
  CPU: 24 × AMD Ryzen 9 7900X 12-Core Processor
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, znver3)
  Threads: 34 on 24 virtual cores
Environment:
  JULIA_PKG_USE_CLI_GIT = true

@nsajko
Copy link
Contributor Author

nsajko commented Aug 9, 2023

Doesn't repro on ab94fad, but does on 117ef2e. So #50618 caused it, @aviatesk.

@Seelengrab
Copy link
Contributor

Seelengrab commented Aug 9, 2023

Thanks for the bisection! I also can't repro on 1.9.1, so it's truly a regression compared to an earlier release.

@Seelengrab Seelengrab added the kind:regression Regression in behavior compared to a previous version label Aug 9, 2023
@KristofferC KristofferC added this to the 1.11 milestone Aug 9, 2023
@aviatesk
Copy link
Sponsor Member

This seems to be a problem on JET-side. I will update JET and release a new version quickly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain:types and dispatch Types, subtyping and method dispatch kind:regression Regression in behavior compared to a previous version performance Must go faster
Projects
None yet
Development

No branches or pull requests

5 participants