GC: enable precise global/thread local storage tracing

4 files changed, 46 insertions, 62 deletions

diff --git a/lib/system/gc.nim b/lib/system/gc.nim
index cd5c6870d..66d49ce1b 100644
--- a/lib/system/gc.nim
+++ b/lib/system/gc.nim
@@ -92,7 +92,7 @@ type
     additionalRoots: CellSeq # dummy roots for GC_ref/unref
     when hasThreadSupport:
       toDispose: SharedList[pointer]
-    isMainThread: bool
+    gcThreadId: int
 
 var
   gch {.rtlThreadVar.}: GcHeap
@@ -159,12 +159,12 @@ when defined(logGC):
         if not c.typ.name.isNil:
           typName = c.typ.name
 
-  when leakDetector:
-    c_fprintf(stdout, "[GC] %s: %p %d %s rc=%ld from %s(%ld)\n",
-              msg, c, kind, typName, c.refcount shr rcShift, c.filename, c.line)
-  else:
-    c_fprintf(stdout, "[GC] %s: %p %d %s rc=%ld; color=%ld\n",
-              msg, c, kind, typName, c.refcount shr rcShift, c.color)
+    when leakDetector:
+      c_fprintf(stdout, "[GC] %s: %p %d %s rc=%ld from %s(%ld)\n",
+                msg, c, kind, typName, c.refcount shr rcShift, c.filename, c.line)
+    else:
+      c_fprintf(stdout, "[GC] %s: %p %d %s rc=%ld; thread=%ld\n",
+                msg, c, kind, typName, c.refcount shr rcShift, gch.gcThreadId)
 
 template gcTrace(cell, state: untyped) =
   when traceGC: traceCell(cell, state)
@@ -312,7 +312,8 @@ proc initGC() =
     init(gch.additionalRoots)
     when hasThreadSupport:
       init(gch.toDispose)
-    gch.isMainThread = true
+    gch.gcThreadId = atomicInc(gHeapidGenerator) - 1
+    gcAssert(gch.gcThreadId >= 0, "invalid computed thread ID")
 
 proc cellsetReset(s: var CellSet) =
   deinit(s)
@@ -459,7 +460,7 @@ proc rawNewObj(typ: PNimType, size: int, gch: var GcHeap): pointer =
   release(gch)
   when useCellIds:
     inc gch.idGenerator
-    res.id = gch.idGenerator
+    res.id = gch.idGenerator * 1000_000 + gch.gcThreadId
   result = cellToUsr(res)
   sysAssert(allocInv(gch.region), "rawNewObj end")
 
@@ -506,7 +507,7 @@ proc newObjRC1(typ: PNimType, size: int): pointer {.compilerRtl.} =
   release(gch)
   when useCellIds:
     inc gch.idGenerator
-    res.id = gch.idGenerator
+    res.id = gch.idGenerator * 1000_000 + gch.gcThreadId
   result = cellToUsr(res)
   zeroMem(result, size)
   sysAssert(allocInv(gch.region), "newObjRC1 end")
@@ -576,7 +577,7 @@ proc growObj(old: pointer, newsize: int, gch: var GcHeap): pointer =
   release(gch)
   when useCellIds:
     inc gch.idGenerator
-    res.id = gch.idGenerator
+    res.id = gch.idGenerator * 1000_000 + gch.gcThreadId
   result = cellToUsr(res)
   sysAssert(allocInv(gch.region), "growObj end")
   when defined(memProfiler): nimProfile(newsize-oldsize)
@@ -621,7 +622,7 @@ proc markS(gch: var GcHeap, c: PCell) =
       forAllChildren(d, waMarkPrecise)
 
 proc markGlobals(gch: var GcHeap) =
-  if gch.isMainThread:
+  if gch.gcThreadId == 0:
     for i in 0 .. globalMarkersLen-1: globalMarkers[i]()
   for i in 0 .. threadLocalMarkersLen-1: threadLocalMarkers[i]()
   let d = gch.additionalRoots.d
