fix #16752: threadvar now works with importcpp types; osx now uses native TLS (--tlsEmulation:off), which can be orders of magnitude faster (#16750)

* osx now uses native TLS, which can be orders of magnitude faster

* add {.cppNonPod.}

* improve test

* changelog, docs, disable part of windows test

changelog.md

@@ -132,10 +132,13 @@ with other backends. see #9125. Use `-d:nimLegacyJsRound` for previous behavior.
 - Added `nim --eval:cmd` to evaluate a command directly, see `nim --help`.
 
 - VM now supports `addr(mystring[ind])` (index + index assignment)
+
 - Type mismatch errors now show more context, use `-d:nimLegacyTypeMismatch` for previous
   behavior.
 
 - Added `--hintAsError` with similar semantics as `--warningAsError`.
+- TLS: OSX now uses native TLS (`--tlsEmulation:off`), TLS now works with importcpp non-POD types,
+  such types must use `.cppNonPod` and `--tlsEmulation:off`should be used.
 
 ## Tool changes
 

compiler/ast.nim

@@ -265,6 +265,10 @@ type
     sfShadowed,       # a symbol that was shadowed in some inner scope
     sfThread,         # proc will run as a thread
                       # variable is a thread variable
+    sfCppNonPod,      # tells compiler to treat such types as non-pod's, so that
+                      # `thread_local` is used instead of `__thread` for
+                      # {.threadvar.} + `--threads`. Only makes sense for importcpp types.
+                      # This has a performance impact so isn't set by default.
     sfCompileTime,    # proc can be evaluated at compile time
     sfConstructor,    # proc is a C++ constructor
     sfDispatcher,     # copied method symbol is the dispatcher

compiler/ccgthreadvars.nim

@@ -7,8 +7,8 @@
 #    distribution, for details about the copyright.
 #
 
-## Thread var support for crappy architectures that lack native support for
+## Thread var support for architectures that lack native support for
-## thread local storage. (**Thank you Mac OS X!**)
+## thread local storage.
 
 # included from cgen.nim
 
@@ -35,7 +35,11 @@ proc declareThreadVar(m: BModule, s: PSym, isExtern: bool) =
     if isExtern: m.s[cfsVars].add("extern ")
     elif lfExportLib in s.loc.flags: m.s[cfsVars].add("N_LIB_EXPORT_VAR ")
     else: m.s[cfsVars].add("N_LIB_PRIVATE ")
-    if optThreads in m.config.globalOptions: m.s[cfsVars].add("NIM_THREADVAR ")
+    if optThreads in m.config.globalOptions:
+      let sym = s.typ.sym
+      if sym != nil and sfCppNonPod in sym.flags:
+        m.s[cfsVars].add("NIM_THREAD_LOCAL ")
+      else: m.s[cfsVars].add("NIM_THREADVAR ")
     m.s[cfsVars].add(getTypeDesc(m, s.loc.t))
     m.s[cfsVars].addf(" $1;$n", [s.loc.r])
 

compiler/options.nim

@@ -380,11 +380,12 @@ proc hasWarn*(conf: ConfigRef, note: TNoteKind): bool =
 
 proc hcrOn*(conf: ConfigRef): bool = return optHotCodeReloading in conf.globalOptions
 
-template depConfigFields*(fn) {.dirty.} =
+when false:
-  fn(target)
+  template depConfigFields*(fn) {.dirty.} = # deadcode
-  fn(options)
+    fn(target)
-  fn(globalOptions)
+    fn(options)
-  fn(selectedGC)
+    fn(globalOptions)
+    fn(selectedGC)
 
 const oldExperimentalFeatures* = {implicitDeref, dotOperators, callOperator, parallel}
 

compiler/pragmas.nim

@@ -65,7 +65,7 @@ const
     wPure, wHeader, wCompilerProc, wCore, wFinal, wSize, wShallow,
     wIncompleteStruct, wCompleteStruct, wByCopy, wByRef,
     wInheritable, wGensym, wInject, wRequiresInit, wUnchecked, wUnion, wPacked,
