improvements on the hot code reloading support (#10892)

* calling the "_actual" versions of functions when defined within the same module - slowdown for the snappy compression is now down from x6 to x4-x5 when HCR is ON
* dynamically linking to the runtime for VS when HCR is on - binaries are smaller
* compilerProcs are also called using the _actual direct version within the module they are defined (system)!
* updated comments & goals
* handling VS-compatible compilers on Windows in a cleaner way
* now the .dll/.so files end up in the nimcache even when --nimcache isn't explicitly stated

7 files changed, 42 insertions, 16 deletions

diff --git a/compiler/ccgcalls.nim b/compiler/ccgcalls.nim
index cefa89289..dd126161d 100644
--- a/compiler/ccgcalls.nim
+++ b/compiler/ccgcalls.nim
@@ -174,6 +174,11 @@ template genParamLoop(params) {.dirty.} =
     if params != nil: add(params, ~", ")
     add(params, genArgNoParam(p, ri.sons[i]))
 
+proc addActualPrefixForHCR(res: var Rope, module: PSym, sym: PSym) =
+  if sym.flags * {sfImportc, sfNonReloadable} == {} and
+      (sym.typ.callConv == ccInline or sym.owner.id == module.id):
+    res = res & "_actual".rope
+
 proc genPrefixCall(p: BProc, le, ri: PNode, d: var TLoc) =
   var op: TLoc
   # this is a hotspot in the compiler
@@ -186,7 +191,10 @@ proc genPrefixCall(p: BProc, le, ri: PNode, d: var TLoc) =
   var length = sonsLen(ri)
   for i in countup(1, length - 1):
     genParamLoop(params)
-  fixupCall(p, le, ri, d, rdLoc(op), params)
+  var callee = rdLoc(op)
+  if p.hcrOn and ri.sons[0].kind == nkSym:
+    callee.addActualPrefixForHCR(p.module.module, ri.sons[0].sym)
+  fixupCall(p, le, ri, d, callee, params)
 
 proc genClosureCall(p: BProc, le, ri: PNode, d: var TLoc) =
 
diff --git a/compiler/cgen.nim b/compiler/cgen.nim
index 1436880d9..25921e9f3 100644
--- a/compiler/cgen.nim
+++ b/compiler/cgen.nim
@@ -706,6 +706,8 @@ proc cgsym(m: BModule, name: string): Rope =
     # we're picky here for the system module too:
     rawMessage(m.config, errGenerated, "system module needs: " & name)
   result = sym.loc.r
+  if m.hcrOn and sym != nil and sym.kind in skProc..skIterator:
+    result.addActualPrefixForHCR(m.module, sym)
 
 proc generateHeaders(m: BModule) =
   add(m.s[cfsHeaders], "\L#include \"nimbase.h\"\L")
diff --git a/compiler/commands.nim b/compiler/commands.nim
index 3a18e69ed..e029ed9f5 100644
--- a/compiler/commands.nim
+++ b/compiler/commands.nim
@@ -494,6 +494,10 @@ proc processSwitch*(switch, arg: string, pass: TCmdLinePass, info: TLineInfo;
     if conf.hcrOn:
       defineSymbol(conf.symbols, "hotcodereloading")
       defineSymbol(conf.symbols, "useNimRtl")
+      # hardcoded linking with dynamic runtime for MSVC for smaller binaries
+      # should do the same for all compilers (wherever applicable)
+      if isVSCompatible(conf):
+        extccomp.addCompileOptionCmd(conf, "/MD")
     else:
       undefSymbol(conf.symbols, "hotcodereloading")
       undefSymbol(conf.symbols, "useNimRtl")
diff --git a/compiler/extccomp.nim b/compiler/extccomp.nim
index 5a1d33ade..ab42f4f52 100644
--- a/compiler/extccomp.nim
+++ b/compiler/extccomp.nim
@@ -223,7 +223,6 @@ compiler bcc:
     props: {hasSwitchRange, hasComputedGoto, hasCpp, hasGcGuard,
             hasAttribute})
 
-
 # Digital Mars C Compiler
 compiler dmc:
   result = (
@@ -376,6 +375,11 @@ proc nameToCC*(name: string): TSystemCC =
       return i
   result = ccNone
 
+proc isVSCompatible*(conf: ConfigRef): bool =
+  return conf.cCompiler == ccVcc or 
+          conf.cCompiler == ccClangCl or
+          (conf.cCompiler == ccIcl and conf.target.hostOS in osDos..osWindows)
+
 proc getConfigVar(conf: ConfigRef; c: TSystemCC, suffix: string): string =
   # use ``cpu.os.cc`` for cross compilation, unless ``--compileOnly`` is given
   # for niminst support
@@ -734,8 +738,9 @@ proc getLinkCmd(conf: ConfigRef; output: AbsoluteFile,
     # way of being able to debug and rebuild the program at the same time. This
     # is accomplished using the /PDB:<filename> flag (there also exists the
     # /PDBALTPATH:<filename> flag). The only downside is that the .pdb files are
-    # atleast 5-10mb big and will quickly accumulate. There is a hacky solution:
-    # we could try to delete all .pdb files with a pattern and swallow exceptions.
+    # atleast 300kb big (when linking statically to the runtime - or else 5mb+) 
+    # and will quickly accumulate. There is a hacky solution: we could try to
+    # delete all .pdb files with a pattern and swallow exceptions.
     #
     # links about .pdb files and hot code reloading:
     # https://ourmachinery.com/post/dll-hot-reloading-in-theory-and-practice/
@@ -747,7 +752,7 @@ proc getLinkCmd(conf: ConfigRef; output: AbsoluteFile,
     # and a bit about the .pdb format in case that is ever needed:
     # https://github.com/crosire/blink
     # http://www.debuginfo.com/articles/debuginfomatch.html#pdbfiles
-    if conf.hcrOn and conf.cCompiler == ccVcc:
+    if conf.hcrOn and isVSCompatible(conf):
       let t = now()
       let pdb = output.string & "." & format(t, "MMMM-yyyy-HH-mm-") & $t.nanosecond & ".pdb"
       result.add " /link /PDB:" & pdb
@@ -838,7 +843,7 @@ proc hcrLinkTargetName(conf: ConfigRef, objFile: string, isMain = false): Absolu
   let basename = splitFile(objFile).name
   let targetName = if isMain: basename & ".exe"
                    else: platform.OS[conf.target.targetOS].dllFrmt % basename
-  result = conf.nimcacheDir / RelativeFile(targetName)
+  result = conf.getNimcacheDir / RelativeFile(targetName)
 
 proc callCCompiler*(conf: ConfigRef) =
   var
@@ -883,7 +888,7 @@ proc callCCompiler*(conf: ConfigRef) =
         add(cmds, getLinkCmd(conf, linkTarget, objfiles & " " & quoteShell(objFile), buildDll))
         # try to remove all .pdb files for the current binary so they don't accumulate endlessly in the nimcache
         # for more info check the comment inside of getLinkCmd() where the /PDB:<filename> MSVC flag is used
-        if conf.cCompiler == ccVcc:
+        if isVSCompatible(conf):
           for pdb in walkFiles(objFile & ".*.pdb"):
             discard tryRemoveFile(pdb)
       # execute link commands in parallel - output will be a bit different
diff --git a/lib/nimhcr.nim b/lib/nimhcr.nim
index 7a2152f96..37331b825 100644
--- a/lib/nimhcr.nim
+++ b/lib/nimhcr.nim
@@ -105,13 +105,12 @@
 #   - the handlers for a reloaded module are always removed when reloading and then
 #     registered when the top-level scope is executed (thanks to `executeOnReload`)
 #
-# TODO - after first merge in upstream Nim:
+# TODO next:
 #
-# - profile
-#   - build speed with and without hot code reloading - difference should be small
-#   - runtime degradation of HCR-enabled code - important!!!
+# - implement the before/after handlers and hasModuleChanged for the javascript target
 # - ARM support for the trampolines
 # - investigate:
+#   - soon the system module might be importing other modules - the init order...?
 #   - rethink the closure iterators
 #     - ability to keep old versions of dynamic libraries alive
 #       - because of async server code
@@ -132,7 +131,7 @@
 #     - currently building with useNimRtl is problematic - lots of problems...
 #     - how to supply the nimrtl/nimhcr shared objects to all test binaries...?
 #     - think about building to C++ instead of only to C - added type safety
-#   - run tests through valgrind and the sanitizers! of HUGE importance!
+#   - run tests through valgrind and the sanitizers!
 #
 # TODO - nice to have cool stuff:
 #
@@ -148,7 +147,6 @@
 # - pragma annotations for files - to be excluded from dll shenanigans
 #   - for such file-global pragmas look at codeReordering or injectStmt
 #   - how would the initialization order be kept? messy...
-#   - per function exclude pragmas would be TOO messy and hard...
 # - C code calling stable exportc interface of nim code (for bindings)
 #   - generate proxy functions with the stable names
 #     - in a non-reloadable part (the main binary) that call the function pointers
@@ -160,9 +158,18 @@
 #   - issue an error
 #     - or let the user handle this by transferring the state properly
 #       - perhaps in the before/afterCodeReload handlers
-# - optimization: calls to procs within a module (+inlined) to use the _actual versions
 # - implement executeOnReload for global vars too - not just statements (and document!)
 # - cleanup at shutdown - freeing all globals
+# - fallback mechanism if the program crashes (the program should detect crashes
+#   by itself using SEH/signals on Windows/Unix) - should be able to revert to
+#   previous versions of the .dlls by calling some function from HCR
+# - improve runtime performance - possibilities
+#   - implement a way for multiple .nim files to be bundled into the same dll
+#     and have all calls within that domain to use the "_actual" versions of
+#     procs so there are no indirections (or the ability to just bundle everything
+#     except for a few unreloadable modules into a single mega reloadable dll)
+#   - try to load the .dlls at specific addresses of memory (close to each other)
+#     allocated with execution flags - check this: https://github.com/fancycode/MemoryModule
 #
 # TODO - unimportant:
 #
diff --git a/lib/system/timers.nim b/lib/system/timers.nim
index 4cd2fe840..b0882ffa1 100644
--- a/lib/system/timers.nim
+++ b/lib/system/timers.nim
@@ -8,7 +8,7 @@
 #
 
 ## Timer support for the realtime GC. Based on
-## `<https://github.com/jckarter/clay/blob/master/compiler/src/hirestimer.cpp>`_
+## `<https://github.com/jckarter/clay/blob/master/compiler/hirestimer.cpp>`_
 
 type
   Ticks = distinct int64
diff --git a/tests/dll/nimhcr_integration.nim b/tests/dll/nimhcr_integration.nim
index daabe918f..40ba90f72 100644
--- a/tests/dll/nimhcr_integration.nim
+++ b/tests/dll/nimhcr_integration.nim
@@ -72,7 +72,7 @@ done
 ## executing:
 ##   <this_file>.exe nim c --hotCodeReloading:on --nimCache:<folder> <this_file>.nim
 
-import os, osproc, times, strutils, hotcodereloading
+import os, osproc, strutils, hotcodereloading
 
 import nimhcr_0 # getInt() - the only thing we continually call from the main module