[MLton-commit] r6183
Matthew Fluet
fluet at mlton.org
Mon Nov 19 18:58:39 PST 2007
Fixed a bug in handling of weak pointers by the mark-compact garbage
collector.
When threading pointers in 'updateForwardPointersForMarkCompact', we
call 'clearIfWeakAndUnmarkedForMarkCompact' and also compress a
sequence of unmarked objects into one (word8) vector. There is a bug
here, because 'clearIfWeakAndUnmarkedForMarkCompact' inspects the
header of the object pointer to determine if it is still live.
However, if the object pointer is unmarked and compressed into a
(word8) vector, then 'clearIfWeakAndUnmarkedForMarkCompact' might see
the counter word of the compressed (word8) vector, because
the normal object layout:
header :: <>(non heap-pointers)* :: (heap pointers)* :::: ...
becomes
counter :: <>length :: header :: (word8)*
where <> denotes the location pointed to by the object pointer of the
weak pointer.
While I think it would be correct to always clear the weak pointer if
the inspected header is 0 (equal to the newly written counter word),
it seems cleaner to process all the weak pointers before threading
pointers. To do so, 'dfsMarkByMode' takes a 'bool shouldLinkWeaks';
when true (as in the mark-compact marking phase), during the dfs, all
encountered weaks are linked. The
'clearIfWeakAndUnmarkedForMarkCompact' function is replaced by a
'updateWeaksForMarkCompact' function, which clears weak pointers whose
object pointer points to an unmarked object. This function
constitutes a new phase that runs after marking and before pointer
threading.
This should also be more efficient, since the overhead to handle weak
pointers is now proportional to the number of (live) weak pointers in
the heap, rather than proportional to the number of (live) objects in
the heap.
Thanks to Sean McLaughlin for the bug report and Florian Weimer for
the initial diagnosis.
----------------------------------------------------------------------
U mlton/trunk/doc/changelog
U mlton/trunk/runtime/gc/dfs-mark.c
U mlton/trunk/runtime/gc/dfs-mark.h
U mlton/trunk/runtime/gc/mark-compact.c
U mlton/trunk/runtime/gc/mark-compact.h
U mlton/trunk/runtime/gc/share.c
U mlton/trunk/runtime/gc/size.c
U mlton/trunk/runtime/gc/weak.h
----------------------------------------------------------------------
Modified: mlton/trunk/doc/changelog
===================================================================
--- mlton/trunk/doc/changelog 2007-11-20 02:52:38 UTC (rev 6182)
+++ mlton/trunk/doc/changelog 2007-11-20 02:58:38 UTC (rev 6183)
@@ -1,11 +1,15 @@
Here are the changes from version 20070826 to version YYYYMMDD.
+* 2007-11-19
+ - Fixed bug in handling of weak pointers by the mark-compact
+ garbage collector. Thanks to Sean McLaughlin for the bug report
+ and Florian Weimer for the initial diagnosis.
+
* 2007-11-07
- Added %posint command to ml-lex, to set the yypos type and allow
the lexing of multi-gigabyte input files. Thanks to Florian
Weimer for the feature concept and original patch.
-
* 2007-11-07
- Added command-line switch -mlb-path-var '<name> <value>' for
specifying MLB path variables.
Modified: mlton/trunk/runtime/gc/dfs-mark.c
===================================================================
--- mlton/trunk/runtime/gc/dfs-mark.c 2007-11-20 02:52:38 UTC (rev 6182)
+++ mlton/trunk/runtime/gc/dfs-mark.c 2007-11-20 02:58:38 UTC (rev 6183)
@@ -25,7 +25,7 @@
}
}
-/* dfsMarkByMode (s, r, m, shc)
+/* dfsMarkByMode (s, r, m, shc, slw)
*
* Sets all the mark bits in the object graph pointed to by r.
*
@@ -34,10 +34,14 @@
*
* If shc, it hash-conses the objects marked.
*
+ * If slw, it links the weak objects marked.
+ *
* It returns the total size in bytes of the objects marked.
*/
size_t dfsMarkByMode (GC_state s, pointer root,
- GC_markMode mode, bool shouldHashCons) {
+ GC_markMode mode,
+ bool shouldHashCons,
+ bool shouldLinkWeaks) {
GC_header mark; /* Used to set or clear the mark bit. */
size_t size; /* Total number of bytes marked. */
pointer cur; /* The current object being marked. */
@@ -158,6 +162,23 @@
goto markNext;
} else if (WEAK_TAG == tag) {
/* Store the marked header and don't follow any pointers. */
+ if (shouldLinkWeaks) {
+ GC_weak w;
+
+ w = (GC_weak)(cur + offsetofWeak (s));
+ if (DEBUG_WEAK)
+ fprintf (stderr, "marking weak "FMTPTR" ",
+ (uintptr_t)w);
+ if (isObjptr (w->objptr)) {
+ if (DEBUG_WEAK)
+ fprintf (stderr, "linking\n");
+ w->link = s->weaks;
+ s->weaks = w;
+ } else {
+ if (DEBUG_WEAK)
+ fprintf (stderr, "not linking\n");
+ }
+ }
goto ret;
} else if (ARRAY_TAG == tag) {
/* When marking arrays:
@@ -337,23 +358,23 @@
assert (FALSE);
}
-void dfsMarkWithHashCons (GC_state s, objptr *opp) {
+void dfsMarkWithHashConsWithLinkWeaks (GC_state s, objptr *opp) {
pointer p;
p = objptrToPointer (*opp, s->heap.start);
- dfsMarkByMode (s, p, MARK_MODE, TRUE);
+ dfsMarkByMode (s, p, MARK_MODE, TRUE, TRUE);
}
-void dfsMarkWithoutHashCons (GC_state s, objptr *opp) {
+void dfsMarkWithoutHashConsWithLinkWeaks (GC_state s, objptr *opp) {
pointer p;
p = objptrToPointer (*opp, s->heap.start);
- dfsMarkByMode (s, p, MARK_MODE, FALSE);
+ dfsMarkByMode (s, p, MARK_MODE, FALSE, TRUE);
}
void dfsUnmark (GC_state s, objptr *opp) {
pointer p;
p = objptrToPointer (*opp, s->heap.start);
- dfsMarkByMode (s, p, UNMARK_MODE, FALSE);
+ dfsMarkByMode (s, p, UNMARK_MODE, FALSE, FALSE);
}
Modified: mlton/trunk/runtime/gc/dfs-mark.h
===================================================================
--- mlton/trunk/runtime/gc/dfs-mark.h 2007-11-20 02:52:38 UTC (rev 6182)
+++ mlton/trunk/runtime/gc/dfs-mark.h 2007-11-20 02:58:38 UTC (rev 6183)
@@ -1,4 +1,4 @@
-/* Copyright (C) 1999-2005 Henry Cejtin, Matthew Fluet, Suresh
+/* Copyright (C) 1999-2007 Henry Cejtin, Matthew Fluet, Suresh
* Jagannathan, and Stephen Weeks.
* Copyright (C) 1997-2000 NEC Research Institute.
*
@@ -20,9 +20,11 @@
static inline bool isPointerMarked (pointer p);
static inline bool isPointerMarkedByMode (pointer p, GC_markMode m);
static size_t dfsMarkByMode (GC_state s, pointer root,
- GC_markMode mode, bool shouldHashCons);
-static inline void dfsMarkWithHashCons (GC_state s, objptr *opp);
-static inline void dfsMarkWithoutHashCons (GC_state s, objptr *opp);
+ GC_markMode mode,
+ bool shouldHashCons,
+ bool shouldLinkWeaks);
+static inline void dfsMarkWithHashConsWithLinkWeaks (GC_state s, objptr *opp);
+static inline void dfsMarkWithoutHashConsWithLinkWeaks (GC_state s, objptr *opp);
static inline void dfsUnmark (GC_state s, objptr *opp);
#endif /* (defined (MLTON_GC_INTERNAL_FUNCS)) */
Modified: mlton/trunk/runtime/gc/mark-compact.c
===================================================================
--- mlton/trunk/runtime/gc/mark-compact.c 2007-11-20 02:52:38 UTC (rev 6182)
+++ mlton/trunk/runtime/gc/mark-compact.c 2007-11-20 02:58:38 UTC (rev 6183)
@@ -11,6 +11,10 @@
/* ---------------------------------------------------------------- */
void copyForThreadInternal (pointer dst, pointer src) {
+ if (FALSE)
+ fprintf (stderr,
+ "copyForThreadInternal dst = "FMTPTR" src = "FMTPTR"\n",
+ (uintptr_t)dst, (uintptr_t)src);
if (OBJPTR_SIZE > GC_HEADER_SIZE) {
size_t count;
@@ -56,38 +60,31 @@
copyForThreadInternal ((pointer)(headerp), (pointer)(&opop));
}
-/* If p is weak, the object pointer was valid, and points to an
- * unmarked object, then clear the object pointer.
+/* If the object pointer is valid, and points to an unmarked object,
+ * then clear the object pointer.
*/
-void clearIfWeakAndUnmarkedForMarkCompact (GC_state s, pointer p) {
- GC_header header;
- GC_header *headerp;
- uint16_t bytesNonObjptrs, numObjptrs;
- GC_objectTypeTag tag;
+void updateWeaksForMarkCompact (GC_state s) {
+ pointer p;
+ GC_weak w;
- headerp = getHeaderp (p);
- header = *headerp;
- splitHeader(s, *headerp, &tag, NULL, &bytesNonObjptrs, &numObjptrs);
- if (WEAK_TAG == tag and 1 == numObjptrs) {
- GC_header objptrHeader;
- GC_weak w;
+ for (w = s->weaks; w != NULL; w = w->link) {
+ assert (BOGUS_OBJPTR != w->objptr);
- if (DEBUG_MARK_COMPACT or DEBUG_WEAK)
- fprintf (stderr, "clearIfWeakAndUnmarkedForMarkCompact ("FMTPTR") header = "FMTHDR"\n",
- (uintptr_t)p, header);
- w = (GC_weak)(p + offsetofWeak (s));
- objptrHeader = getHeader (objptrToPointer(w->objptr, s->heap.start));
- /* If it's not threaded and unmarked, clear the weak pointer. */
- if ((GC_VALID_HEADER_MASK & objptrHeader)
- and not (MARK_MASK & objptrHeader)) {
- w->objptr = BOGUS_OBJPTR;
- header = GC_WEAK_GONE_HEADER | MARK_MASK;
+ if (DEBUG_WEAK)
+ fprintf (stderr, "updateWeaksForMarkCompact w = "FMTPTR" ", (uintptr_t)w);
+ p = objptrToPointer(w->objptr, s->heap.start);
+ /* If it's unmarked, clear the weak pointer. */
+ if (isPointerMarked(p)) {
if (DEBUG_WEAK)
- fprintf (stderr, "cleared. new header = "FMTHDR"\n",
- header);
- *headerp = header;
+ fprintf (stderr, "not cleared\n");
+ } else {
+ if (DEBUG_WEAK)
+ fprintf (stderr, "cleared\n");
+ *(getHeaderp((pointer)w - offsetofWeak (s))) = GC_WEAK_GONE_HEADER | MARK_MASK;
+ w->objptr = BOGUS_OBJPTR;
}
}
+ s->weaks = NULL;
}
void updateForwardPointersForMarkCompact (GC_state s) {
@@ -122,7 +119,6 @@
* Thread internal pointers.
*/
thread:
- clearIfWeakAndUnmarkedForMarkCompact (s, p);
size = sizeofObject (s, p);
if (DEBUG_MARK_COMPACT)
fprintf (stderr, "threading "FMTPTR" of size %"PRIuMAX"\n",
@@ -296,11 +292,12 @@
s->lastMajorStatistics.bytesHashConsed = 0;
s->cumulativeStatistics.numHashConsGCs++;
s->objectHashTable = allocHashTable (s);
- foreachGlobalObjptr (s, dfsMarkWithHashCons);
+ foreachGlobalObjptr (s, dfsMarkWithHashConsWithLinkWeaks);
freeHashTable (s->objectHashTable);
} else {
- foreachGlobalObjptr (s, dfsMarkWithoutHashCons);
+ foreachGlobalObjptr (s, dfsMarkWithoutHashConsWithLinkWeaks);
}
+ updateWeaksForMarkCompact (s);
foreachGlobalObjptr (s, threadInternalObjptr);
updateForwardPointersForMarkCompact (s);
updateBackwardPointersAndSlideForMarkCompact (s);
Modified: mlton/trunk/runtime/gc/mark-compact.h
===================================================================
--- mlton/trunk/runtime/gc/mark-compact.h 2007-11-20 02:52:38 UTC (rev 6182)
+++ mlton/trunk/runtime/gc/mark-compact.h 2007-11-20 02:58:38 UTC (rev 6183)
@@ -1,4 +1,4 @@
-/* Copyright (C) 1999-2005 Henry Cejtin, Matthew Fluet, Suresh
+/* Copyright (C) 1999-2007 Henry Cejtin, Matthew Fluet, Suresh
* Jagannathan, and Stephen Weeks.
* Copyright (C) 1997-2000 NEC Research Institute.
*
@@ -10,7 +10,7 @@
static inline void copyForThreadInternal (pointer dst, pointer src);
static inline void threadInternalObjptr (GC_state s, objptr *opp);
-static inline void clearIfWeakAndUnmarkedForMarkCompact (GC_state s, pointer p);
+static inline void updateWeaksForMarkCompact (GC_state s);
static void updateForwardPointersForMarkCompact (GC_state s);
static void updateBackwardPointersAndSlideForMarkCompact (GC_state s);
static void majorMarkCompactGC (GC_state s);
Modified: mlton/trunk/runtime/gc/share.c
===================================================================
--- mlton/trunk/runtime/gc/share.c 2007-11-20 02:52:38 UTC (rev 6182)
+++ mlton/trunk/runtime/gc/share.c 2007-11-20 02:58:38 UTC (rev 6183)
@@ -1,4 +1,4 @@
-/* Copyright (C) 1999-2006 Henry Cejtin, Matthew Fluet, Suresh
+/* Copyright (C) 1999-2007 Henry Cejtin, Matthew Fluet, Suresh
* Jagannathan, and Stephen Weeks.
* Copyright (C) 1997-2000 NEC Research Institute.
*
@@ -14,10 +14,10 @@
if (DEBUG_SHARE or s->controls.messages)
s->lastMajorStatistics.bytesHashConsed = 0;
// Don't hash cons during the first round of marking.
- total = dfsMarkByMode (s, object, MARK_MODE, FALSE);
+ total = dfsMarkByMode (s, object, MARK_MODE, FALSE, FALSE);
s->objectHashTable = allocHashTable (s);
- // Hash cons during the second round of marking.
- dfsMarkByMode (s, object, UNMARK_MODE, TRUE);
+ // Hash cons during the second round of (un)marking.
+ dfsMarkByMode (s, object, UNMARK_MODE, TRUE, FALSE);
freeHashTable (s->objectHashTable);
if (DEBUG_SHARE or s->controls.messages)
printBytesHashConsedMessage (s, total);
Modified: mlton/trunk/runtime/gc/size.c
===================================================================
--- mlton/trunk/runtime/gc/size.c 2007-11-20 02:52:38 UTC (rev 6182)
+++ mlton/trunk/runtime/gc/size.c 2007-11-20 02:58:38 UTC (rev 6183)
@@ -1,4 +1,4 @@
-/* Copyright (C) 1999-2005 Henry Cejtin, Matthew Fluet, Suresh
+/* Copyright (C) 1999-2007 Henry Cejtin, Matthew Fluet, Suresh
* Jagannathan, and Stephen Weeks.
* Copyright (C) 1997-2000 NEC Research Institute.
*
@@ -11,9 +11,9 @@
if (DEBUG_SIZE)
fprintf (stderr, "GC_size marking\n");
- res = dfsMarkByMode (s, root, MARK_MODE, FALSE);
+ res = dfsMarkByMode (s, root, MARK_MODE, FALSE, FALSE);
if (DEBUG_SIZE)
fprintf (stderr, "GC_size unmarking\n");
- dfsMarkByMode (s, root, UNMARK_MODE, FALSE);
+ dfsMarkByMode (s, root, UNMARK_MODE, FALSE, FALSE);
return res;
}
Modified: mlton/trunk/runtime/gc/weak.h
===================================================================
--- mlton/trunk/runtime/gc/weak.h 2007-11-20 02:52:38 UTC (rev 6182)
+++ mlton/trunk/runtime/gc/weak.h 2007-11-20 02:58:38 UTC (rev 6183)
@@ -24,7 +24,7 @@
* There may be zero or more bytes of padding for alignment purposes.
*
* The link native-pointer is used to chain the live weaks together
- * during a copying gc and is otherwise unused.
+ * during a copying/mark-compact gc and is otherwise unused.
*
* The final component is the weak object-pointer.
*
More information about the MLton-commit
mailing list