From 09f8634433a782040a6fea75fc8ac52a39052e5b Mon Sep 17 00:00:00 2001
From: rdb <git@rdb.name>
Date: Mon, 21 Nov 2022 18:29:51 +0100
Subject: [PATCH] pipeline: Minor optimizations in cycler copy constructor

When copying a dirty cycler, don't use two separate lock-grabbing calls to add to the clean set, then remove from clean set and move to dirty set

Short-cut the cdata copy loop for the common case of only 1 stage
---
 panda/src/pipeline/pipeline.cxx               | 36 +++++++++++++++++++
 panda/src/pipeline/pipeline.h                 |  1 +
 panda/src/pipeline/pipelineCyclerTrueImpl.cxx | 30 ++++++++--------
 3 files changed, 53 insertions(+), 14 deletions(-)

diff --git a/panda/src/pipeline/pipeline.cxx b/panda/src/pipeline/pipeline.cxx
index a3633226bc..ae2d88b512 100644
--- a/panda/src/pipeline/pipeline.cxx
+++ b/panda/src/pipeline/pipeline.cxx
@@ -357,6 +357,42 @@ add_cycler(PipelineCyclerTrueImpl *cycler) {
 }
 #endif  // THREADED_PIPELINE
 
+#ifdef THREADED_PIPELINE
+/**
+ * Adds the indicated cycler to the list of cyclers associated with the
+ * pipeline.  This method only exists when true pipelining is configured on.
+ *
+ * If the dirty flag is true, it will be marked as dirty in addition, as though
+ * add_dirty_cycler() were called immediately afterward.
+ */
+void Pipeline::
+add_cycler(PipelineCyclerTrueImpl *cycler, bool dirty) {
+  // It's safe to add it to the list while cycling, since the _clean list is
+  // not touched during the cycle loop.
+  MutexHolder holder(_lock);
+  nassertv(!cycler->_dirty);
+
+  if (!dirty) {
+    cycler->insert_before(&_clean);
+  }
+  else {
+    nassertv(_num_stages != 1);
+    cycler->insert_before(&_dirty);
+    cycler->_dirty = _next_cycle_seq;
+    ++_num_dirty_cyclers;
+
+#ifdef DEBUG_THREADS
+    inc_cycler_type(_dirty_cycler_types, cycler->get_parent_type(), 1);
+#endif
+  }
+  ++_num_cyclers;
+
+#ifdef DEBUG_THREADS
+  inc_cycler_type(_all_cycler_types, cycler->get_parent_type(), 1);
+#endif
+}
+#endif  // THREADED_PIPELINE
+
 #ifdef THREADED_PIPELINE
 /**
  * Marks the indicated cycler as "dirty", meaning it will need to be cycled
diff --git a/panda/src/pipeline/pipeline.h b/panda/src/pipeline/pipeline.h
index c06bb2ffef..10fec52b60 100644
--- a/panda/src/pipeline/pipeline.h
+++ b/panda/src/pipeline/pipeline.h
@@ -50,6 +50,7 @@ public:
 
 #ifdef THREADED_PIPELINE
   void add_cycler(PipelineCyclerTrueImpl *cycler);
+  void add_cycler(PipelineCyclerTrueImpl *cycler, bool dirty);
   void add_dirty_cycler(PipelineCyclerTrueImpl *cycler);
   void remove_cycler(PipelineCyclerTrueImpl *cycler);
 
diff --git a/panda/src/pipeline/pipelineCyclerTrueImpl.cxx b/panda/src/pipeline/pipelineCyclerTrueImpl.cxx
index 714e61c0e4..3378b479e2 100644
--- a/panda/src/pipeline/pipelineCyclerTrueImpl.cxx
+++ b/panda/src/pipeline/pipelineCyclerTrueImpl.cxx
@@ -56,24 +56,26 @@ PipelineCyclerTrueImpl(const PipelineCyclerTrueImpl &copy) :
   nassertv(_num_stages == copy._num_stages);
   _data = new CycleDataNode[_num_stages];
 
-  // It's no longer critically important that we preserve pointerwise
-  // equivalence between different stages in the copy, but it doesn't cost
-  // much and might be a little more efficient, so we do it anyway.
-  typedef pmap<CycleData *, PT(CycleData) > Pointers;
-  Pointers pointers;
+  if (_num_stages == 1) {
+    _data[0]._cdata = copy._data[0]._cdata->make_copy();
+  }
+  else {
+    // It's no longer critically important that we preserve pointerwise
+    // equivalence between different stages in the copy, but it doesn't cost
+    // much and might be a little more efficient, so we do it anyway.
+    typedef pmap<CycleData *, PT(CycleData) > Pointers;
+    Pointers pointers;
 
-  for (int i = 0; i < _num_stages; ++i) {
-    PT(CycleData) &new_pt = pointers[copy._data[i]._cdata];
-    if (new_pt == nullptr) {
-      new_pt = copy._data[i]._cdata->make_copy();
+    for (int i = 0; i < _num_stages; ++i) {
+      PT(CycleData) &new_pt = pointers[copy._data[i]._cdata];
+      if (new_pt == nullptr) {
+        new_pt = copy._data[i]._cdata->make_copy();
+      }
+      _data[i]._cdata = new_pt.p();
     }
-    _data[i]._cdata = new_pt.p();
   }
 
-  _pipeline->add_cycler(this);
-  if (copy._dirty) {
-    _pipeline->add_dirty_cycler(this);
-  }
+  _pipeline->add_cycler(this, copy._dirty != 0);
 }
 
 /**