C code gen: generate even better code for the GC

compiler/ccgexprs.nim

@@ -231,11 +231,15 @@ proc genAssignment(p: BProc, dest, src: TLoc, flags: TAssignmentFlags) =
       genRefAssign(p, dest, src, flags)
     else:
       if dest.s == OnStack or optRefcGC notin gGlobalOptions:
-        appcg(p, cpsStmts, "$1 = #copyString($2);$n", [rdLoc(dest), rdLoc(src)])
+        appcg(p, cpsStmts, "$1 = #copyString($2);$n", [dest.rdLoc, src.rdLoc])
         if needToKeepAlive in flags: keepAlive(p, dest)
       elif dest.s == OnHeap:
-        appcg(p, cpsStmts, "#asgnRefNoCycle((void**) $1, #copyString($2));$n",
-             [addrLoc(dest), rdLoc(src)])
+        # we use a temporary to care for the dreaded self assignment:
+        var tmp: TLoc
+        getTemp(p, ty, tmp)
+        appcg(p, cpsStmts, "$3 = $1; $1 = #copyStringRC1($2);$n",
+             [dest.rdLoc, src.rdLoc, tmp.rdLoc])
+        appcg(p, cpsStmts, "if ($1) #nimGCunrefNoCycle($1);$n", tmp.rdLoc)
       else:
         appcg(p, cpsStmts, "#unsureAsgnRef((void**) $1, #copyString($2));$n",
              [addrLoc(dest), rdLoc(src)])
@@ -340,7 +344,7 @@ proc binaryExprChar(p: BProc, e: PNode, d: var TLoc, frmt: string) =
   assert(e.sons[2].typ != nil)
   InitLocExpr(p, e.sons[1], a)
   InitLocExpr(p, e.sons[2], b)
-  putIntoDest(p, d, e.typ, ropecg(p.module, frmt, [rdCharLoc(a), rdCharLoc(b)]))
+  putIntoDest(p, d, e.typ, ropecg(p.module, frmt, [a.rdCharLoc, b.rdCharLoc]))
 
 proc unaryExpr(p: BProc, e: PNode, d: var TLoc, frmt: string) =
   var a: TLoc
@@ -904,7 +908,10 @@ proc genNew(p: BProc, e: PNode) =
               getTypeDesc(p.module, skipTypes(reftype.sons[0], abstractRange))]
   if a.s == OnHeap and optRefcGc in gGlobalOptions:
     # use newObjRC1 as an optimization; and we don't need 'keepAlive' either
-    appcg(p, cpsStmts, "if ($1) #nimGCunref($1);$n", a.rdLoc)
+    if canFormAcycle(a.t):
+      appcg(p, cpsStmts, "if ($1) #nimGCunref($1);$n", a.rdLoc)
+    else:
+      appcg(p, cpsStmts, "if ($1) #nimGCunrefNoCycle($1);$n", a.rdLoc)
     b.r = ropecg(p.module, "($1) #newObjRC1($2, sizeof($3))", args)
     appcg(p, cpsStmts, "$1 = $2;$n", a.rdLoc, b.rdLoc)
   else:
@@ -920,7 +927,7 @@ proc genNewSeqAux(p: BProc, dest: TLoc, length: PRope) =
   var call: TLoc
   initLoc(call, locExpr, dest.t, OnHeap)
   if dest.s == OnHeap and optRefcGc in gGlobalOptions:
-    appcg(p, cpsStmts, "if ($1) #nimGCunref($1);$n", dest.rdLoc)
+    appcg(p, cpsStmts, "if ($1) #nimGCunrefNoCycle($1);$n", dest.rdLoc)
     call.r = ropecg(p.module, "($1) #newSeqRC1($2, $3)", args)
     appcg(p, cpsStmts, "$1 = $2;$n", dest.rdLoc, call.rdLoc)
   else:

lib/system/gc.nim

@@ -230,7 +230,8 @@ proc decRef(c: PCell) {.inline.} =
   if --c.refcount:
     rtlAddZCT(c)
   elif canBeCycleRoot(c):