-    wBorrow, wGcSafe, wPartial, wExplain, wPackage}
+    wCppNonPod, wBorrow, wGcSafe, wPartial, wExplain, wPackage}
   fieldPragmas* = declPragmas + {
     wGuard, wBitsize, wCursor, wRequiresInit, wNoalias} - {wExportNims, wNodecl} # why exclude these?
   varPragmas* = declPragmas + {wVolatile, wRegister, wThreadVar,
@@ -843,6 +843,8 @@ proc singlePragma(c: PContext, sym: PSym, n: PNode, i: var int,
         else: invalidPragma(c, it)
       of wImportCpp:
         processImportCpp(c, sym, getOptionalStr(c, it, "$1"), it.info)
+      of wCppNonPod:
+        incl(sym.flags, sfCppNonPod)
       of wImportJs:
         if c.config.backend != backendJs:
           localError(c.config, it.info, "`importjs` pragma requires the JavaScript target")

compiler/wordrecg.nim

@@ -38,7 +38,8 @@ type
     wCursor = "cursor", wNoalias = "noalias",
 
     wImmediate = "immediate", wConstructor = "constructor", wDestructor = "destructor", 
-    wDelegator = "delegator", wOverride = "override", wImportCpp = "importcpp", 
+    wDelegator = "delegator", wOverride = "override", wImportCpp = "importcpp",
+    wCppNonPod = "cppNonPod", 
     wImportObjC = "importobjc", wImportCompilerProc = "importcompilerproc",
     wImportc = "importc", wImportJs = "importjs", wExportc = "exportc", wExportCpp = "exportcpp", 
     wExportNims = "exportnims",

config/nim.cfg

@@ -114,8 +114,6 @@ path="$lib/pure"
   @if bsd:
     # BSD got posix_spawn only recently, so we deactivate it for osproc:
     define:useFork
-    # at least NetBSD has problems with thread local storage:
-    tlsEmulation:on
   @elif haiku:
     gcc.options.linker = "-Wl,--as-needed -lnetwork"
     gcc.cpp.options.linker = "-Wl,--as-needed -lnetwork"
@@ -167,9 +165,13 @@ path="$lib/pure"
 
 gcc.maxerrorsimpl = "-fmax-errors=3"
 
+@if bsd:
+  # at least NetBSD has problems with thread local storage:
+  tlsEmulation:on
+@end
+
 @if macosx or freebsd or openbsd:
   cc = clang
-  tlsEmulation:on
   gcc.options.always %= "-w ${gcc.maxerrorsimpl}"
   gcc.cpp.options.always %= "-w ${gcc.maxerrorsimpl} -fpermissive"
 @elif windows:

doc/manual.rst

@@ -7214,6 +7214,19 @@ will generate this code:
   __interrupt void myinterrupt()
 
 
+`cppNonPod` pragma
+------------------
+
+The `.cppNonPod` pragma should be used for non-POD `importcpp` types so that they
+work properly (in particular regarding constructor and destructor) for
+`.threadvar` variables. This requires `--tlsEmulation:off`.
+
+.. code-block:: nim
+  type Foo {.cppNonPod, importcpp, header: "funs.h".} = object
+    x: cint
+  proc main()=
+    var a {.threadvar.}: Foo
+
 InjectStmt pragma
 -----------------
 

lib/nimbase.h

@@ -143,6 +143,10 @@ __AVR__
 #  error "Cannot define NIM_THREADVAR"
 #endif
 
+#if defined(__cplusplus)
+  #define NIM_THREAD_LOCAL thread_local
+#endif
+
 /* --------------- how int64 constants should be declared: ----------- */
 #if defined(__GNUC__) || defined(__LCC__) || \
     defined(__POCC__) || defined(__DMC__) || defined(_MSC_VER)

tests/benchmarks/readme.md

@@ -0,0 +1,5 @@
+# Collection of benchmarks
+
+In future work, benchmarks can be added to CI, but for now we provide benchmarks that can be run locally.
+
+See RFC: https://github.com/timotheecour/Nim/issues/425

tests/benchmarks/ttls.nim

@@ -0,0 +1,30 @@
+discard """
+  action: compile
+"""
+
+#[
+## on osx
+nim r -d:danger --threads --tlsEmulation:off tests/benchmarks/ttls.nim
+9.999999999992654e-07
+
+ditto with `--tlsEmulation:on`:
+0.216999
+]#
+
+import times
+
+proc main2(): int =
+  var g0 {.threadvar.}: int
+  g0.inc
+  result = g0
+
+proc main =
+  let n = 100_000_000
+  var c = 0
+  let t = cpuTime()
+  for i in 0..<n:
+    c += main2()
+  let t2 = cpuTime() - t
+  doAssert c != 0
+  echo t2
+main()

tests/misc/mtlsemulation.h

@@ -0,0 +1,37 @@
+#include <stdio.h>
+
+struct Foo1 {
+  /*
+  uncommenting would give:
+  error: initializer for thread-local variable must be a constant expression
+  N_LIB_PRIVATE NIM_THREADVAR Foo1 g1__9brEZhPEldbVrNpdRGmWESA;
+  */
+  // Foo1() noexcept { }
+
+  /*
+  uncommenting would give:
+  error: type of thread-local variable has non-trivial destruction
+  */
+  // ~Foo1() { }
+  int x;
+};
+
+struct Foo2 {
+  Foo2() noexcept { }
+  ~Foo2() { }
+  int x;
+};
+
+static int ctorCalls = 0;
+static int dtorCalls = 0;
+
+struct Foo3 {
+  Foo3() noexcept {
+    ctorCalls = ctorCalls + 1;
+    x = 10;
+  }
+  ~Foo3() {
+    dtorCalls = dtorCalls + 1;
+  }
+  int x;
+};

tests/misc/ttlsemulation.nim

@@ -0,0 +1,75 @@
+discard """
+  matrix: "-d:nimTtlsemulationCase1 --threads --tlsEmulation:on; -d:nimTtlsemulationCase2 --threads --tlsEmulation:off; -d:nimTtlsemulationCase3 --threads"
+  targets: "c cpp"
+"""
+
+#[
+tests for: `.cppNonPod`, `--tlsEmulation`
+]#
+
+import std/sugar
+
+block:
+  # makes sure the logic in config/nim.cfg or testament doesn't interfere with `--tlsEmulation` so we test the right thing.
+  when defined(nimTtlsemulationCase1):
+    doAssert compileOption("tlsEmulation")
+  elif defined(nimTtlsemulationCase2):
+    doAssert not compileOption("tlsEmulation")
+  elif defined(nimTtlsemulationCase3):
+    when defined(osx):
+      doAssert not compileOption("tlsEmulation")
+  else:
+    doAssert false
+
+block:
+  proc main1(): int =
+    var g0 {.threadvar.}: int
+    g0.inc
+    g0
+  let s = collect:
+    for i in 0..<3: main1()
+  doAssert s == @[1,2,3]
+
+when defined(cpp): # bug #16752
+  when defined(windows) and defined(nimTtlsemulationCase2):
+    discard # xxx this failed with exitCode 1
+  else:
+    type Foo1 {.importcpp: "Foo1", header: "mtlsemulation.h".} = object
+      x: cint
+    type Foo2 {.cppNonPod, importcpp: "Foo2", header: "mtlsemulation.h".} = object
+      x: cint
+
+    var ctorCalls {.importcpp.}: cint
+    var dtorCalls {.importcpp.}: cint
+    type Foo3 {.cppNonPod, importcpp: "Foo3", header: "mtlsemulation.h".} = object
+      x: cint
+
+    proc sub(i: int) =
+      var g1 {.threadvar.}: Foo1
+      var g2 {.threadvar.}: Foo2
+      var g3 {.threadvar.}: Foo3
+      discard g1
+      discard g2
+
+      # echo (g3.x, ctorCalls, dtorCalls)
+      when compileOption("tlsEmulation"):
+        # xxx bug
+        discard
+      else:
+        doAssert g3.x.int == 10 + i
+        doAssert ctorCalls == 2
+      doAssert dtorCalls == 1
+      g3.x.inc
+
+    proc main() =
+      doAssert ctorCalls == 0
+      doAssert dtorCalls == 0
+      block:
+        var f3: Foo3
+        doAssert f3.x == 10
+      doAssert ctorCalls == 1
+      doAssert dtorCalls == 1
+
+      for i in 0..<3:
+        sub(i)
+    main()