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.

Sign up for GitHub

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

Jump to bottom

Weird dynamic dispatch #2

Closed
quinnj opened this issue Feb 23, 2023 · 1 comment
Closed

Weird dynamic dispatch #2

quinnj opened this issue Feb 23, 2023 · 1 comment

Comments

@quinnj
Copy link
Owner

quinnj commented Feb 23, 2023

I can't quite pin down what's going on in this case, but we have some unknown stackframes/functions getting inserted when calling _materialize which then calls materialize. I'm somewhat suspicious there's a real Julia compiler bug here when a @generated function calls a @noinline which then calls an @inline.

image
@quinnj
Copy link
Owner Author

quinnj commented Feb 23, 2023

Steps to reproduce:

using JSONBase
struct A
    a::Int
    b::Int
    c::Int
    d::Int
end
JSONBase.materialize("""{ "a": 1,"b": 2,"c": 3,"d": 4}""", A)

aviatesk added a commit to aviatesk/JSONBase.jl that referenced this issue Mar 19, 2023
@aviatesk
add a hack to avoid inference recursion limit to allow more optimization
f004e51 
For this target:
```julia
using JSONBase, BenchmarkTools
struct A
    a::Int
    b::Int
    c::Int
    d::Int
end
@benchmark JSONBase.materialize("""{ "a": 1, "b": 2, "c": 3, "d": 4}""", A)
```

Before:
```julia
BenchmarkTools.Trial: 10000 samples with 331 evaluations.
 Range (min … max):  258.937 ns …   4.373 μs  ┊ GC (min … max): 0.00% … 91.32%
 Time  (median):     279.202 ns               ┊ GC (median):    0.00%
 Time  (mean ± σ):   291.054 ns ± 185.340 ns  ┊ GC (mean ± σ):  3.24% ±  4.74%

           ▃█▃    ▂
  ▁▂▂▂▁▁▁▁▃███▃▂▂▄█▇▅▅▅▄▂▂▂▃▃▄▃▂▂▂▂▂▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁ ▂
  259 ns           Histogram: frequency by time          334 ns <

 Memory estimate: 384 bytes, allocs estimate: 10.
```

After:
```julia
BenchmarkTools.Trial: 10000 samples with 750 evaluations.
 Range (min … max):  173.333 ns …  1.618 μs  ┊ GC (min … max): 0.00% … 86.43%
 Time  (median):     179.389 ns              ┊ GC (median):    0.00%
 Time  (mean ± σ):   184.518 ns ± 58.847 ns  ┊ GC (mean ± σ):  1.40% ±  3.86%

          ▂▄▆██▇▅▄▁▁ ▂▃▄▅▄▄▄▄▂▁▁▁▂▂▂▂▂▂▂▂▂▂▁                   ▂
  ▂▅▄▅▅▃▃█████████████████████████████████████████████▇▇▅▄▅▆▅▅ █
  173 ns        Histogram: log(frequency) by time       200 ns <

 Memory estimate: 128 bytes, allocs estimate: 2.
```

closes quinnj#2
@aviatesk aviatesk mentioned this issue Mar 19, 2023
Closed
aviatesk added a commit to aviatesk/JSONBase.jl that referenced this issue Mar 27, 2023
@aviatesk
add a hack to avoid inference recursion limit to allow more optimization
e0c0731 
For this target:
```julia
using JSONBase, BenchmarkTools
struct A
    a::Int
    b::Int
    c::Int
    d::Int
end
@benchmark JSONBase.materialize("""{ "a": 1, "b": 2, "c": 3, "d": 4}""", A)
```

Before:
```julia
BenchmarkTools.Trial: 10000 samples with 331 evaluations.
 Range (min … max):  258.937 ns …   4.373 μs  ┊ GC (min … max): 0.00% … 91.32%
 Time  (median):     279.202 ns               ┊ GC (median):    0.00%
 Time  (mean ± σ):   291.054 ns ± 185.340 ns  ┊ GC (mean ± σ):  3.24% ±  4.74%

           ▃█▃    ▂
  ▁▂▂▂▁▁▁▁▃███▃▂▂▄█▇▅▅▅▄▂▂▂▃▃▄▃▂▂▂▂▂▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁ ▂
  259 ns           Histogram: frequency by time          334 ns <

 Memory estimate: 384 bytes, allocs estimate: 10.
```

After:
```julia
BenchmarkTools.Trial: 10000 samples with 750 evaluations.
 Range (min … max):  173.333 ns …  1.618 μs  ┊ GC (min … max): 0.00% … 86.43%
 Time  (median):     179.389 ns              ┊ GC (median):    0.00%
 Time  (mean ± σ):   184.518 ns ± 58.847 ns  ┊ GC (mean ± σ):  1.40% ±  3.86%

          ▂▄▆██▇▅▄▁▁ ▂▃▄▅▄▄▄▄▂▁▁▁▂▂▂▂▂▂▂▂▂▂▁                   ▂
  ▂▅▄▅▅▃▃█████████████████████████████████████████████▇▇▅▄▅▆▅▅ █
  173 ns        Histogram: log(frequency) by time       200 ns <

 Memory estimate: 128 bytes, allocs estimate: 2.
```

closes quinnj#2
@quinnj quinnj mentioned this issue Jul 31, 2023
Open
quinnj pushed a commit that referenced this issue Dec 21, 2023
@aviatesk @quinnj
add a hack to avoid inference recursion limit to allow more optimization
b8c4cb5 
For this target:
```julia
using JSONBase, BenchmarkTools
struct A
    a::Int
    b::Int
    c::Int
    d::Int
end
@benchmark JSONBase.materialize("""{ "a": 1, "b": 2, "c": 3, "d": 4}""", A)
```

Before:
```julia
BenchmarkTools.Trial: 10000 samples with 331 evaluations.
 Range (min … max):  258.937 ns …   4.373 μs  ┊ GC (min … max): 0.00% … 91.32%
 Time  (median):     279.202 ns               ┊ GC (median):    0.00%
 Time  (mean ± σ):   291.054 ns ± 185.340 ns  ┊ GC (mean ± σ):  3.24% ±  4.74%

           ▃█▃    ▂
  ▁▂▂▂▁▁▁▁▃███▃▂▂▄█▇▅▅▅▄▂▂▂▃▃▄▃▂▂▂▂▂▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁ ▂
  259 ns           Histogram: frequency by time          334 ns <

 Memory estimate: 384 bytes, allocs estimate: 10.
```

After:
```julia
BenchmarkTools.Trial: 10000 samples with 750 evaluations.
 Range (min … max):  173.333 ns …  1.618 μs  ┊ GC (min … max): 0.00% … 86.43%
 Time  (median):     179.389 ns              ┊ GC (median):    0.00%
 Time  (mean ± σ):   184.518 ns ± 58.847 ns  ┊ GC (mean ± σ):  1.40% ±  3.86%

          ▂▄▆██▇▅▄▁▁ ▂▃▄▅▄▄▄▄▂▁▁▁▂▂▂▂▂▂▂▂▂▂▁                   ▂
  ▂▅▄▅▅▃▃█████████████████████████████████████████████▇▇▅▄▅▆▅▅ █
  173 ns        Histogram: log(frequency) by time       200 ns <

 Memory estimate: 128 bytes, allocs estimate: 2.
```

closes #2
@quinnj quinnj closed this as completed in e1c8166 Dec 21, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

add a hack to avoid inference recursion limit to allow more optimization aviatesk/JSONBase.jl
1 participant
@quinnj