regression: compilation 5x slower (33s => 171s for nim_temp c testament)

[timotheecour]

#16480 causes some compilations to be 5x slower /cc @juancarlospaco

Example

./koch temp
t1: XDG_CONFIG_HOME= bin/nim_temp c testament/testament.nim
t2: XDG_CONFIG_HOME= nim c testament/testament.nim

right before #16480 (1d2e2b5)
t1: 33s
t2: 2.6s

right after:
t1: 171s
t2: 6.5s

Additional Information

still a problem on 61fd19c
--stackTraceMsgs + -d:nimHasStacktraceMsgs really helped when trying to figure out the culprit:
when I ^C in a seemingly frozen compilation (t1) it showed:

compiler/semexprs.nim(2861) semExpr /Users/timothee/git_clone/nim/Nim_prs/lib/pure/mimetypes.nim(1933, 3) nkConstSection
compiler/semstmts.nim(647) semConst
compiler/semexprs.nim(77) semExprWithType
compiler/semexprs.nim(68) semExprCheck
compiler/semexprs.nim(2758) semExpr /Users/timothee/git_clone/nim/Nim_prs/lib/pure/mimetypes.nim(1934, 27) nkCall
compiler/semexprs.nim(1011) semDirectOp
compiler/semexprs.nim(908) afterCallActions
compiler/semexprs.nim(812) evalAtCompileTime
compiler/vm.nim(2208) evalStaticExpr
compiler/vm.nim(2200) evalConstExprAux
compiler/vm.nim(755) rawExecute
compiler/vm.nim(240) writeField
compiler/vm.nim(193) copyValue
compiler/ast.nim(1153) newNode
lib/system/gc.nim(439) newObj

which points to mimetypes.nim(1934, 27), leading me to #16480

EDIT: root cause is #16790

[juancarlospaco]

Yep, originally my idea was run-time but it was requested to pre-calc at compile time. 🤷

[timotheecour]

I think we should revert #16480; even if it didn't cause this regression (see also fixable issues in https://github.com/nim-lang/Nim/pull/16480/files#r556247845), there are better ways to get the same effect in a more generic way (see below) without having to add any symbol to std/mimetypes; IMO mimesExtMaxLen, mimesMaxLen, mimesLongest is code bloat.

Instead we can do as follows:

template wrapStatic(s): untyped =
  # needed pending https://github.com/nim-lang/RFCs/issues/276
  (proc(): auto = s)()

# to add to sequtils
template foldlIt*(s, op, ret): untyped =
  block:
    var a {.inject.} = ret
    for b {.inject.} in s:
      a = op
    a

import std/mimetypes
doAssert foldlIt(mimes3, (max(a[0], b[0].len), max(a[1], b[1].len)), (0,0)) == (24, 73)
static:
  doAssert wrapStatic foldlIt(mimes3, (max(a[0], b[0].len), max(a[1], b[1].len)), (0,0)) == (24, 73)

and perhaps even add a maxIt:

static:
  doAssert maxIt(mimes3, (a[0].len, b[0].len) == (24, 73)

• note that there's still an underlying performance bug in VM here, likely due to the fact that const variables gets pasted at each iteration instead of referenced:

    static:
      # this is very fast (milliseconds)
      for i in 0..<3:
        echo wrapStatic(block:
          let z = mimes3
          foldlIt(z, (max(a[0], b[0].len), max(a[1], b[1].len)), (0,0)))
      # this is very slow (4 seconds per iter)
      for i in 0..<3:
        echo wrapStatic foldlIt(mimes3, (max(a[0], b[0].len), max(a[1], b[1].len)), (0,0))

links

see also #15528 which woud avoid this using ref types

[Araq]

Please revert or improve it.

[timotheecour]

=> #16711

[juancarlospaco]

Sorry, this pull request couldn’t be reverted. It may have already been reverted. 🤷
GitHub seems to autodetect the other PR.