-    # XXX if 'incRef' does this check, it should be unnecessary in 'decRef'
+    # unfortunately this is necessary here too, because a cycle might just
+    # have been broken up and we could recycle it.
     rtlAddCycleRoot(c) 
 
 proc incRef(c: PCell) {.inline.} = 
@@ -241,6 +242,11 @@ proc incRef(c: PCell) {.inline.} =
 proc nimGCref(p: pointer) {.compilerProc, inline.} = incRef(usrToCell(p))
 proc nimGCunref(p: pointer) {.compilerProc, inline.} = decRef(usrToCell(p))
 
+proc nimGCunrefNoCycle(p: pointer) {.compilerProc, inline.} =
+  var c = usrToCell(p)
+  if --c.refcount:
+    rtlAddZCT(c)
+
 proc asgnRef(dest: ppointer, src: pointer) {.compilerProc, inline.} =
   # the code generator calls this proc!
   sysAssert(not isOnStack(dest), "asgnRef")
@@ -252,7 +258,7 @@ proc asgnRef(dest: ppointer, src: pointer) {.compilerProc, inline.} =
 proc asgnRefNoCycle(dest: ppointer, src: pointer) {.compilerProc, inline.} =
   # the code generator calls this proc if it is known at compile time that no 
   # cycle is possible.
-  if src != nil: 
+  if src != nil:
     var c = usrToCell(src)
     ++c.refcount
   if dest[] != nil: 
@@ -761,7 +767,7 @@ proc collectCT(gch: var TGcHeap) =
     inc(gch.stat.stackScans)
     collectZCT(gch)
     when cycleGC:
-      if getOccupiedMem() >= gch.cycleThreshold or stressGC:
+      if getOccupiedMem(gch.region) >= gch.cycleThreshold or stressGC:
         collectCycles(gch)
         collectZCT(gch)
         inc(gch.stat.cycleCollections)

lib/system/sysstr.nim

@@ -72,6 +72,20 @@ proc copyString(src: NimString): NimString {.compilerProc.} =
     result.len = src.len
     c_memcpy(result.data, src.data, (src.len + 1) * sizeof(Char))
 
+proc copyStringRC1(src: NimString): NimString {.compilerProc.} =
+  if src != nil:
+    var s = src.space
+    if s < 8: s = 7
+    when defined(newObjRC1):
+      result = cast[NimString](newObjRC1(addr(strDesc), sizeof(TGenericSeq) +
+                               (s+1) * sizeof(char)))
+    else:
+      result = cast[NimString](newObj(addr(strDesc), sizeof(TGenericSeq) +
+                               (s+1) * sizeof(char)))
+    result.space = s
+    result.len = src.len
+    c_memcpy(result.data, src.data, (src.len + 1) * sizeof(Char))
+
 proc hashString(s: string): int {.compilerproc.} =
   # the compiler needs exactly the same hash function!
   # this used to be used for efficient generation of string case statements

todo.txt

@@ -1,11 +1,9 @@
 version 0.8.14
 ==============
 
-- compiler should generate better code wrt GC
-  - compiler should optimize string creation
-  - marker procs for the GC
-  - need to generate code to prevent tail call optimization
-  - write barrier specialization
+- compiler/GC interaction need to generate code to prevent tail call
+  optimization
+
 - warning for implicit openArray -> varargs convention
 - implement explicit varargs; **but** ``len(varargs)`` problem remains! 
   --> solve by implicit conversion from varargs to openarray
@@ -15,7 +13,8 @@ version 0.9.0
 =============
 
 - GC: marker procs for native Nimrod GC and Boehm GC; precise stack marking;
-  escape analysis for string/seq seems to be easy to do too
+  escape analysis for string/seq seems to be easy to do too;
+  even further write barrier specialization
 - dead code elim for JS backend; 'of' operator for JS backend
 - test the sort implementation again
 - const ptr/ref