update for 1.8 #516
update for 1.8 #516
Conversation
| cconvert_stmt = unsafe_convert_expr.args[3]::SSAValue | ||
| push!(delete_idx, cconvert_stmt.id) # delete the cconvert | ||
| cconvert_expr = code.code[cconvert_stmt.id]::Expr | ||
| push!(args, cconvert_expr.args[3]) | ||
| cconvert_val = unsafe_convert_expr.args[3] | ||
| if isa(cconvert_val, SSAValue) | ||
| push!(delete_idx, cconvert_val.id) # delete the cconvert | ||
| newarg = (code.code[cconvert_val.id]::Expr).args[3] | ||
| push!(args, newarg) | ||
| else | ||
| @assert isa(cconvert_val, SlotNumber) | ||
| push!(args, cconvert_val) | ||
| end |
There was a problem hiding this comment.
I'm actually not too sure why we need this dead code elimination of unsafe_converts. Maybe could @KristofferC clarify the reason?
There was a problem hiding this comment.
Not sure. Maybe it didn't get eliminated all the time. Or I was trying to be smart.
There was a problem hiding this comment.
If we remove those conversions, the same conversion will be applied anyway within a generated compiled call. I'd like to just delete these DCE because of its maintenance cost.
There was a problem hiding this comment.
Did you do any benchmarks whether removing them might hurt performance? Or does it just always get removed by the optimizer anyways?
There was a problem hiding this comment.
The following simple benchmark shows that this DCE is certainly useful to optimize performance:
~/julia/packages/JuliaInterpreter master
❯ julia -e 'using JuliaInterpreter, BenchmarkTools; display(@benchmark @interpret joinpath("/home/julia/base", "sysimg.jl"))'
BenchmarkTools.Trial: 2025 samples with 1 evaluation.
Range (min … max): 2.045 ms … 15.942 ms ┊ GC (min … max): 0.00% … 80.39%
Time (median): 2.274 ms ┊ GC (median): 0.00%
Time (mean ± σ): 2.463 ms ± 677.894 μs ┊ GC (mean ± σ): 2.99% ± 7.99%
▅█▇▃▂
██████▇▆▅▄▄▄▃▄▄▄▃▄▃▃▃▃▃▂▂▂▂▂▂▂▂▂▁▁▁▁▁▁▁▂▁▁▁▁▁▁▁▂▂▂▂▂▂▂▂▂▂▁▂ ▃
2.05 ms Histogram: frequency by time 5.32 ms <
Memory estimate: 1.18 MiB, allocs estimate: 23941.
~/julia/packages/JuliaInterpreter avi/update 20s
❯ julia -e 'using JuliaInterpreter, BenchmarkTools; display(@benchmark @interpret joinpath("/home/julia/base", "sysimg.jl"))'
BenchmarkTools.Trial: 1890 samples with 1 evaluation.
Range (min … max): 2.200 ms … 9.467 ms ┊ GC (min … max): 0.00% … 68.55%
Time (median): 2.459 ms ┊ GC (median): 0.00%
Time (mean ± σ): 2.640 ms ± 585.427 μs ┊ GC (mean ± σ): 2.60% ± 7.68%
▂██▄▄▁
██████▇█▇▆▅▅▄▄▃▃▄▄▄▃▃▃▃▃▃▃▂▂▂▂▂▂▂▂▂▂▁▂▁▁▁▁▁▁▁▁▂▁▂▂▂▂▂▂▂▂▂▂▂ ▃
2.2 ms Histogram: frequency by time 5.15 ms <
Memory estimate: 1.27 MiB, allocs estimate: 25811.
There was a problem hiding this comment.
ok, now the performance is recovered:
❯ julia -e 'using JuliaInterpreter, BenchmarkTools; display(@benchmark @interpret joinpath("/home/julia/base", "sysimg.jl"))'
BenchmarkTools.Trial: 1909 samples with 1 evaluation.
Range (min … max): 2.040 ms … 18.711 ms ┊ GC (min … max): 0.00% … 85.75%
Time (median): 2.426 ms ┊ GC (median): 0.00%
Time (mean ± σ): 2.612 ms ± 749.552 μs ┊ GC (mean ± σ): 2.75% ± 7.63%
▇██▇▅▄▂▁▁
██████████▇█▇█▇▆▅▆▅▅▅▄▃▃▃▃▃▂▃▂▂▂▂▂▁▂▂▂▂▁▂▂▂▂▁▂▂▂▁▂▁▂▂▂▂▂▂▂▂ ▄
2.04 ms Histogram: frequency by time 5.17 ms <
Memory estimate: 1.18 MiB, allocs estimate: 23942.
Thanks @simeonschaub for pointing this out!
Codecov Report
@@ Coverage Diff @@
## master #516 +/- ##
==========================================
- Coverage 86.70% 85.81% -0.89%
==========================================
Files 12 12
Lines 2376 2355 -21
==========================================
- Hits 2060 2021 -39
- Misses 316 334 +18
Continue to review full report at Codecov.
|
- it seems like `:ccall` preserves may now include `SlotNumber` objects, and so we need to update `build_compiled_call!` accordingly - JuliaLang/julia#43671 increased the number of statements of code that involves assignment to a global variable
:ccallpreserves may now includeSlotNumberobjects,and so we need to update
build_compiled_call!accordinglystatements of code that involves assignment to a global variable