Restore performance of parsing

[KristofferC]

Fixes #205

# Before
julia> @btime Parsers.parse(Float64, "1.23e-4")
  197.714 ns (9 allocations: 304 bytes)
0.000123

# After
julia> @btime Parsers.parse(Float64, "1.23e-4")
  10.438 ns (0 allocations: 0 bytes)
0.000123

I doubt all these inlines are necessary but I just want to get back to status quo w.r.t performance.

[KristofferC]

I doubt all these inlines are necessary but I just want to get back to status quo w.r.t performance.

I had Claude go through all these inlines using this benchmark:

using BenchmarkTools, Parsers, Dates
  BenchmarkTools.DEFAULT_PARAMETERS.seconds = 2  # 0.5 during the bisection runs
  macro bench(name, ex)
      quote
          b = @benchmark $ex
          println(rpad($name, 22), ": ", round(minimum(b.times), digits=1), " ns, ", b.allocs, " allocs")
      end |> esc
  end

  const OPTS_DELIM = Parsers.Options(delim=',')
  const OPTS_QUOTED = Parsers.Options(delim=',', quoted=true)
  const OPTS_SENT = Parsers.Options(delim=',', sentinel=["NA"])
  const OPTS_GROUP = Parsers.Options(delim=',', groupmark=',')

  @bench "float"          Parsers.parse(Float64, "1.23e-4")
  @bench "float_long"     Parsers.parse(Float64, "123456.789012e10")
  @bench "float32"        Parsers.parse(Float32, "1.23e-4")
  @bench "int"            Parsers.parse(Int64, "12345")
  @bench "int_big"        Parsers.parse(Int64, "1234567890123")
  @bench "bool"           Parsers.parse(Bool, "true")
  @bench "date"           Parsers.parse(Date, "2023-01-15")
  @bench "datetime"       Parsers.parse(DateTime, "2023-01-15T10:30:00")
  @bench "xparse_float"   Parsers.xparse(Float64, "1.23e-4,", 1, 8, OPTS_DELIM)
  @bench "xparse_int"     Parsers.xparse(Int64, "12345,", 1, 6, OPTS_DELIM)
  @bench "xparse_string"  Parsers.xparse(String, "hello,world", 1, 11, OPTS_DELIM)
  @bench "xparse_quoted"  Parsers.xparse(Float64, "\"1.23e-4\",", 1, 10, OPTS_QUOTED)
  @bench "xparse_qstring" Parsers.xparse(String, "\"hey there\",", 1, 12, OPTS_QUOTED)
  @bench "xparse_sent"    Parsers.xparse(Float64, "NA,", 1, 3, OPTS_SENT)
  @bench "xparse_group"   Parsers.xparse(Int64, "1,234,567;", 1, 10, OPTS_GROUP)

to find the minimum set of inlines to keep which mattered for performance. I've updated the commit to only add these inlines.

  ┌────────────────┬─────────────┬───────────────┬─────────────────┬──────────────────────┐
  │   Benchmark    │   v2.8.1    │     main      │ kc/restore_perf │ restore_perf vs main │
  ├────────────────┼─────────────┼───────────────┼─────────────────┼──────────────────────┤
  │ float          │ 14.9 ns (0) │  241.6 ns (9) │     14.8 ns (0) │                16.3× │
  ├────────────────┼─────────────┼───────────────┼─────────────────┼──────────────────────┤
  │ float_long     │ 22.9 ns (0) │ 251.1 ns (10) │     23.2 ns (0) │                10.8× │
  ├────────────────┼─────────────┼───────────────┼─────────────────┼──────────────────────┤
  │ float32        │ 14.7 ns (0) │  227.1 ns (9) │     15.0 ns (0) │                15.1× │
  ├────────────────┼─────────────┼───────────────┼─────────────────┼──────────────────────┤
  │ int            │  7.9 ns (0) │   16.3 ns (0) │      9.0 ns (0) │                 1.8× │
  ├────────────────┼─────────────┼───────────────┼─────────────────┼──────────────────────┤
  │ int_big        │ 11.7 ns (0) │   18.4 ns (0) │     10.6 ns (0) │                 1.7× │
  ├────────────────┼─────────────┼───────────────┼─────────────────┼──────────────────────┤
  │ bool           │  8.2 ns (0) │   15.5 ns (0) │      8.1 ns (0) │                 1.9× │
  ├────────────────┼─────────────┼───────────────┼─────────────────┼──────────────────────┤
  │ date           │ 23.5 ns (0) │   26.7 ns (0) │     20.2 ns (0) │                 1.3× │
  ├────────────────┼─────────────┼───────────────┼─────────────────┼──────────────────────┤
  │ datetime       │ 63.9 ns (3) │   65.7 ns (3) │     66.8 ns (3) │                 1.0× │
  ├────────────────┼─────────────┼───────────────┼─────────────────┼──────────────────────┤
  │ xparse_float   │ 21.7 ns (0) │ 358.0 ns (14) │     20.3 ns (0) │                17.6× │
  ├────────────────┼─────────────┼───────────────┼─────────────────┼──────────────────────┤
  │ xparse_int     │ 15.9 ns (0) │   40.7 ns (0) │     14.6 ns (0) │                 2.8× │
  ├────────────────┼─────────────┼───────────────┼─────────────────┼──────────────────────┤
  │ xparse_string  │ 18.2 ns (0) │   48.6 ns (0) │     18.1 ns (0) │                 2.7× │
  ├────────────────┼─────────────┼───────────────┼─────────────────┼──────────────────────┤
  │ xparse_quoted  │ 24.3 ns (0) │ 365.0 ns (14) │     22.3 ns (0) │                16.4× │
  ├────────────────┼─────────────┼───────────────┼─────────────────┼──────────────────────┤
  │ xparse_qstring │ 21.1 ns (0) │   48.4 ns (0) │     20.7 ns (0) │                 2.3× │
  ├────────────────┼─────────────┼───────────────┼─────────────────┼──────────────────────┤
  │ xparse_sent    │ 24.4 ns (0) │ 262.5 ns (11) │     19.2 ns (0) │                13.7× │
  ├────────────────┼─────────────┼───────────────┼─────────────────┼──────────────────────┤
  │ xparse_group   │ 15.2 ns (0) │   39.3 ns (0) │     14.9 ns (0) │                 2.6× │
  └────────────────┴─────────────┴───────────────┴─────────────────┴──────────────────────┘

