compiler, ci: remove acyclic and run leak tests (#1104)

## Summary
Local tests have shown that the compiler does create cycles and leak
memory during compilation. As such we are dropping all acyclic
annotations and prepare the stage to either live without them or have
them reintroduced over time.


## Details
Aside from dropping acyclic annotations, this commit prepares the
compiler for future work on memory optimizations with these changes:

- Cycle collection is disabled for the compiler itself. This is because
the compilation command is not a long-running task and we value speed
over memory usage here.

- nimsuggest is set to collect cycles only after running a command. This
should keep memory usage low during extended usage while allowing the
compilation phases to stay at top speed.

- A CI pipeline has been setup to test the compiler using LeakSanitizer,
making sure that leaks are caught before they are introduced.


Some performance numbers due to these changes:

Benchmarked via: 
`hyperfine -- "bin/nim c --forceBuild --compileOnly --verbosity:0
--hints:off --warnings:off compiler/nim.nim"`


            Time             Ratio
    Before  9.155s ± 0.044s  1.00
    After   9.599s ± 0.039s  1.05

.github/workflows/leak.yml

@@ -0,0 +1,40 @@
+name: Test compiler and tools for memory leaks
+
+on:
+  # This is for passing required checks
+  pull_request:
+  merge_group:
+
+jobs:
+  leaktest:
+    # Skip this for PRs
+    if: github.event_name != 'pull_request'
+
+    name: Compiler and tools leak test
+    runs-on: ubuntu-latest
+
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+          filter: tree:0
+
+      - name: Enable annotations
+        run: echo "::add-matcher::.github/nim-problem-matcher.json"
+
+      - name: Build release binaries with LeakSanitizer
+        run: |
+          ./koch.py all-strict \
+            -d:useMalloc \
+            --passC:-fsanitize=leak \
+            --passC:"-fno-omit-frame-pointer -mno-omit-leaf-frame-pointer" \
+            --passL:-fsanitize=leak \
+            --debuginfo \
+            --linedir
+
+      - name: Run tools tests
+        run: ./koch.py testTools
+
+      # NOTE: We don't run the full test suite here as self-bootstrap
+      # and tools testsuite can be considered enough to rat out memory leaks
+      # within the compiler itself.

compiler/ast/ast_parsed_types.nim

@@ -226,7 +226,8 @@ type
     base*: NumericalBase
     literal*: string
 
-  ParsedNodeData*{.final, acyclic.} = object
+  # TODO: This type should be marked acyclic
+  ParsedNodeData*{.final.} = object
     # TODO: replace token fields with indexing into a token sequence, this
     #       should also address line info tracking.
     comment*: string       # TODO: replace with an index into a token stream

compiler/ast/ast_types.nim

@@ -901,7 +901,7 @@ type
 
 
 type
-  TIdObj* {.acyclic.} = object of RootObj
+  TIdObj* = object of RootObj
     itemId*: ItemId
   PIdObj* = ref TIdObj
 
@@ -1256,7 +1256,7 @@ type
     adSemDeprecatedCompilerOptArg   # warning promoted to error
 
   PAstDiag* = ref TAstDiag
-  TAstDiag* {.acyclic.} = object
+  TAstDiag* = object
     # xxx: consider splitting storage type vs message
     # xxx: consider breaking up diag into smaller types
     # xxx: try to shrink the int/int128 etc types for counts/ordinals
@@ -1561,7 +1561,7 @@ type
           adSemDefNameSymIllformedAst:
         discard
 
-  TNode*{.final, acyclic.} = object # on a 32bit machine, this takes 32 bytes
+  TNode*{.final.} = object # on a 32bit machine, this takes 32 bytes
                                     # on a 64bit machine, this takes 40 bytes
     typ*: PType
     id*: NodeId  # placed after `typ` field to save space due to field alignment
@@ -1642,15 +1642,16 @@ type
 
   PInstantiation* = ref TInstantiation
 
-  TScope* {.acyclic.} = object
+  # TODO: This type should be marked acyclic
+  TScope* = object
     depthLevel*: int
     symbols*: TStrTable
     parent*: PScope
     allowPrivateAccess*: seq[PSym] #  # enable access to private fields
 
   PScope* = ref TScope
 
-  TSym* {.acyclic.} = object of TIdObj # Keep in sync with PackedSym
+  TSym* = object of TIdObj # Keep in sync with PackedSym
     ## proc and type instantiations are cached in the generic symbol
     case kind*: TSymKind
     of routineKinds - {skMacro}:
@@ -1728,7 +1729,7 @@ type
     attachedTrace,
     attachedDeepCopy
 
-  TType* {.acyclic.} = object of TIdObj
+  TType*  = object of TIdObj
     ## types are identical only if they have the same id; there may be multiple
     ## copies of a type in memory! Keep in sync with PackedType
     kind*: TTypeKind          ## kind of type

compiler/ast/idents.nim

@@ -22,7 +22,8 @@ import
 
 type
   PIdent* = ref TIdent
-  TIdent*{.acyclic.} = object
+  # TODO: This type should be marked acyclic
+  TIdent* = object
     id*: int ## unique id; use this for comparisons and not the pointers
     s*: string
     next*: PIdent ## for hash-table chaining

compiler/backend/cgir.nim

@@ -301,4 +301,4 @@ proc merge*(dest: var Body, source: Body): CgNode =
       dest.code.kids.add source.code
     else:
       dest.code = newStmt(cnkStmtList, dest.code.info,
-                          [dest.code, source.code])
+                          [dest.code, source.code])

