Browse Source

Fix GC scan roots, max thread stack size, 64bit gmTable, 64bit byte code gen patched branches

Greg 14 years ago
parent
commit
1c1dff5322

+ 6 - 0
gmsrc/doc/ChangeLog.txt

@@ -379,3 +379,9 @@ o Changed #define in MSVC platform file to _M_X64 which is correct 64bit build m
 
 29/06/11
 o Add 64bit MSVC build configurations for GME
+
+2/10/11
+o Fixed GC root scan.
+o Increased GMTHREAD_MAXBYTESIZE to allow samples to run in 64bit
+o Fixed gmTable 64bit gmVariable equality test.
+o Fixed byte code generator. 64bit was mixing 32 and 64 bit patched addresses.

+ 3 - 3
gmsrc/src/gm/gmCodeGen.cpp

@@ -667,7 +667,7 @@ bool gmCodeGenPrivate::GenStmtBreak(const gmCodeTreeNode * a_node, gmByteCodeGen
   {
     a_byteCode->Emit(BC_BRA);
     Patch * patch = &m_patches.InsertLast();
-    patch->m_address = a_byteCode->Skip(sizeof(gmuint32));
+    patch->m_address = a_byteCode->Skip(sizeof(gmptr)); // NOTE: Using gmptr size addresses
     patch->m_next = m_loopStack[m_currentLoop].m_breaks;
     m_loopStack[m_currentLoop].m_breaks = m_patches.Count()-1;
     return true;
@@ -687,7 +687,7 @@ bool gmCodeGenPrivate::GenStmtContinue(const gmCodeTreeNode * a_node, gmByteCode
   {
     a_byteCode->Emit(BC_BRA);
     Patch * patch = &m_patches.InsertLast();
-    patch->m_address = a_byteCode->Skip(sizeof(gmuint32));
+    patch->m_address = a_byteCode->Skip(sizeof(gmptr)); // NOTE: Using gmptr size addresses
     patch->m_next = m_loopStack[m_currentLoop].m_continues;
     m_loopStack[m_currentLoop].m_continues = m_patches.Count()-1;
     return true;
@@ -1596,7 +1596,7 @@ void gmCodeGenPrivate::ApplyPatches(int a_patches, gmByteCodeGen * a_byteCode, g
     Patch * curPatch = &m_patches[a_patches];
 
     a_byteCode->Seek(curPatch->m_address);
-    *a_byteCode << a_value;
+    *a_byteCode << (gmptr)a_value; // NOTE: we are using gmptr size addresses/offsets
     a_patches = curPatch->m_next;
   }
   a_byteCode->Seek(pos);

+ 1 - 1
gmsrc/src/gm/gmConfig.h

@@ -54,7 +54,7 @@ enum gmEndian
 // RUNTIME THREAD
 
 #define GMTHREAD_INITIALBYTESIZE    512       // initial stack byte size for a single thread
-#define GMTHREAD_MAXBYTESIZE        128000    //1024  // max stack byte size for a single thread (Sample scripts like it big)
+#define GMTHREAD_MAXBYTESIZE        (150*1024) //1024  // max stack byte size for a single thread (Sample scripts like it big)
 
 // MACHINE
 

+ 79 - 1
gmsrc/src/gm/gmIncGC.cpp

@@ -232,7 +232,69 @@ void gmGCColorSet::GrayThisObject(gmGCObjBase* a_obj)
 #endif //GM_GC_DEBUG
 }
 
+
+void gmGCColorSet::GrayThisRootObject(gmGCObjBase* a_obj)
+{
+  // Normally, a object will never go from black to gray, only black to white or white to gray/black.
+  // We need to enforce that roots are treated as gray, and in this language and GC implementation
+  // it is convenient to force roots to gray from whatever logical state they are in.
+  // This is due to GC design choices including: atomic root snapshot, write barrier and allocate black.
+
+  gmGCObjBase* objPrev = a_obj->GetPrev();
+  gmGCObjBase* objNext = a_obj->GetNext();
+
+#if GM_GC_DEBUG
+  GM_ASSERT(a_obj->m_curPosColor != GM_GC_DEBUG_COL_FREE);
+  GM_ASSERT(a_obj->m_curPosColor != GM_GC_DEBUG_COL_INVALID);
+  a_obj->m_curPosColor = GM_GC_DEBUG_COL_GRAY;
+#endif //GM_GC_DEBUG
+
+  // Set object`s color to shaded first.
+  a_obj->SetColor(m_gc->GetCurShadeColor());
+
+  // The object must be shaded
+  GM_ASSERT(m_gc->IsShaded(a_obj));
+
+  // Splice the object out of the list
+  objPrev->SetNext(objNext);
+  objNext->SetPrev(objPrev);
+
+  // Fix sentinels
+  if( m_scan == a_obj )
+  {
+    m_scan = m_scan->GetNext(); // Not Prev as scan should be the start of black inclusive
+  }
+  if( m_free == a_obj )
+  {
+    m_free = m_free->GetNext();
+  }
   
+  // We are scanning roots, not tracing, check that this is so
+  GM_ASSERT( m_gc->GetTraceState().m_done );
+
+  // Put the object into the correct place in the gray list
+
+#if DEPTH_FIRST
+  // Put the gray object at the head of the gray list.
+  a_obj->SetPrev(m_scan->GetPrev());
+  a_obj->SetNext(m_scan);
+  m_scan->GetPrev()->SetNext(a_obj);
+  m_scan->SetPrev(a_obj);
+#else // BREADTH_FIRST
+  // Put the gray object at the tail of the gray list.
+  a_obj->SetPrev(m_gray);
+  a_obj->SetNext(m_gray->GetNext());
+  m_gray->GetNext()->SetPrev(a_obj);
+  m_gray->SetNext(a_obj);
+#endif // BREADTH_FIRST
+
+#if GM_GC_DEBUG
+  // Slow, paranoid check
+  VerifyIntegrity();
+#endif //GM_GC_DEBUG
+}
+
+
 void gmGCColorSet::Revive(gmGCObjBase* a_obj)
 {
   // NOTE: Once objects are in the free list, we can't trust the color mark, 
@@ -291,6 +353,12 @@ void gmGCColorSet::ReclaimGarbage()
 {
   GM_ASSERT(m_scan->GetPrev() == m_gray);
 
+#if GM_GC_DEBUG
+  // Slow, paranoid check
+  VerifyIntegrity();
+#endif //GM_GC_DEBUG
+
+
 #if GM_GC_DEBUG
   {
     // Traverse the newly found garbage objects just to make sure there
@@ -373,6 +441,11 @@ void gmGCColorSet::ReclaimGarbage()
 
   GM_ASSERT(m_gray->GetNext() == m_scan);
 
+#if GM_GC_DEBUG
+  // Slow, paranoid check
+  VerifyIntegrity();
+#endif //GM_GC_DEBUG
+
 #if GM_GC_DEBUG
   {
     int count = 0;
@@ -439,6 +512,12 @@ int gmGCColorSet::DestructSomeFreeObjects(int a_maxToDestruct)
       m_scan = m_free;
     }
   }
+
+#if GM_GC_DEBUG
+  // Slow, paranoid check
+  VerifyIntegrity();
+#endif //GM_GC_DEBUG
+
   return numDestructed;
 }
 
@@ -736,7 +815,6 @@ void gmGarbageCollector::FullCollect()
 }
 
 
-
 //////////////////////////////////////////////////
 // Helper functions for VM and debugger
 //////////////////////////////////////////////////

+ 6 - 0
gmsrc/src/gm/gmIncGC.h

@@ -161,6 +161,9 @@ public:
 
   /// \brief Gray this object.
   void GrayThisObject(gmGCObjBase* a_obj);
+
+  /// \brief Gray this root object (Called to force roots to gray, which they logically always are)
+  void GrayThisRootObject(gmGCObjBase* a_obj);
   
   /// \brief Called on a new object being allocated.
   void Allocate(gmGCObjBase* a_obj);
@@ -287,6 +290,9 @@ public:
   /// \brief Do a full collect and don't return until done.
   void FullCollect();
 
+  /// \brief Called during trace by machine ONLY on ROOT objects (eg. stacks)
+  void GetNextRootObject(gmGCObjBase* a_obj)      {m_colorSet.GrayThisRootObject(a_obj);}
+
   /// \brief Called during trace by client code, and by scan roots callback.  
   /// This grays a white object.
   inline void GetNextObject(gmGCObjBase* a_obj)   {m_colorSet.GrayAWhite(a_obj);}

+ 4 - 4
gmsrc/src/gm/gmMachine.cpp

@@ -157,7 +157,7 @@ void GM_CDECL gmMachine::ScanRootsCallBack(gmMachine* a_machine, gmGarbageCollec
   for(cgmoIt = a_machine->m_cppOwnedGMObjs.First(); cgmoIt; ++cgmoIt)
   {
     gmObject* curObj = cgmoIt->m_obj;
-    a_gc->GetNextObject(curObj);
+    a_gc->GetNextRootObject(curObj);
   }
 
   // iterate over all threads and mark the stacks.
@@ -169,20 +169,20 @@ void GM_CDECL gmMachine::ScanRootsCallBack(gmMachine* a_machine, gmGarbageCollec
   // iterate over global variables and mark
   if(a_machine->m_global)
   {
-    a_gc->GetNextObject(a_machine->m_global);
+    a_gc->GetNextRootObject(a_machine->m_global);
   }
   // iterate over type variables and mark
   gmuint i;
   for(i = 0; i < a_machine->m_types.Count(); ++i)
   {
-    a_gc->GetNextObject(a_machine->m_types[i].m_variables);
+    a_gc->GetNextRootObject(a_machine->m_types[i].m_variables);
   }
 
 #if !GM_GC_KEEP_PERSISTANT_SEPARATE
   //NOTE This needs to be spread over time perhaps.
   for(i=0; i<(gmuint)a_machine->m_numPermanantStrings; ++i)
   {
-    a_gc->GetNextObject(a_machine->m_permanantStrings[i]);
+    a_gc->GetNextRootObject(a_machine->m_permanantStrings[i]);
   }
 #endif //!GM_GC_KEEP_PERSISTANT_SEPARATE
 }

+ 48 - 4
gmsrc/src/gm/gmTableObject.cpp

@@ -102,6 +102,52 @@ void gmTableObject::Destruct(gmMachine * a_machine)
 }
 
 
+// Helper to minimally compare if two variables are equal, for finding in table
+// This function is a candidate for platform specific optimization
+inline int VariablesEqual(const gmVariable& a_varA, const gmVariable& a_varB)
+{
+  // We can't assume unused bits in m_ref are zero.
+  // Some GM variants can't assume sizeof(m_ref) is the largest union component.
+
+#if GMMACHINE_NULL_VAR_CTOR
+  if( (a_varA.m_ref == a_varB.m_ref) &&
+      (a_varA.m_type == a_varB.m_type) )
+  {
+    return true;
+  }
+#else //GMMACHINE_NULL_VAR_CTOR
+  if( a_varA.m_type == a_varB.m_type )
+  {
+    switch( a_varA.m_type )
+    {
+      case GM_INT: 
+      {
+        if( a_varA.m_value.m_int == a_varB.m_value.m_int )
+        {
+          return true;
+        }
+        break;
+      }
+      case GM_FLOAT:
+      {
+        //if( a_varA.m_value.m_float == a_varB.m_value.m_float ) return true;
+        return ( memcmp(&a_varA.m_value.m_ref, &a_varB.m_value.m_ref, sizeof(gmfloat)) == 0 ); // Don't use FPU for comparison
+        break;
+      }
+      default: // All other types are reference, NULLs are not stored.
+      {
+        if( a_varA.m_value.m_ref == a_varB.m_value.m_ref )
+        {
+          return true;
+        }
+        break;
+      }
+    }
+  }
+#endif  
+  return false;
+}
+
 
 gmVariable gmTableObject::Get(const gmVariable &a_key) const
 {
@@ -113,8 +159,7 @@ gmVariable gmTableObject::Get(const gmVariable &a_key) const
 
     do
     {
-      if(a_key.m_value.m_ref == foundNode->m_key.m_value.m_ref &&
-         a_key.m_type == foundNode->m_key.m_type)
+      if( VariablesEqual(a_key, foundNode->m_key) )
       {
         return foundNode->m_value;
       }
@@ -159,8 +204,7 @@ void gmTableObject::Set(gmMachine * a_machine, const gmVariable &a_key, const gm
   // find key, if it exists
   do
   {
-    if( (a_key.m_value.m_ref == foundNode->m_key.m_value.m_ref) &&
-        (a_key.m_type == foundNode->m_key.m_type))
+    if( VariablesEqual(a_key, foundNode->m_key) )
     {
       //If found and value is null, remove it
       if(GM_NULL == a_value.m_type)

+ 2 - 0
gmsrc/src/gm/gmTableObject.h

@@ -116,6 +116,8 @@ private:
   void Construct(gmMachine * a_machine);
  
   void RemoveAndDeleteAll(gmMachine * a_machine);
+  
+  // This is a candidate for optimization. May want to specialize hash for variable types and platform.
   inline gmTableNode * GetAtHashPos(const gmVariable* a_key) const
   {
     unsigned int hash = (unsigned int)a_key->m_value.m_ref; // Use lower 32 bits for now (NOTE: Alternate hashing method may improve performance)

+ 8 - 4
gmsrc/src/gm/gmThread.cpp

@@ -102,7 +102,7 @@ void gmThread::GCScanRoots(gmMachine* a_machine, gmGarbageCollector* a_gc)
     if(m_stack[i].IsReference())
     {
       gmObject * object = GM_MOBJECT(m_machine, m_stack[i].m_value.m_ref);
-      a_gc->GetNextObject(object);
+      a_gc->GetNextRootObject(object);
     }
   }
 
@@ -113,7 +113,7 @@ void gmThread::GCScanRoots(gmMachine* a_machine, gmGarbageCollector* a_gc)
     if(signal->m_signal.IsReference())
     {
       gmObject * object = GM_MOBJECT(m_machine, signal->m_signal.m_value.m_ref);
-      a_gc->GetNextObject(object);
+      a_gc->GetNextRootObject(object);
     }
     signal = signal->m_nextSignal;
   }
@@ -125,7 +125,7 @@ void gmThread::GCScanRoots(gmMachine* a_machine, gmGarbageCollector* a_gc)
     if(block->m_block.IsReference())
     {
       gmObject * object = GM_MOBJECT(m_machine, block->m_block.m_value.m_ref);
-      a_gc->GetNextObject(object);
+      a_gc->GetNextRootObject(object);
     }
     block = block->m_nextBlock;
   }
@@ -1237,7 +1237,11 @@ bool gmThread::Touch(int a_extra)
   bool reAlloc = false; 
   while((m_top + a_extra + GMTHREAD_SLACKSPACE) >= m_size) 
   { 
-    if(sizeof(gmVariable) * m_size > GMTHREAD_MAXBYTESIZE) return false; 
+    if(sizeof(gmVariable) * m_size > GMTHREAD_MAXBYTESIZE)
+    {
+      GM_ASSERT(!"GMTHREAD_MAXBYTESIZE exceeded");
+      return false; 
+    }
     m_size *= 2; 
     reAlloc = true; 
   }