progress on multithreaded Panda

panda/src/pgraph/config_pgraph.cxx

@@ -188,7 +188,13 @@ ConfigVariableBool auto_break_cycles
  PRC_DESC("Set this true to automatically detect and break reference-count "
           "cycles in the TransformState and RenderState caches.  When this "
           "is false, you must explicitly call TransformState.clear_cache() "
-          "from time to time to prevent gradual memory bloat."));
+          "from time to time to prevent gradual memory bloat.  This has "
+          "no meaning when garbage-collect-states is true."));
+
+ConfigVariableBool garbage_collect_states
+("garbage-collect-states", false,
+ PRC_DESC("This temporary config variable is used for development only.  "
+          "Do not set!"));
 
 ConfigVariableBool transform_cache
 ("transform-cache", true,

panda/src/pgraph/config_pgraph.h

@@ -44,6 +44,7 @@ extern ConfigVariableBool compose_componentwise;
 extern ConfigVariableBool uniquify_matrix;
 extern ConfigVariableBool paranoid_const;
 extern ConfigVariableBool auto_break_cycles;
+extern ConfigVariableBool garbage_collect_states;
 extern ConfigVariableBool transform_cache;
 extern ConfigVariableBool state_cache;
 extern ConfigVariableBool uniquify_transforms;

panda/src/pgraph/renderAttrib.cxx

@@ -144,6 +144,11 @@ cull_callback(CullTraverser *, const CullTraverserData &) const {
 ////////////////////////////////////////////////////////////////////
 bool RenderAttrib::
 unref() const {
+  // This is flawed, but this is development only.
+  if (garbage_collect_states) {
+    return ReferenceCount::unref();
+  }
+
   // We always have to grab the lock, since we will definitely need to
   // be holding it if we happen to drop the reference count to 0.
   LightReMutexHolder holder(*_attribs_lock);

panda/src/pgraph/renderState.cxx

@@ -419,11 +419,9 @@ compose(const RenderState *other) const {
     return this;
   }
 
-#ifndef NDEBUG
   if (!state_cache) {
     return do_compose(other);
   }
-#endif  // NDEBUG
 
   LightReMutexHolder holder(*_states_lock);
 
@@ -516,11 +514,9 @@ invert_compose(const RenderState *other) const {
     return make_empty();
   }
 
-#ifndef NDEBUG
   if (!state_cache) {
     return do_invert_compose(other);
   }
-#endif  // NDEBUG
 
   LightReMutexHolder holder(*_states_lock);
 
@@ -706,6 +702,11 @@ adjust_all_priorities(int adjustment) const {
 ////////////////////////////////////////////////////////////////////
 bool RenderState::
 unref() const {
+  // This is flawed, but this is development only.
+  if (garbage_collect_states) {
+    return ReferenceCount::unref();
+  }
+
   // We always have to grab the lock, since we will definitely need to
   // be holding it if we happen to drop the reference count to 0.
   LightReMutexHolder holder(*_states_lock);
@@ -1460,11 +1461,9 @@ CPT(RenderState) RenderState::
 return_unique(RenderState *state) {
   nassertr(state != (RenderState *)NULL, state);
 
-#ifndef NDEBUG
   if (!state_cache) {
     return state;
   }
-#endif
 
 #ifndef NDEBUG
   if (paranoid_const) {

panda/src/pgraph/transformState.cxx

@@ -579,10 +579,6 @@ set_shear2d(float shear) const {
 ////////////////////////////////////////////////////////////////////
 CPT(TransformState) TransformState::
 compose(const TransformState *other) const {
-  // This method isn't strictly const, because it updates the cache,
-  // but we pretend that it is because it's only a cache which is
-  // transparent to the rest of the interface.
-
   // We handle identity as a trivial special case.
   if (is_identity()) {
     return other;
@@ -599,72 +595,37 @@ compose(const TransformState *other) const {
     return other;
   }
 
-#ifndef NDEBUG
   if (!transform_cache) {
     return do_compose(other);
   }
-#endif  // NDEBUG
-
-  LightReMutexHolder holder(*_states_lock);
 
   // Is this composition already cached?
-  int index = _composition_cache.find(other);
-  if (index != -1) {
-    Composition &comp = ((TransformState *)this)->_composition_cache.modify_data(index);
-    if (comp._result == (const TransformState *)NULL) {
-      // Well, it wasn't cached already, but we already had an entry
-      // (probably created for the reverse direction), so use the same
-      // entry to store the new result.
-      CPT(TransformState) result = do_compose(other);
-      comp._result = result;
-
-      if (result != (const TransformState *)this) {
-        // See the comments below about the need to up the reference
-        // count only when the result is not the same as this.
-        result->cache_ref();
-      }
+  CPT(TransformState) result;
+  {
+    LightReMutexHolder holder(*_states_lock);
+    int index = _composition_cache.find(other);
+    if (index != -1) {
+      const Composition &comp = _composition_cache.get_data(index);
+      result = comp._result;
+    }
+    if (result != (TransformState *)NULL) {
+      _cache_stats.inc_hits();
     }
-    // Here's the cache!
-    _cache_stats.inc_hits();
-    return comp._result;
-  }
-  _cache_stats.inc_misses();
-
-  // We need to make a new cache entry, both in this object and in the
-  // other object.  We make both records so the other TransformState
-  // object will know to delete the entry from this object when it
-  // destructs, and vice-versa.
-
-  // The cache entry in this object is the only one that indicates the
-  // result; the other will be NULL for now.
-  CPT(TransformState) result = do_compose(other);
-
-  _cache_stats.add_total_size(1);
-  _cache_stats.inc_adds(_composition_cache.get_size() == 0);
-
-  ((TransformState *)this)->_composition_cache[other]._result = result;
-
-  if (other != this) {
-    _cache_stats.add_total_size(1);
-    _cache_stats.inc_adds(other->_composition_cache.get_size() == 0);
-    ((TransformState *)other)->_composition_cache[this]._result = NULL;
   }
 
-  if (result != (const TransformState *)this) {
-    // If the result of compose() is something other than this,
-    // explicitly increment the reference count.  We have to be sure
-    // to decrement it again later, when the composition entry is
-    // removed from the cache.
-    result->cache_ref();
-    
-    // (If the result was just this again, we still store the
-    // result, but we don't increment the reference count, since
-    // that would be a self-referential leak.)
+  if (result != (TransformState *)NULL) {
+    // Success!
+    return result;
   }
 
-  _cache_stats.maybe_report("TransformState");
+  // Not in the cache.  Compute a new result.  It's important that we
+  // don't hold the lock while we do this, or we lose the benefit of
+  // parallelization.
+  result = do_compose(other);
 
-  return result;
+  // It's OK to cast away the constness of this pointer, because the
+  // cache is a transparent property of the class.
+  return ((TransformState *)this)->store_compose(other, result);
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -704,69 +665,38 @@ invert_compose(const TransformState *other) const {
     return make_identity();
   }
 
-#ifndef NDEBUG
   if (!transform_cache) {
     return do_invert_compose(other);
   }
-#endif  // NDEBUG
 
   LightReMutexHolder holder(*_states_lock);
 
-  // Is this composition already cached?
-  int index = _invert_composition_cache.find(other);
-  if (index != -1) {
-    Composition &comp = ((TransformState *)this)->_invert_composition_cache.modify_data(index);
-    if (comp._result == (const TransformState *)NULL) {
-      // Well, it wasn't cached already, but we already had an entry
-      // (probably created for the reverse direction), so use the same
-      // entry to store the new result.
-      CPT(TransformState) result = do_invert_compose(other);
-      comp._result = result;
-
-      if (result != (const TransformState *)this) {
-        // See the comments below about the need to up the reference
-        // count only when the result is not the same as this.
-        result->cache_ref();
-      }
+  CPT(TransformState) result;
+  {
+    LightReMutexHolder holder(*_states_lock);
+    int index = _invert_composition_cache.find(other);
+    if (index != -1) {
+      const Composition &comp = _invert_composition_cache.get_data(index);
+      result = comp._result;
+    }
+    if (result != (TransformState *)NULL) {
+      _cache_stats.inc_hits();
     }
-    // Here's the cache!
-    _cache_stats.inc_hits();
-    return comp._result;
   }
-  _cache_stats.inc_misses();
 
-  // We need to make a new cache entry, both in this object and in the
-  // other object.  We make both records so the other TransformState
-  // object will know to delete the entry from this object when it
-  // destructs, and vice-versa.
-
-  // The cache entry in this object is the only one that indicates the
-  // result; the other will be NULL for now.
-  CPT(TransformState) result = do_invert_compose(other);
-
-  _cache_stats.add_total_size(1);
-  _cache_stats.inc_adds(_invert_composition_cache.get_size() == 0);
-  ((TransformState *)this)->_invert_composition_cache[other]._result = result;
-
-  if (other != this) {
-    _cache_stats.add_total_size(1);
-    _cache_stats.inc_adds(other->_invert_composition_cache.get_size() == 0);
-    ((TransformState *)other)->_invert_composition_cache[this]._result = NULL;
+  if (result != (TransformState *)NULL) {
+    // Success!
+    return result;
   }
 
-  if (result != (const TransformState *)this) {
-    // If the result of compose() is something other than this,
-    // explicitly increment the reference count.  We have to be sure
-    // to decrement it again later, when the composition entry is
-    // removed from the cache.
-    result->cache_ref();
-    
-    // (If the result was just this again, we still store the
-    // result, but we don't increment the reference count, since
-    // that would be a self-referential leak.)
-  }
+  // Not in the cache.  Compute a new result.  It's important that we
+  // don't hold the lock while we do this, or we lose the benefit of
+  // parallelization.
+  result = do_invert_compose(other);
 
-  return result;
+  // It's OK to cast away the constness of this pointer, because the
+  // cache is a transparent property of the class.
+  return ((TransformState *)this)->store_invert_compose(other, result);
 }
 
 ////////////////////////////////////////////////////////////////////
@@ -781,6 +711,11 @@ invert_compose(const TransformState *other) const {
 ////////////////////////////////////////////////////////////////////
 bool TransformState::
 unref() const {
+  // This is flawed, but this is development only.
+  if (garbage_collect_states) {
+    return ReferenceCount::unref();
+  }
+
   // We always have to grab the lock, since we will definitely need to
   // be holding it if we happen to drop the reference count to 0.
   LightReMutexHolder holder(*_states_lock);
@@ -1526,11 +1461,9 @@ CPT(TransformState) TransformState::
 return_unique(TransformState *state) {
   nassertr(state != (TransformState *)NULL, state);
 
-#ifndef NDEBUG
   if (!transform_cache) {
     return state;
   }
-#endif
 
 #ifndef NDEBUG
   if (paranoid_const) {
@@ -1647,6 +1580,158 @@ do_compose(const TransformState *other) const {
   }
 }
 
+////////////////////////////////////////////////////////////////////
+//     Function: TransformState::store_compose
+//       Access: Private
+//  Description: Stores the result of a composition in the cache.
+//               Returns the stored result (it may be a different
+//               object than the one passed in, due to another thread
+//               having computed the composition first).
+////////////////////////////////////////////////////////////////////
+CPT(TransformState) TransformState::
+store_compose(const TransformState *other, const TransformState *result) {
+  // Identity should have already been screened.
+  nassertr(!is_identity(), other);
+  nassertr(!other->is_identity(), this);
+
+  // So should have validity.
+  nassertr(!is_invalid(), this);
+  nassertr(!other->is_invalid(), other);
+
+  LightReMutexHolder holder(*_states_lock);
+
+  // Is this composition already cached?
+  int index = _composition_cache.find(other);
+  if (index != -1) {
+    Composition &comp = _composition_cache.modify_data(index);
+    if (comp._result == (const TransformState *)NULL) {
+      // Well, it wasn't cached already, but we already had an entry
+      // (probably created for the reverse direction), so use the same
+      // entry to store the new result.
+      comp._result = result;
+
+      if (result != (const TransformState *)this) {
+        // See the comments below about the need to up the reference
+        // count only when the result is not the same as this.
+        result->cache_ref();
+      }
+    }
+    // Here's the cache!
+    _cache_stats.inc_hits();
+    return comp._result;
+  }
+  _cache_stats.inc_misses();
+
+  // We need to make a new cache entry, both in this object and in the
+  // other object.  We make both records so the other TransformState
+  // object will know to delete the entry from this object when it
+  // destructs, and vice-versa.
+
+  // The cache entry in this object is the only one that indicates the
+  // result; the other will be NULL for now.
+  _cache_stats.add_total_size(1);
+  _cache_stats.inc_adds(_composition_cache.get_size() == 0);
+
+  _composition_cache[other]._result = result;
+
+  if (other != this) {
+    _cache_stats.add_total_size(1);
+    _cache_stats.inc_adds(other->_composition_cache.get_size() == 0);
+    ((TransformState *)other)->_composition_cache[this]._result = NULL;
+  }
+
+  if (result != (TransformState *)this) {
+    // If the result of do_compose() is something other than this,
+    // explicitly increment the reference count.  We have to be sure
+    // to decrement it again later, when the composition entry is
+    // removed from the cache.
+    result->cache_ref();
+    
+    // (If the result was just this again, we still store the
+    // result, but we don't increment the reference count, since
+    // that would be a self-referential leak.)
+  }
+
+  _cache_stats.maybe_report("TransformState");
+
+  return result;
+}
+
+////////////////////////////////////////////////////////////////////
+//     Function: TransformState::store_invert_compose
+//       Access: Private
+//  Description: Stores the result of a composition in the cache.
+//               Returns the stored result (it may be a different
+//               object than the one passed in, due to another thread
+//               having computed the composition first).
+////////////////////////////////////////////////////////////////////
+CPT(TransformState) TransformState::
+store_invert_compose(const TransformState *other, const TransformState *result) {
+  // Identity should have already been screened.
+  nassertr(!is_identity(), other);
+
+  // So should have validity.
+  nassertr(!is_invalid(), this);
+  nassertr(!other->is_invalid(), other);
+
+  nassertr(other != this, make_identity());
+
+  LightReMutexHolder holder(*_states_lock);
+
+  // Is this composition already cached?
+  int index = _invert_composition_cache.find(other);
+  if (index != -1) {
+    Composition &comp = ((TransformState *)this)->_invert_composition_cache.modify_data(index);
+    if (comp._result == (const TransformState *)NULL) {
+      // Well, it wasn't cached already, but we already had an entry
+      // (probably created for the reverse direction), so use the same
+      // entry to store the new result.
+      comp._result = result;
+
+      if (result != (const TransformState *)this) {
+        // See the comments below about the need to up the reference
+        // count only when the result is not the same as this.
+        result->cache_ref();
+      }
+    }
+    // Here's the cache!
+    _cache_stats.inc_hits();
+    return comp._result;
+  }
+  _cache_stats.inc_misses();
+
+  // We need to make a new cache entry, both in this object and in the
+  // other object.  We make both records so the other TransformState
+  // object will know to delete the entry from this object when it
+  // destructs, and vice-versa.
+
+  // The cache entry in this object is the only one that indicates the
+  // result; the other will be NULL for now.
+  _cache_stats.add_total_size(1);
+  _cache_stats.inc_adds(_invert_composition_cache.get_size() == 0);
+  _invert_composition_cache[other]._result = result;
+
+  if (other != this) {
+    _cache_stats.add_total_size(1);
+    _cache_stats.inc_adds(other->_invert_composition_cache.get_size() == 0);
+    ((TransformState *)other)->_invert_composition_cache[this]._result = NULL;
+  }
+
+  if (result != (TransformState *)this) {
+    // If the result of compose() is something other than this,
+    // explicitly increment the reference count.  We have to be sure
+    // to decrement it again later, when the composition entry is
+    // removed from the cache.
+    result->cache_ref();
+    
+    // (If the result was just this again, we still store the
+    // result, but we don't increment the reference count, since
+    // that would be a self-referential leak.)
+  }
+
+  return result;
+}
+
 ////////////////////////////////////////////////////////////////////
 //     Function: TransformState::do_invert_compose
 //       Access: Private

panda/src/pgraph/transformState.h

@@ -234,7 +234,9 @@ private:
   static CPT(TransformState) return_unique(TransformState *state);
 
   CPT(TransformState) do_compose(const TransformState *other) const;
+  CPT(TransformState) store_compose(const TransformState *other, const TransformState *result);
   CPT(TransformState) do_invert_compose(const TransformState *other) const;
+  CPT(TransformState) store_invert_compose(const TransformState *other, const TransformState *result);
   static bool r_detect_cycles(const TransformState *start_state,
                               const TransformState *current_state,
                               int length, UpdateSeq this_seq,

panda/src/testbed/pgrid.cxx

@@ -213,16 +213,20 @@ load_gridded_models(WindowFramework *window,
   // Load up all the files indicated in the list of gridded filenames
   // and store them in the given vector.
 
+  Loader loader;
+  LoaderOptions options;
+  options.set_flags(options.get_flags() | LoaderOptions::LF_no_ram_cache);
+
   // First, load up each model from disk once, and store them all
   // separate from the scene graph.  Also count up the total number of
   // models we'll be putting in the grid.
   int grid_count = 0;
-  NodePath models("models");
   GriddedFilenames::iterator fi;
   for (fi = filenames.begin(); fi != filenames.end(); ++fi) {
     GriddedFilename &gf = (*fi);
-    gf._model = window->load_model(models, gf._filename);
-    if (!gf._model.is_empty()) {
+    PT(PandaNode) node = loader.load_sync(gf._filename, options);
+    if (node != (PandaNode *)NULL) {
+      gf._model = NodePath(node);
       grid_count += gf._count;
     }
   }
@@ -254,7 +258,7 @@ load_gridded_models(WindowFramework *window,
   int passnum = 0;
   bool loaded_any;
 
-  NodePath render = window->get_render();
+  NodePath root = window->get_panda_framework()->get_models();
   do {
     loaded_any = false;
 
@@ -265,9 +269,15 @@ load_gridded_models(WindowFramework *window,
         // Copy this model into the scene graph, and assign it a
         // position on the grid.
 
-        string model_name = format_string(++model_count);
-        NodePath model = render.attach_new_node(model_name);
-        gf._model.copy_to(model);
+        ++model_count;
+        PT(PandaNode) node = loader.load_sync(gf._filename, options);
+        NodePath model;
+        if (node == (PandaNode *)NULL) {
+          model = gf._model.copy_to(NodePath());
+        } else {
+          model = NodePath(node);
+        }
+        model.reparent_to(root);
 
         gridded_file_info info;
         info.node = model.node();
@@ -369,14 +379,6 @@ load_gridded_models(WindowFramework *window,
 
     passnum++;
   } while (loaded_any);
-
-  // Finally, remove the source models we loaded up.  Not a real big deal.
-  for (fi = filenames.begin(); fi != filenames.end(); ++fi) {
-    GriddedFilename &gf = (*fi);
-    if (!gf._model.is_empty()) {
-      gf._model.remove_node();
-    }
-  }
 }
 
 int
@@ -402,13 +404,15 @@ main(int argc, char *argv[]) {
 
     window->enable_keyboard();
     window->setup_trackball();
-    window->load_models(window->get_render(), static_filenames);
+    window->load_models(framework.get_models(), static_filenames);
+    framework.get_models().instance_to(window->get_render());
 
     GriddedInfoArray info_arr;
     load_gridded_models(window, gridded_filenames, info_arr);
 
     window->loop_animations();
     window->stagger_animations();
+    window->center_trackball(framework.get_models());
 
     framework.enable_default_keys();