@@ -669,13 +670,7 @@ proc doOperation(p: pointer, op: WalkOp) =
   of waPush:
     add(gch.tempStack, c)
   of waMarkGlobal:
-    when hasThreadSupport:
-      # could point to a cell which we don't own and don't want to touch/trace
-      # XXX: This should not be required anymore!
-      if isAllocatedPtr(gch.region, c):
-        markS(gch, c)
-    else:
-      markS(gch, c)
+    markS(gch, c)
   of waMarkPrecise:
     add(gch.tempStack, c)
   #of waDebug: debugGraph(c)
diff --git a/lib/system/gc2.nim b/lib/system/gc2.nim
index cd90c6d62..ca2a35f60 100644
--- a/lib/system/gc2.nim
+++ b/lib/system/gc2.nim
@@ -104,7 +104,7 @@ type
     pDumpHeapFile: pointer # File that is used for GC_dumpHeap
     when hasThreadSupport:
       toDispose: SharedList[pointer]
-    isMainThread: bool
+    gcThreadId: int
 
 var
   gch {.rtlThreadVar.}: GcHeap
@@ -120,23 +120,6 @@ template release(gch: GcHeap) =
   when hasThreadSupport and hasSharedHeap:
     releaseSys(HeapLock)
 
-proc initGC() =
-  when not defined(useNimRtl):
-    gch.red = (1-gch.black)
-    gch.cycleThreshold = InitialCycleThreshold
-    gch.stat.stackScans = 0
-    gch.stat.completedCollections = 0
-    gch.stat.maxThreshold = 0
-    gch.stat.maxStackSize = 0
-    gch.stat.maxStackCells = 0
-    gch.stat.cycleTableSize = 0
-    # init the rt
-    init(gch.additionalRoots)
-    init(gch.greyStack)
-    when hasThreadSupport:
-      init(gch.toDispose)
-    gch.isMainThread = true
-
 # Which color to use for new objects is tricky: When we're marking,
 # they have to be *white* so that everything is marked that is only
 # reachable from them. However, when we are sweeping, they have to
@@ -342,6 +325,24 @@ proc gcInvariant*() =
 
 include gc_common
 
+proc initGC() =
+  when not defined(useNimRtl):
+    gch.red = (1-gch.black)
+    gch.cycleThreshold = InitialCycleThreshold
+    gch.stat.stackScans = 0
+    gch.stat.completedCollections = 0
+    gch.stat.maxThreshold = 0
+    gch.stat.maxStackSize = 0
+    gch.stat.maxStackCells = 0
+    gch.stat.cycleTableSize = 0
+    # init the rt
+    init(gch.additionalRoots)
+    init(gch.greyStack)
+    when hasThreadSupport:
+      init(gch.toDispose)
+    gch.gcThreadId = atomicInc(gHeapidGenerator) - 1
+    gcAssert(gch.gcThreadId >= 0, "invalid computed thread ID")
+
 proc rawNewObj(typ: PNimType, size: int, gch: var GcHeap): pointer =
   # generates a new object and sets its reference counter to 0
   sysAssert(allocInv(gch.region), "rawNewObj begin")
@@ -480,7 +481,7 @@ proc GC_dumpHeap*(file: File) =
         c_fprintf(file, "onstack %p\n", d[i])
       else:
         c_fprintf(file, "onstack_invalid %p\n", d[i])
-  if gch.isMainThread:
+  if gch.gcThreadId == 0:
     for i in 0 .. globalMarkersLen-1: globalMarkers[i]()
   for i in 0 .. threadLocalMarkersLen-1: threadLocalMarkers[i]()
   while true:
@@ -569,7 +570,7 @@ proc markIncremental(gch: var GcHeap): bool =
   result = true
 
 proc markGlobals(gch: var GcHeap) =
-  if gch.isMainThread:
+  if gch.gcThreadId == 0:
     for i in 0 .. globalMarkersLen-1: globalMarkers[i]()
   for i in 0 .. threadLocalMarkersLen-1: threadLocalMarkers[i]()
 