compiler/front/main.nim

@@ -519,6 +519,17 @@ proc mainCommand*(graph: ModuleGraph) =
   when defined(nimDebugUnreportedErrors):
     addExitProc proc = echoAndResetUnreportedErrors(conf)
 
+  when defined(gcOrc):
+    # Compilation is currently very taxing on ORC due to frequent
+    # creations and destructions of ref objects with potential cycles.
+    #
+    # Disable ORC to reduce overhead from the cycle collector at the
+    # cost of memory usage.
+    #
+    # We don't collect cycles afterwards as the command is one-shot and
+    # memory should be freed once the program stops.
+    GC_disableOrc()
+
   ## process all commands
   case conf.cmd
   of cmdBackends: compileToBackend()

compiler/front/options.nim

@@ -206,7 +206,7 @@ type
     ## This is useful for environments such as nimsuggest, which discard
     ## the output.
 
-  ConfigRef* {.acyclic.} = ref object
+  ConfigRef* = ref object
     ## every global configuration fields marked with '*' are subject to the
     ## incremental compilation mechanisms (+) means "part of the dependency"
 
@@ -1558,4 +1558,4 @@ func inDebug*(conf: ConfigRef): bool {.
   deprecated: "DEBUG proc, do not use in the final build!",
   noSideEffect.} =
   ## Check whether 'nim compiler debug' is defined right now.
-  return conf.isDefined("nimCompilerDebug")
+  return conf.isDefined("nimCompilerDebug")

compiler/modules/modulegraphs.nim

@@ -87,7 +87,7 @@ type
     concreteTypes*: seq[FullId]
     inst*: PInstantiation
 
-  ModuleGraph* {.acyclic.} = ref object
+  ModuleGraph* = ref object
     ifaces*: seq[Iface]  ## indexed by int32 fileIdx
     packed*: PackedModuleGraph
     encoders*: seq[PackedEncoder]

compiler/sem/semdata.nim

@@ -59,7 +59,8 @@ type
 
   POptionEntry* = ref TOptionEntry
   PProcCon* = ref TProcCon
-  TProcCon* {.acyclic.} = object ## procedure context; also used for top-level
+  # TODO: This type should be marked acyclic
+  TProcCon* = object ## procedure context; also used for top-level
                                  ## statements
     owner*: PSym              ## the symbol this context belongs to
     resultSym*: PSym          ## the result symbol (if we are in a proc)