[quinnj]

Inline suggestions trimming the three annotations that measured as net-negative for precompile size — summary with numbers in the PR comment below. [posted by claude]

[quinnj]

findendquoted/finddelimiter are the two big scanning loops, and the quoted/delimiter layers wrap every type's pipeline — marking them @inline stamps a copy of the loop body into every (type × source × return-type) pipeline specialization the workload compiles. Keeping them as shared compiled units (together with the String typeparser suggestion) measured -1.4MB cache (-20%) and ~-0.3s precompile on this PR, with runtime parity: they do O(field-length) work per call, so the call into a shared type-stable instance amortizes — unlike the per-character float digit machine, where inlining is the right move.

Suggested change

	@inline function findendquoted(::Type{T}, source, pos, len, b, code, pl, isquoted, cq, e, stripquoted) where {T}
	function findendquoted(::Type{T}, source, pos, len, b, code, pl, isquoted, cq, e, stripquoted) where {T}

[posted by claude]

[quinnj]

Same as findendquoted above.

Suggested change

	@inline function finddelimiter(::Type{T}, source, pos, len, b, code, pl, delim, ignorerepeated, cmt, ignoreemptylines, stripwhitespace) where {T}
	function finddelimiter(::Type{T}, source, pos, len, b, code, pl, delim, ignorerepeated, cmt, ignoreemptylines, stripwhitespace) where {T}

[posted by claude]

[quinnj]

Inlining the String typeparser lets the String path flatten transitively into each pipeline specialization (typeparser → findendquoted → ...), which compounds the cache cost of the scanning-loop inlines; xparse(String, ...) benchmarks measured parity without it.

Suggested change

	@inline function typeparser(::AbstractConf{T}, source, pos, len, b, code, pl, opts) where {T <: AbstractString}
	function typeparser(::AbstractConf{T}, source, pos, len, b, code, pl, opts) where {T <: AbstractString}

[posted by claude]

[quinnj]

I also had Claude fable do a review/independent attempt at this and it basically came up w/ the same changes, but w/ the 3 inline changes/suggestions posted above. I'd like to keep as much of the precompile timing/cache size wins we had from before, while getting perf back.

[KristofferC]

Inline suggestions trimming the three annotations that measured as net-negative for precompile size — summary with numbers in the PR comment below. [posted by claude]

I said that the removal of these inline significantly affects performance, is this saying that it does not, or? I'm worried the comments talk about precompile size without giving any data about performance. Anyway, I'll benchmark again with those inline removed.

[KristofferC]

Ok, so here are the regressions from running the suggested diff:

  ┌────────────────┬───────────────────────┬───────────────────┬──────────┐
  │   Benchmark    │ branch (with inlines) │ 3 inlines removed │ slowdown │
  ├────────────────┼───────────────────────┼───────────────────┼──────────┤
  │ xparse_float   │ 14.6 ns               │ 19.6 ns           │ 1.34×    │
  ├────────────────┼───────────────────────┼───────────────────┼──────────┤
  │ xparse_int     │ 11.0 ns               │ 14.9 ns           │ 1.35×    │
  ├────────────────┼───────────────────────┼───────────────────┼──────────┤
  │ xparse_string  │ 14.9 ns               │ 22.3 ns           │ 1.50×    │
  ├────────────────┼───────────────────────┼───────────────────┼──────────┤
  │ xparse_quoted  │ 15.5 ns               │ 20.2 ns           │ 1.30×    │
  ├────────────────┼───────────────────────┼───────────────────┼──────────┤
  │ xparse_qstring │ 17.2 ns               │ 23.6 ns           │ 1.37×    │
  ├────────────────┼───────────────────────┼───────────────────┼──────────┤
  │ xparse_sent    │ 15.9 ns               │ 19.8 ns           │ 1.25×    │
  ├────────────────┼───────────────────────┼───────────────────┼──────────┤
  │ xparse_group   │ 10.3 ns               │ 14.6 ns           │ 1.42×    │
  └────────────────┴───────────────────────┴───────────────────┴──────────┘

xparse(String, ...) benchmarks measured parity without it.

Not for me at least

A 0.3s precompile time reduction for these 50% regressions in the core Parsing library doesn't seem like the right trade-off. It is possible we can inline less but it would at least be good to first get down to baseline w.r.t performance and then look into less inlining of some parts (but then with careful benchmarking before and after).

[KristofferC]

bump

[quinnj]

Hmmmmm, maybe this is AI being too hand-wavy with me and saying perf was at parity when it really wasn't. Let's merge and get all the perf back. I was just hoping we could also retain some of the precompile cache size gains we got from earlier.

[KristofferC]

This is still inlinig quite a bit less, so hopefully some of that is indeed kept. The inference barrier is also still there, just that it doesn't poison the return type anymore.