[MLton-commit] r6443

[spoons at mlton.org]

Avoid global synchronizations for some thread operations.

Avoid taking the global lock when switching threads.  Try to
ensure the stack invariant without the global lock.  Also,
when copying threads, allocate space based on the reserved size
(as opposed to used size) of the source thread and stack
objects.

----------------------------------------------------------------------

U   mlton/branches/shared-heap-multicore/runtime/gc/copy-thread.c
U   mlton/branches/shared-heap-multicore/runtime/gc/garbage-collection.c
U   mlton/branches/shared-heap-multicore/runtime/gc/switch-thread.c

----------------------------------------------------------------------

Modified: mlton/branches/shared-heap-multicore/runtime/gc/copy-thread.c
===================================================================
--- mlton/branches/shared-heap-multicore/runtime/gc/copy-thread.c	2008-03-03 15:27:16 UTC (rev 6442)
+++ mlton/branches/shared-heap-multicore/runtime/gc/copy-thread.c	2008-03-03 15:29:00 UTC (rev 6443)
@@ -38,14 +38,27 @@
   GC_thread toThread;
   GC_stack toStack;
 
-  if (DEBUG_THREADS)
-    fprintf (stderr, "GC_copyCurrentThread\n");
-  enter (s);
-  fromThread = (GC_thread)(objptrToPointer(s->currentThread, s->heap.start) 
+  if (DEBUG_THREADS or s->controls->messages)
+    fprintf (stderr, "GC_copyCurrentThread [%d]\n", Proc_processorNumber (s));
+
+  /* Used to be an ENTER here, but we don't really need to synchronize unless
+     we don't have enough room to allocate a new thread and stack. */
+  
+  /* XXX copied from enter() */
+  /* used needs to be set because the mutator has changed s->stackTop. */
+  getStackCurrent(s)->used = sizeofGCStateCurrentStackUsed (s);
+  getThreadCurrent(s)->exnStack = s->exnStack;
+
+  fromThread = (GC_thread)(objptrToPointer(s->currentThread, s->heap->start) 
                            + offsetofThread (s));
-  fromStack = (GC_stack)(objptrToPointer(fromThread->stack, s->heap.start));
-  toThread = copyThread (s, fromThread, fromStack->used);
-  toStack = (GC_stack)(objptrToPointer(toThread->stack, s->heap.start));
+  fromStack = (GC_stack)(objptrToPointer(fromThread->stack, s->heap->start));
+  toThread = copyThread (s, fromThread, fromStack->reserved);
+
+  /* Look up these again since a GC may have occurred and moved them */
+  fromThread = (GC_thread)(objptrToPointer(s->currentThread, s->heap->start) 
+                           + offsetofThread (s));
+  fromStack = (GC_stack)(objptrToPointer(fromThread->stack, s->heap->start));
+  toStack = (GC_stack)(objptrToPointer(toThread->stack, s->heap->start));
   /* The following assert is no longer true, since alignment
    * restrictions can force the reserved to be slightly larger than
    * the used.
@@ -63,30 +76,43 @@
   GC_thread fromThread;
   GC_stack fromStack;
   GC_thread toThread;
-  GC_stack toStack;
+  //GC_stack toStack;
 
   if (DEBUG_THREADS)
-    fprintf (stderr, "GC_copyThread ("FMTPTR")\n", (uintptr_t)p);
-  enter (s);
+    fprintf (stderr, "GC_copyThread ("FMTPTR") [%d]\n", (uintptr_t)p,
+             Proc_processorNumber (s));
+
+  /* Used to be an ENTER here, but we don't really need to synchronize unless
+     we don't have enough room to allocate a new thread and stack. */
+
+  /* XXX copied from enter() */
+  /* used needs to be set because the mutator has changed s->stackTop. */
+  getStackCurrent(s)->used = sizeofGCStateCurrentStackUsed (s);
+  getThreadCurrent(s)->exnStack = s->exnStack;
+
   fromThread = (GC_thread)(p + offsetofThread (s));
-  fromStack = (GC_stack)(objptrToPointer(fromThread->stack, s->heap.start));
+  fromStack = (GC_stack)(objptrToPointer(fromThread->stack, s->heap->start));
   /* The following assert is no longer true, since alignment
    * restrictions can force the reserved to be slightly larger than
    * the used.
    */
   /* assert (fromStack->reserved == fromStack->used); */
   assert (fromStack->reserved >= fromStack->used);
-  toThread = copyThread (s, fromThread, fromStack->used);
+  toThread = copyThread (s, fromThread, fromStack->reserved);
   /* The following assert is no longer true, since alignment
    * restrictions can force the reserved to be slightly larger than
    * the used.
    */
-  toStack = (GC_stack)(objptrToPointer(toThread->stack, s->heap.start));
+  //toStack = (GC_stack)(objptrToPointer(toThread->stack, s->heap->start));
   /* assert (fromStack->reserved == fromStack->used); */
-  assert (fromStack->reserved >= fromStack->used);
-  leave (s);
+  /* Can't trust fromStack to be set properly (i.e. after GC). */
+  //assert (fromStack->reserved >= fromStack->used);
+
+  /* Formerly: LEAVE2 (s, "toThread", "fromThread"); */
+
   if (DEBUG_THREADS)
-    fprintf (stderr, FMTPTR" = GC_copyThread ("FMTPTR")\n", 
-             (uintptr_t)toThread, (uintptr_t)fromThread);
+    fprintf (stderr, FMTPTR" = GC_copyThread ("FMTPTR") [%d]\n", 
+             (uintptr_t)toThread, (uintptr_t)fromThread,
+             Proc_processorNumber (s));
   return ((pointer)toThread - offsetofThread (s));
 }

