up to 20x faster jsonutils deserialization

[timotheecour]

• fix https://forum.nim-lang.org/t/8074#51731
• jsonutils deserialization (jsonTo, fromJson) is now up to 20x faster
• condStr is now used instead of ignored (unrelated bugfix)

nim r -d:danger main
(taking best of 5 runs)

before PR

0.27

after PR

0.012

example directly adapted from https://forum.nim-lang.org/t/8074

when true:
  import times, std/json, std/jsonutils

  type OptionContract* = ref object
    id*:               string
    right*:            string
    expiration*:       string
    strike_raw*:       float
    premium_raw*:      float
    data_type*:        string

  type OptionChain* = object
    contracts*: seq[OptionContract]

  proc stub_data(): OptionChain =
    result = OptionChain()
    for _ in 1..6000:
      result.contracts.add OptionContract(
        id: "AMZN CALL 2021-03-19 1460.0 USD",
        right: "call",
        expiration: "2021-03-19",
        strike_raw: 1460.0,
        premium_raw: 1676.03,
        data_type: "some type"
      )
  let json_str = stub_data().toJson.pretty

  let j = json_str.parseJson
  let time = cpuTime()
  for i in 1..5:
    discard j.jsonTo(OptionChain)
  let t2 = cpuTime()
  echo "Time taken: ", t2 - time, " sec"

performance lesson learned

avoid function calls in hotspots (even {.inline.} or --passc:-flto would not help here)

actually the real explanation is instead explained here: #18183 (comment), which is that templates, unlike procs, allow lazy parameter evaluation eg:

checkJson ok, $(json.len, num, numMatched, $T, json)
  # The `msg` is only evaluated on failure if checkJson is a template, but not if it's a proc

[Araq]

Suggested change

	proc raiseJsonException(condStr: string, msg: string) =
	proc raiseJsonException(condStr: string, msg: string) {.noinline.} =

[timotheecour]

done, but can't the C compiler figure out on its own (and without PGO) that it's not worth inlining because it's a noreturn ?

[Araq]

With --exceptions:goto it's not a noreturn.

[Araq]

avoid function calls in hotspots (even {.inline.} or --passc:-flto would not help here)

The lesson seems to me "split code into hot/cold sections", it was a template and thus inlined before.

[timotheecour]

avoid function calls in hotspots (even {.inline.} or --passc:-flto would not help here)
The lesson seems to me "split code into hot/cold sections", it was a template and thus inlined before.

actually both my original conclusion and yours are off-track, IMO the real explanation here is that template calls allow for lazy parameter evaluation (in D, the analog is lazy, refs https://dlang.org/spec/function.html#lazy-params):

checkJson ok, $(json.len, num, numMatched, $T, json)

before PR, $(json.len, num, numMatched, $T, json) would have to be evaluated before calling checkJsonImpl proc, even if not needed then when cond is true.

after PR, $(json.len, num, numMatched, $T, json) is only evaluated if cond is false (cold section)

This also explains why {.inline.} or --passc:-flto would have no effect, as the function call semantics would still require evaluating proc arguments.

This is applicable in other scenarios, and in particular means that it's usually ok [1] to have verbose asserts with potentially complex code when a condition fails, so long the assert/enforce is a template, like doAssert or checkJson

[1] instruction cache size being a second-order effect

[Araq]

[1] instruction cache size being a second-order effect

Which I care about though...

[timotheecour]

[1] instruction cache size being a second-order effect

Which I care about though...

that's essentially what --lean is about, #14282, which can be used throughout stdlib (or 3rd party libraries) via compileOption("lean") to get lean error msgs, for smaller binaries (or via push/pop inside a context)