Skip to content

Commit

Permalink
Refactor internal getvalue for JSON3.Array to improve performance (#267)
Browse files Browse the repository at this point in the history
* Refactor internal getvalue for JSON3.Array to improve performance

Due to several 1.9 performance regressions that heavily impact JSON3,
I decided to just dig in and see if we can fix in on our side since
the JuliaLang changes seem pretty involved. (see issues
[here](JuliaLang/julia#50762) and
[here](JuliaLang/julia#48229) for context).

From what I can tell, the issue really boils down to this one `getvalue`
internal method on `JSON3.Array` where it was having to do some expensive
"sparams" computation to instantiate the `JSON3.Array` type.
By manually unrolling this method a bit and fully specifying all the type
parameters this seems to resolve the performance issues. Benchmarks look
like:

Julia 1.8:
```julia
julia> @benchmark JSON3.defaultminimum(x)
BenchmarkTools.Trial: 10000 samples with 1 evaluation.
 Range (min … max):  31.625 μs … 140.959 μs  ┊ GC (min … max): 0.00% … 0.00%
 Time  (median):     32.250 μs               ┊ GC (median):    0.00%
 Time  (mean ± σ):   32.602 μs ±   1.562 μs  ┊ GC (mean ± σ):  0.00% ± 0.00%

      ▂█▃▂▃
  ▁▂▅██████▆▄▅▂▂▂▂▂▃▂▂▃▂▂▂▂▂▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁ ▂
  31.6 μs         Histogram: frequency by time         36.6 μs <

 Memory estimate: 6.70 KiB, allocs estimate: 84.
```

Julia 1.9:
```julia
julia> @benchmark JSON3.defaultminimum(x)
BenchmarkTools.Trial: 10000 samples with 1 evaluation.
 Range (min … max):  165.584 μs …  10.519 ms  ┊ GC (min … max): 0.00% … 94.46%
 Time  (median):     168.166 μs               ┊ GC (median):    0.00%
 Time  (mean ± σ):   170.886 μs ± 103.652 μs  ┊ GC (mean ± σ):  0.58% ±  0.94%

     ▄█▆▃
  ▁▂▇████▇▇▅▄▄▄▄▃▃▂▂▂▃▃▃▄▄▃▃▃▂▂▂▂▂▂▂▂▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁▁ ▂
  166 μs           Histogram: frequency by time          185 μs <

 Memory estimate: 10.11 KiB, allocs estimate: 166.
```

This PR:
```julia
julia> @benchmark JSON3.defaultminimum(x)
BenchmarkTools.Trial: 10000 samples with 3 evaluations.
 Range (min … max):  8.431 μs … 497.806 μs  ┊ GC (min … max): 0.00% … 95.49%
 Time  (median):     8.820 μs               ┊ GC (median):    0.00%
 Time  (mean ± σ):   9.157 μs ±   9.056 μs  ┊ GC (mean ± σ):  1.92% ±  1.91%

   ▂▄▃▂  ▅██▆▄▂▁▃▅▆▅▄▃▂▂▂▂▂▂▁▁▁                               ▂
  ▅████▇▆██████████████████████▇▇▇▇▇▇▇▇▇▇▆▇▇▅▆▆▅▆▆▆▅▄▅▅▅▄▄▅▅▄ █
  8.43 μs      Histogram: log(frequency) by time      10.8 μs <

 Memory estimate: 6.20 KiB, allocs estimate: 72.
```

* Remove print-related test that breaks post 1.9
  • Loading branch information
quinnj authored Aug 2, 2023
1 parent e33786c commit ac35be7
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 4 deletions.
51 changes: 48 additions & 3 deletions src/utils.jl
Original file line number Diff line number Diff line change
Expand Up @@ -129,9 +129,54 @@ function getvalue(::Type{Object}, buf, tape, tapeidx, t)
end

function getvalue(::Type{Array}, buf, tape, tapeidx, t)
x = Array{geteltype(tape[tapeidx+1])}(buf, Base.unsafe_view(tape, tapeidx:tapeidx + getnontypemask(t)), Int[])
populateinds!(x)
return x
T = tape[tapeidx+1]
ttape = Base.unsafe_view(tape, tapeidx:tapeidx + getnontypemask(t))
inds = Int[]
if empty(T)
x = Array{Union{},typeof(buf),typeof(ttape)}(buf, ttape, inds)
populateinds!(x)
return x
elseif isany(T)
x = Array{Any,typeof(buf),typeof(ttape)}(buf, ttape, inds)
populateinds!(x)
return x
elseif isobject(T)
x = Array{Object,typeof(buf),typeof(ttape)}(buf, ttape, inds)
populateinds!(x)
return x
elseif isarray(T)
x = Array{Array,typeof(buf),typeof(ttape)}(buf, ttape, inds)
populateinds!(x)
return x
elseif isstring(T)
x = Array{String,typeof(buf),typeof(ttape)}(buf, ttape, inds)
populateinds!(x)
return x
elseif isint(T)
x = Array{Int64,typeof(buf),typeof(ttape)}(buf, ttape, inds)
populateinds!(x)
return x
elseif isfloat(T)
x = Array{Float64,typeof(buf),typeof(ttape)}(buf, ttape, inds)
populateinds!(x)
return x
elseif isbool(T)
x = Array{Bool,typeof(buf),typeof(ttape)}(buf, ttape, inds)
populateinds!(x)
return x
elseif isnull(T)
x = Array{Nothing,typeof(buf),typeof(ttape)}(buf, ttape, inds)
populateinds!(x)
return x
elseif isintfloat(T)
x = Array{Union{Int64, Float64},typeof(buf),typeof(ttape)}(buf, ttape, inds)
populateinds!(x)
return x
else
x = Array{Union{geteltype(nonnull(T)), Nothing},typeof(buf),typeof(ttape)}(buf, ttape, inds)
populateinds!(x)
return x
end
end

function getvalue(::Type{Symbol}, buf, tape, tapeidx, t)
Expand Down
1 change: 0 additions & 1 deletion test/runtests.jl
Original file line number Diff line number Diff line change
Expand Up @@ -783,7 +783,6 @@ expr = JSON3.read("""
@test JSON3.write(Int64) == "\"Int64\""
@test JSON3.write(Float64) == "\"Float64\""
@test JSON3.write(String) == "\"String\""
@test startswith(JSON3.write(NamedTuple{(:a, :b), Tuple{Int64, String}}), "\"NamedTuple{(:a, :b),")
@test startswith(JSON3.write(Dict{Symbol, Any}), "\"Dict{Symbol,")
@test JSON3.write(Bool) == "\"Bool\""
@test JSON3.write(Nothing) == "\"Nothing\""
Expand Down

0 comments on commit ac35be7

Please sign in to comment.