Modified: mlton/branches/shared-heap-multicore/runtime/gc/garbage-collection.c
===================================================================
--- mlton/branches/shared-heap-multicore/runtime/gc/garbage-collection.c	2008-03-03 15:27:16 UTC (rev 6442)
+++ mlton/branches/shared-heap-multicore/runtime/gc/garbage-collection.c	2008-03-03 15:29:00 UTC (rev 6443)
@@ -42,24 +42,26 @@
    * after the GC.
    */
   if (mayResize)
-    resizeHeap (s, s->lastMajorStatistics.bytesLive + bytesRequested);
+    resizeHeap (s, s->lastMajorStatistics->bytesLive + bytesRequested);
   resizeHeapSecondary (s);
-  assert (s->heap.oldGenSize + bytesRequested <= s->heap.size);
+  assert (s->heap->oldGenSize + bytesRequested <= s->heap->size);
 }
 
-void growStackCurrent (GC_state s) {
+void growStackCurrent (GC_state s, bool allocInOldGen) {
   size_t size;
   GC_stack stack;
 
   size = sizeofStackGrow (s, getStackCurrent(s));
-  if (DEBUG_STACKS or s->controls.messages)
+  if (DEBUG_STACKS or s->controls->messages)
     fprintf (stderr, "[GC: Growing stack to size %s bytes.]\n",
              uintmaxToCommaString(sizeofStackWithHeaderAligned (s, size)));
-  assert (hasHeapBytesFree (s, sizeofStackWithHeaderAligned (s, size), 0));
-  stack = newStack (s, size, TRUE);
+  assert (allocInOldGen ? 
+          hasHeapBytesFree (s, sizeofStackWithHeaderAligned (s, size), 0) :
+          hasHeapBytesFree (s, 0, sizeofStackWithHeaderAligned (s, size)));
+  stack = newStack (s, size, allocInOldGen);
   copyStack (s, getStackCurrent(s), stack);
-  getThreadCurrent(s)->stack = pointerToObjptr ((pointer)stack, s->heap.start);
-  markCard (s, objptrToPointer (getThreadCurrentObjptr(s), s->heap.start));
+  getThreadCurrent(s)->stack = pointerToObjptr ((pointer)stack, s->heap->start);
+  markCard (s, objptrToPointer (getThreadCurrentObjptr(s), s->heap->start));
 }
 
 void enterGC (GC_state s) {

Modified: mlton/branches/shared-heap-multicore/runtime/gc/switch-thread.c
===================================================================
--- mlton/branches/shared-heap-multicore/runtime/gc/switch-thread.c	2008-03-03 15:27:16 UTC (rev 6442)
+++ mlton/branches/shared-heap-multicore/runtime/gc/switch-thread.c	2008-03-03 15:29:00 UTC (rev 6443)
@@ -32,16 +32,30 @@
      * check on every thread switch.
      * So, we'll stick with the else branch for now.
      */
-    enter (s);
+    //ENTER1 (s, p);
+    /* XXX copied from enter() */
+    /* used needs to be set because the mutator has changed s->stackTop. */
+    getStackCurrent(s)->used = sizeofGCStateCurrentStackUsed (s);
+    getThreadCurrent(s)->exnStack = s->exnStack;
+    beginAtomic (s);
+
     getThreadCurrent(s)->bytesNeeded = ensureBytesFree;
-    switchToThread (s, pointerToObjptr(p, s->heap.start));
+    switchToThread (s, pointerToObjptr(p, s->heap->start));
     s->atomicState--;
-    switchToSignalHandlerThreadIfNonAtomicAndSignalPending (s);
-    ensureInvariantForMutator (s, FALSE);
+    /* XX spoons don't bother to check the signal handler here since we
+       (probably) aren't bothering to synchronize.  we'll get it on the next
+       failed allocation request. */
+    //switchToSignalHandlerThreadIfNonAtomicAndSignalPending (s);
+    ensureHasHeapBytesFreeAndOrInvariantForMutator (s, FALSE,
+                                                    TRUE, TRUE,
+                                                    0, 0);
+
+    endAtomic (s);
     assert (invariantForMutatorFrontier(s));
     assert (invariantForMutatorStack(s));
-    leave (s);
+    //LEAVE0 (s);
   } else {
+    assert (false and "unsafe in a multiprocessor context");
     /* BEGIN: enter(s); */
     getStackCurrent(s)->used = sizeofGCStateCurrentStackUsed (s);
     getThreadCurrent(s)->exnStack = s->exnStack;