@@ -591,22 +592,14 @@ proc doOperation(p: pointer, op: WalkOp) =
         markRoot(gch, c)
       else:
         dumpRoot(gch, c)
-    when hasThreadSupport:
-      # could point to a cell which we don't own and don't want to touch/trace
-      if isAllocatedPtr(gch.region, c): handleRoot()
-    else:
-      #gcAssert(isAllocatedPtr(gch.region, c), "doOperation: waMarkGlobal")
+    handleRoot()
+    discard allocInv(gch.region)
+  of waMarkGrey:
+    when false:
       if not isAllocatedPtr(gch.region, c):
-        c_fprintf(stdout, "[GC] not allocated anymore: MarkGlobal %p\n", c)
+        c_fprintf(stdout, "[GC] not allocated anymore: MarkGrey %p\n", c)
         #GC_dumpHeap()
         sysAssert(false, "wtf")
-      handleRoot()
-    discard allocInv(gch.region)
-  of waMarkGrey:
-    if not isAllocatedPtr(gch.region, c):
-      c_fprintf(stdout, "[GC] not allocated anymore: MarkGrey %p\n", c)
-      #GC_dumpHeap()
-      sysAssert(false, "wtf")
     if c.color == 1-gch.black:
       c.setColor(rcGrey)
       add(gch.greyStack, c)
diff --git a/lib/system/gc_common.nim b/lib/system/gc_common.nim
index f0abd918a..ad37df0e0 100644
--- a/lib/system/gc_common.nim
+++ b/lib/system/gc_common.nim
@@ -401,6 +401,7 @@ var
   globalMarkers: array[0.. 3499, GlobalMarkerProc]
   threadLocalMarkersLen: int
   threadLocalMarkers: array[0.. 3499, GlobalMarkerProc]
+  gHeapidGenerator: int
 
 proc nimRegisterGlobalMarker(markerProc: GlobalMarkerProc) {.compilerProc.} =
   if globalMarkersLen <= high(globalMarkers):
diff --git a/lib/system/gc_ms.nim b/lib/system/gc_ms.nim
index 0754f2cfe..101185dfb 100644
--- a/lib/system/gc_ms.nim
+++ b/lib/system/gc_ms.nim
@@ -73,7 +73,7 @@ type
     stat: GcStat
     when hasThreadSupport:
       toDispose: SharedList[pointer]
-    isMainThread: bool
+    gcThreadId: int
     additionalRoots: CellSeq # dummy roots for GC_ref/unref
 
 var
@@ -220,7 +220,8 @@ proc initGC() =
       init(gch.marked)
     when hasThreadSupport:
       init(gch.toDispose)
-    gch.isMainThread = true
+    gch.gcThreadId = atomicInc(gHeapidGenerator) - 1
+    gcAssert(gch.gcThreadId >= 0, "invalid computed thread ID")
 
 proc forAllSlotsAux(dest: pointer, n: ptr TNimNode, op: WalkOp) {.benign.} =
   var d = cast[ByteAddress](dest)
@@ -394,13 +395,7 @@ proc doOperation(p: pointer, op: WalkOp) =
   var c: PCell = usrToCell(p)
   gcAssert(c != nil, "doOperation: 1")
   case op
-  of waMarkGlobal:
-    when hasThreadSupport:
-      # could point to a cell which we don't own and don't want to touch/trace
-      if isAllocatedPtr(gch.region, c):
-        mark(gch, c)
-    else:
-      mark(gch, c)
+  of waMarkGlobal: mark(gch, c)
   of waMarkPrecise: add(gch.tempStack, c)
 
 proc nimGCvisit(d: pointer, op: int) {.compilerRtl.} =
@@ -437,7 +432,7 @@ when false:
           quit 1
 
 proc markGlobals(gch: var GcHeap) =
-  if gch.isMainThread:
+  if gch.gcThreadId == 0:
     for i in 0 .. globalMarkersLen-1: globalMarkers[i]()
   for i in 0 .. threadLocalMarkersLen-1: threadLocalMarkers[i]()
   let d = gch.additionalRoots.d