compiler/sem/semtypinst.nim

@@ -121,7 +121,8 @@ proc cacheTypeInst(c: PContext; inst: PType) =
   addToGenericCache(c, gt.sym, inst)
 
 type
-  LayeredIdTable* {.acyclic.} = ref object
+  # TODO: This type should be marked acyclic
+  LayeredIdTable* = ref object
     topLayer*: TIdTable
     nextLayer*: LayeredIdTable
 
@@ -1072,4 +1073,4 @@ proc tryGenerateInstance*(c: PContext, pt: TIdTable, info: TLineInfo, t: PType):
   if containsUnboundTypeVar(pt, t):
     result = nil
   else:
-    result = generateTypeInstance(c, pt, info, t)
+    result = generateTypeInstance(c, pt, info, t)

compiler/utils/btrees.nim

@@ -16,7 +16,8 @@ const
   Mhalf = M div 2
 
 type
-  Node[Key, Val] {.acyclic.} = ref object
+  # TODO: This type should be marked acyclic
+  Node[Key, Val]  = ref object
     entries: int
     keys: array[M, Key]
     case isInternal: bool

nimsuggest/nimsuggest.nim

@@ -207,6 +207,14 @@ proc listEpc(): SexpNode =
     methodDesc.add(docstring)
     result.add(methodDesc)
 
+proc runGc() =
+  ## Runs a GC collection pass. This procedure will also collect any cycles assuming ORC is in use.
+  ##
+  ## When nimsuggest is being used as a library, no GC mode change will be performed.
+  GC_fullCollect()
+  when isMainModule and defined(gcOrc):
+    GC_disableOrc()
+
 proc executeNoHooks(cmd: IdeCmd, file, dirtyfile: AbsoluteFile, line, col: int;
              graph: ModuleGraph) =
   let conf = graph.config
@@ -217,6 +225,7 @@ proc executeNoHooks(cmd: IdeCmd, file, dirtyfile: AbsoluteFile, line, col: int;
   )
 
   executeCmd(cmd, file, dirtyfile, line, col, graph)
+  runGc() # prune cycles
   if conf.ideCmd in {ideUse, ideDus}:
     let u = graph.findTrackedSym()
     if u != nil:
@@ -516,7 +525,7 @@ proc recompileFullProject(graph: ModuleGraph) =
   resetSystemArtifacts(graph)
   graph.vm = nil
   graph.resetAllModules()
-  GC_fullCollect()
+  runGc()
   compileProject(graph)
   #echo GC_getStatistics()
 
@@ -722,6 +731,15 @@ proc handleCmdLine(cache: IdentCache; conf: ConfigRef, argv: openArray[string])
     mainCommand(graph)
 
 when isMainModule:
+  when defined(gcOrc):
+    # Compilation is currently very taxing on ORC due to frequent
+    # creations and destructions of ref objects with potential cycles.
+    #
+    # Disable ORC to reduce overhead from the cycle collector. Nimsuggest
+    # will manually run cycle collection at strategic location instead to
+    # minimize GC overhead within hotspots.
+    GC_disableOrc()
+
   let argv = getExecArgs()
   let conf = newConfigRef(cli_reporter.reportHook)
   conf.astDiagToLegacyReport = cli_reporter.legacyReportBridge

tools/koch/koch.nim

@@ -536,10 +536,10 @@ when isMainModule:
       else: showHelp(success = false)
     of cmdArgument:
       case normalize(op.key)
-      of "all": buildReleaseBinaries()
+      of "all": buildReleaseBinaries(op.cmdLineRest)
       of "all-strict":
         # when using strict mode, don't abort after the first error
-        buildReleaseBinaries("-d:nimStrictMode --errorMax:3")
+        buildReleaseBinaries("-d:nimStrictMode --errorMax:3 " & op.cmdLineRest)
       of "boot": boot(op.cmdLineRest)
       of "clean": clean(op.cmdLineRest)
       of "doc", "docs": buildDocs(op.cmdLineRest)