Compositor: Keep WorkPackages and Data Around.

WorkPackages struct was created when scheduled. This patch keeps the
WorkPackages around and stores additional data with the workpackages.

The speedup is to small to notice, but it is needed as preparation
to introduce a faster scheduling method.

8 files changed, 71 insertions, 96 deletions

diff --git a/source/blender/compositor/intern/COM_CPUDevice.cc b/source/blender/compositor/intern/COM_CPUDevice.cc
index e3081ca7bf4..29a82bec636 100644
--- a/source/blender/compositor/intern/COM_CPUDevice.cc
+++ b/source/blender/compositor/intern/COM_CPUDevice.cc
@@ -18,22 +18,22 @@
 
 #include "COM_CPUDevice.h"
 
+#include "COM_ExecutionGroup.h"
+
+#include "BLI_rect.h"
+
 namespace blender::compositor {
 
 CPUDevice::CPUDevice(int thread_id) : m_thread_id(thread_id)
 {
 }
 
-void CPUDevice::execute(WorkPackage *work)
+void CPUDevice::execute(WorkPackage *work_package)
 {
-  const unsigned int chunkNumber = work->chunk_number;
-  ExecutionGroup *executionGroup = work->execution_group;
-  rcti rect;
-
-  executionGroup->determineChunkRect(&rect, chunkNumber);
-
-  executionGroup->getOutputOperation()->executeRegion(&rect, chunkNumber);
+  const unsigned int chunkNumber = work_package->chunk_number;
+  ExecutionGroup *executionGroup = work_package->execution_group;
 
+  executionGroup->getOutputOperation()->executeRegion(&work_package->rect, chunkNumber);
   executionGroup->finalizeChunkExecution(chunkNumber, nullptr);
 }
 
diff --git a/source/blender/compositor/intern/COM_ExecutionGroup.cc b/source/blender/compositor/intern/COM_ExecutionGroup.cc
index 87c9e6e8a69..7ba787e2f52 100644
--- a/source/blender/compositor/intern/COM_ExecutionGroup.cc
+++ b/source/blender/compositor/intern/COM_ExecutionGroup.cc
@@ -123,12 +123,19 @@ NodeOperation *ExecutionGroup::getOutputOperation() const
 
 void ExecutionGroup::initExecution()
 {
-  m_chunk_execution_states.clear();
+  m_work_packages.clear();
   determineNumberOfChunks();
 
   if (this->m_chunks_len != 0) {
-    m_chunk_execution_states.resize(this->m_chunks_len);
-    m_chunk_execution_states.fill(eChunkExecutionState::NOT_SCHEDULED);
+    m_work_packages.resize(this->m_chunks_len);
+    for (unsigned int index = 0; index < m_chunks_len; index++) {
+      m_work_packages[index] = {
+          .state = eChunkExecutionState::NotScheduled,
+          .execution_group = this,
+          .chunk_number = index,
+      };
+      determineChunkRect(&m_work_packages[index].rect, index);
+    }
   }
 
   unsigned int max_offset = 0;
@@ -146,7 +153,7 @@ void ExecutionGroup::initExecution()
 
 void ExecutionGroup::deinitExecution()
 {
-  m_chunk_execution_states.clear();
+  m_work_packages.clear();
   this->m_chunks_len = 0;
   this->m_x_chunks_len = 0;
   this->m_y_chunks_len = 0;
@@ -214,11 +221,10 @@ blender::Array<unsigned int> ExecutionGroup::determine_chunk_execution_order() c
       ChunkOrderHotspot hotspot(border_width * centerX, border_height * centerY, 0.0f);
       blender::Array<ChunkOrder> chunk_orders(m_chunks_len);
       for (index = 0; index < this->m_chunks_len; index++) {
-        rcti rect;
-        determineChunkRect(&rect, index);
+        const WorkPackage &work_package = m_work_packages[index];
         chunk_orders[index].index = index;
-        chunk_orders[index].x = rect.xmin - this->m_viewerBorder.xmin;
-        chunk_orders[index].y = rect.ymin - this->m_viewerBorder.ymin;
+        chunk_orders[index].x = work_package.rect.xmin - this->m_viewerBorder.xmin;
+        chunk_orders[index].y = work_package.rect.ymin - this->m_viewerBorder.ymin;
         chunk_orders[index].update_distance(&hotspot, 1);
       }
 
@@ -252,11 +258,10 @@ blender::Array<unsigned int> ExecutionGroup::determine_chunk_execution_order() c
 
       blender::Array<ChunkOrder> chunk_orders(m_chunks_len);
       for (index = 0; index < this->m_chunks_len; index++) {
-        rcti rect;
-        determineChunkRect(&rect, index);
+        const WorkPackage &work_package = m_work_packages[index];
         chunk_orders[index].index = index;
-        chunk_orders[index].x = rect.xmin - this->m_viewerBorder.xmin;
-        chunk_orders[index].y = rect.ymin - this->m_viewerBorder.ymin;
+        chunk_orders[index].x = work_package.rect.xmin - this->m_viewerBorder.xmin;
+        chunk_orders[index].y = work_package.rect.ymin - this->m_viewerBorder.ymin;
         chunk_orders[index].update_distance(hotspots, 9);
       }
 
@@ -320,8 +325,9 @@ void ExecutionGroup::execute(ExecutionSystem *graph)
       chunk_index = chunk_order[index];
       int yChunk = chunk_index / this->m_x_chunks_len;
       int xChunk = chunk_index - (yChunk * this->m_x_chunks_len);
-      switch (m_chunk_execution_states[chunk_index]) {
-        case eChunkExecutionState::NOT_SCHEDULED: {
+      const WorkPackage &work_package = m_work_packages[chunk_index];
+      switch (work_package.state) {
+        case eChunkExecutionState::NotScheduled: {
           scheduleChunkWhenPossible(graph, xChunk, yChunk);
           finished = false;
           startEvaluated = true;
@@ -332,13 +338,13 @@ void ExecutionGroup::execute(ExecutionSystem *graph)
           }
           break;
         }
-        case eChunkExecutionState::SCHEDULED: {
+        case eChunkExecutionState::Scheduled: {
           finished = false;
           startEvaluated = true;
           numberEvaluated++;
           break;
         }
-        case eChunkExecutionState::EXECUTED: {
+        case eChunkExecutionState::Executed: {
           if (!startEvaluated) {
             startIndex = index + 1;
           }
@@ -358,15 +364,14 @@ void ExecutionGroup::execute(ExecutionSystem *graph)
 
 MemoryBuffer **ExecutionGroup::getInputBuffersOpenCL(int chunkNumber)
 {
-  rcti rect;
-  determineChunkRect(&rect, chunkNumber);
+  WorkPackage &work_package = m_work_packages[chunkNumber];
 
   MemoryBuffer **memoryBuffers = (MemoryBuffer **)MEM_callocN(
       sizeof(MemoryBuffer *) * this->m_max_read_buffer_offset, __func__);
   rcti output;
   for (ReadBufferOperation *readOperation : m_read_operations) {
     MemoryProxy *memoryProxy = readOperation->getMemoryProxy();
-    this->determineDependingAreaOfInterest(&rect, readOperation, &output);
+    this->determineDependingAreaOfInterest(&work_package.rect, readOperation, &output);
     MemoryBuffer *memoryBuffer = memoryProxy->getExecutor()->constructConsolidatedMemoryBuffer(
         *memoryProxy, output);
     memoryBuffers[readOperation->getOffset()] = memoryBuffer;
@@ -385,8 +390,9 @@ MemoryBuffer *ExecutionGroup::constructConsolidatedMemoryBuffer(MemoryProxy &mem
 
 void ExecutionGroup::finalizeChunkExecution(int chunkNumber, MemoryBuffer **memoryBuffers)
 {
-  if (this->m_chunk_execution_states[chunkNumber] == eChunkExecutionState::SCHEDULED) {
-    this->m_chunk_execution_states[chunkNumber] = eChunkExecutionState::EXECUTED;
+  WorkPackage &work_package = m_work_packages[chunkNumber];
+  if (work_package.state == eChunkExecutionState::Scheduled) {
+    work_package.state = eChunkExecutionState::Executed;
   }
 
   atomic_add_and_fetch_u(&this->m_chunks_finished, 1);
@@ -418,7 +424,7 @@ void ExecutionGroup::finalizeChunkExecution(int chunkNumber, MemoryBuffer **memo
   }
 }
 
-inline void ExecutionGroup::determineChunkRect(rcti *rect,
+inline void ExecutionGroup::determineChunkRect(rcti *r_rect,
                                                const unsigned int xChunk,
                                                const unsigned int yChunk) const
 {
@@ -427,14 +433,14 @@ inline void ExecutionGroup::determineChunkRect(rcti *rect,
 
   if (this->m_flags.single_threaded) {
     BLI_rcti_init(
-        rect, this->m_viewerBorder.xmin, border_width, this->m_viewerBorder.ymin, border_height);
+        r_rect, this->m_viewerBorder.xmin, border_width, this->m_viewerBorder.ymin, border_height);
   }
   else {
     const unsigned int minx = xChunk * this->m_chunkSize + this->m_viewerBorder.xmin;
     const unsigned int miny = yChunk * this->m_chunkSize + this->m_viewerBorder.ymin;
     const unsigned int width = MIN2((unsigned int)this->m_viewerBorder.xmax, this->m_width);
     const unsigned int height = MIN2((unsigned int)this->m_viewerBorder.ymax, this->m_height);
-    BLI_rcti_init(rect,
+    BLI_rcti_init(r_rect,
                   MIN2(minx, this->m_width),
                   MIN2(minx + this->m_chunkSize, width),
                   MIN2(miny, this->m_height),
@@ -442,11 +448,11 @@ inline void ExecutionGroup::determineChunkRect(rcti *rect,
   }
 }
 
-void ExecutionGroup::determineChunkRect(rcti *rect, const unsigned int chunkNumber) const
+void ExecutionGroup::determineChunkRect(rcti *r_rect, const unsigned int chunkNumber) const
 {
   const unsigned int yChunk = chunkNumber / this->m_x_chunks_len;
   const unsigned int xChunk = chunkNumber - (yChunk * this->m_x_chunks_len);
-  determineChunkRect(rect, xChunk, yChunk);
+  determineChunkRect(r_rect, xChunk, yChunk);
 }
 
 MemoryBuffer *ExecutionGroup::allocateOutputBuffer(rcti &rect)
@@ -498,9 +504,10 @@ bool ExecutionGroup::scheduleAreaWhenPossible(ExecutionSystem *graph, rcti *area
 
 bool ExecutionGroup::scheduleChunk(unsigned int chunkNumber)
 {
-  if (this->m_chunk_execution_states[chunkNumber] == eChunkExecutionState::NOT_SCHEDULED) {
-    this->m_chunk_execution_states[chunkNumber] = eChunkExecutionState::SCHEDULED;
-    WorkScheduler::schedule(this, chunkNumber);
+  WorkPackage &work_package = m_work_packages[chunkNumber];
+  if (work_package.state == eChunkExecutionState::NotScheduled) {
+    work_package.state = eChunkExecutionState::Scheduled;
+    WorkScheduler::schedule(&work_package);
     return true;
   }
   return false;
@@ -519,22 +526,21 @@ bool ExecutionGroup::scheduleChunkWhenPossible(ExecutionSystem *graph,
 
   // Check if chunk is already executed or scheduled and not yet executed.
   const int chunk_index = chunk_y * this->m_x_chunks_len + chunk_x;
-  if (this->m_chunk_execution_states[chunk_index] == eChunkExecutionState::EXECUTED) {
+  WorkPackage &work_package = m_work_packages[chunk_index];
+  if (work_package.state == eChunkExecutionState::Executed) {
     return true;
   }
-  if (this->m_chunk_execution_states[chunk_index] == eChunkExecutionState::SCHEDULED) {
+  if (work_package.state == eChunkExecutionState::Scheduled) {
     return false;
   }
 
-  rcti rect;
-  determineChunkRect(&rect, chunk_x, chunk_y);
   bool can_be_executed = true;
   rcti area;
 
   for (ReadBufferOperation *read_operation : m_read_operations) {
     BLI_rcti_init(&area, 0, 0, 0, 0);
     MemoryProxy *memory_proxy = read_operation->getMemoryProxy();
-    determineDependingAreaOfInterest(&rect, read_operation, &area);
+    determineDependingAreaOfInterest(&work_package.rect, read_operation, &area);
     ExecutionGroup *group = memory_proxy->getExecutor();
 
     if (!group->scheduleAreaWhenPossible(graph, &area)) {
diff --git a/source/blender/compositor/intern/COM_ExecutionGroup.h b/source/blender/compositor/intern/COM_ExecutionGroup.h
index a995da38dec..c345724d4c9 100644
--- a/source/blender/compositor/intern/COM_ExecutionGroup.h
+++ b/source/blender/compositor/intern/COM_ExecutionGroup.h
@@ -31,6 +31,7 @@
 #include "COM_MemoryProxy.h"
 #include "COM_Node.h"
 #include "COM_NodeOperation.h"
+#include "COM_WorkPackage.h"
 #include <vector>
 
 namespace blender::compositor {
@@ -41,25 +42,6 @@ class MemoryBuffer;
 class ReadBufferOperation;
 class Device;
 
-/**
- * \brief the execution state of a chunk in an ExecutionGroup
- * \ingroup Execution
- */
-enum class eChunkExecutionState {
-  /**
-   * \brief chunk is not yet scheduled
-   */
-  NOT_SCHEDULED = 0,
-  /**
-   * \brief chunk is scheduled, but not yet executed
-   */
-  SCHEDULED = 1,
-  /**
-   * \brief chunk is executed.
-   */
-  EXECUTED = 2,
-};
-
 struct ExecutionGroupFlags {
   bool initialized : 1;
   /**
@@ -163,12 +145,9 @@ class ExecutionGroup {
   unsigned int m_chunks_finished;
 
   /**
-   * \brief m_chunk_execution_states holds per chunk the execution state. this state can be
-   *   - eChunkExecutionState::NOT_SCHEDULED: not scheduled
-   *   - eChunkExecutionState::SCHEDULED: scheduled
-   *   - eChunkExecutionState::EXECUTED: executed
+   * \brief m_work_packages holds all unit of work.
    */
-  blender::Vector<eChunkExecutionState> m_chunk_execution_states;
+  blender::Vector<WorkPackage> m_work_packages;
 
   /**
    * \brief denotes boundary for border compositing
@@ -201,7 +180,9 @@ class ExecutionGroup {
    * \note Only gives useful results after the determination of the chunksize
    * \see determineChunkSize()
    */
-  void determineChunkRect(rcti *rect, const unsigned int xChunk, const unsigned int yChunk) const;
+  void determineChunkRect(rcti *r_rect,
+                          const unsigned int xChunk,
+                          const unsigned int yChunk) const;
 
   /**
    * \brief determine the number of chunks, based on the chunkSize, width and height.
@@ -395,7 +376,7 @@ class ExecutionGroup {
    * \note Only gives useful results after the determination of the chunksize
    * \see determineChunkSize()
    */
-  void determineChunkRect(rcti *rect, const unsigned int chunkNumber) const;
+  void determineChunkRect(rcti *r_rect, const unsigned int chunkNumber) const;
 
   void setChunksize(int chunksize)
   {
diff --git a/source/blender/compositor/intern/COM_OpenCLDevice.cc b/source/blender/compositor/intern/COM_OpenCLDevice.cc
index ac6f1490083..b96dbe91434 100644
--- a/source/blender/compositor/intern/COM_OpenCLDevice.cc
+++ b/source/blender/compositor/intern/COM_OpenCLDevice.cc
@@ -57,18 +57,16 @@ OpenCLDevice::~OpenCLDevice()
   }
 }
 
-void OpenCLDevice::execute(WorkPackage *work)
+void OpenCLDevice::execute(WorkPackage *work_package)
 {
-  const unsigned int chunkNumber = work->chunk_number;
-  ExecutionGroup *executionGroup = work->execution_group;
-  rcti rect;
+  const unsigned int chunkNumber = work_package->chunk_number;
+  ExecutionGroup *executionGroup = work_package->execution_group;
 
-  executionGroup->determineChunkRect(&rect, chunkNumber);
   MemoryBuffer **inputBuffers = executionGroup->getInputBuffersOpenCL(chunkNumber);
-  MemoryBuffer *outputBuffer = executionGroup->allocateOutputBuffer(rect);
+  MemoryBuffer *outputBuffer = executionGroup->allocateOutputBuffer(work_package->rect);
 
   executionGroup->getOutputOperation()->executeOpenCLRegion(
-      this, &rect, chunkNumber, inputBuffers, outputBuffer);
+      this, &work_package->rect, chunkNumber, inputBuffers, outputBuffer);
 
   delete outputBuffer;
 
diff --git a/source/blender/compositor/intern/COM_WorkPackage.cc b/source/blender/compositor/intern/COM_WorkPackage.cc
index e7cc1bbf908..c0bc274da8f 100644
--- a/source/blender/compositor/intern/COM_WorkPackage.cc
+++ b/source/blender/compositor/intern/COM_WorkPackage.cc
@@ -20,10 +20,4 @@
 
 namespace blender::compositor {
 
-WorkPackage::WorkPackage(ExecutionGroup *execution_group, unsigned int chunk_number)
-{
-  this->execution_group = execution_group;
-  this->chunk_number = chunk_number;
-}
-
 }  // namespace blender::compositor
diff --git a/source/blender/compositor/intern/COM_WorkPackage.h b/source/blender/compositor/intern/COM_WorkPackage.h
index 5577dcb1eb6..4541a778711 100644
--- a/source/blender/compositor/intern/COM_WorkPackage.h
+++ b/source/blender/compositor/intern/COM_WorkPackage.h
@@ -18,16 +18,21 @@
 
 #pragma once
 
-class ExecutionGroup;
-#include "COM_ExecutionGroup.h"
+#include "COM_defines.h"
+
+#include "BLI_rect.h"
 
 namespace blender::compositor {
+// Forward Declarations.
+class ExecutionGroup;
 
 /**
  * \brief contains data about work that can be scheduled
  * \see WorkScheduler
  */
 struct WorkPackage {
+  eChunkExecutionState state = eChunkExecutionState::NotScheduled;
+
   /**
    * \brief executionGroup with the operations-setup to be evaluated
    */
@@ -39,11 +44,9 @@ struct WorkPackage {
   unsigned int chunk_number;
 
   /**
-   * constructor
-   * \param group: the ExecutionGroup
-   * \param chunk_number: the number of the chunk
+   * Area of the execution group that the work package calculates.
    */
-  WorkPackage(ExecutionGroup *group, unsigned int chunk_number);
+  rcti rect;
 
 #ifdef WITH_CXX_GUARDEDALLOC
   MEM_CXX_CLASS_ALLOC_FUNCS("COM:WorkPackage")
diff --git a/source/blender/compositor/intern/COM_WorkScheduler.cc b/source/blender/compositor/intern/COM_WorkScheduler.cc
index 7224078a2c3..55114e6b72a 100644
--- a/source/blender/compositor/intern/COM_WorkScheduler.cc
+++ b/source/blender/compositor/intern/COM_WorkScheduler.cc
@@ -119,7 +119,6 @@ static void *thread_execute_gpu(void *data)
 
   while ((work = (WorkPackage *)BLI_thread_queue_pop(g_work_scheduler.opencl.queue))) {
     device->execute(work);
-    delete work;
   }
 
   return nullptr;
@@ -306,7 +305,6 @@ static void threading_model_single_thread_execute(WorkPackage *package)
 {
   CPUDevice device(0);
   device.execute(package);
-  delete package;
 }
 
 /* \} */
@@ -322,7 +320,6 @@ static void *threading_model_queue_execute(void *data)
   BLI_thread_local_set(g_thread_device, device);
   while ((work = (WorkPackage *)BLI_thread_queue_pop(g_work_scheduler.queue.queue))) {
     device->execute(work);
-    delete work;
   }
 
   return nullptr;
@@ -433,10 +430,8 @@ static void threading_model_task_stop()
 /** \name Public API
  * \{ */
 
-void WorkScheduler::schedule(ExecutionGroup *group, int chunkNumber)
+void WorkScheduler::schedule(WorkPackage *package)
 {
-  WorkPackage *package = new WorkPackage(group, chunkNumber);
-
   if (COM_is_opencl_enabled()) {
     if (opencl_schedule(package)) {
       return;
diff --git a/source/blender/compositor/intern/COM_WorkScheduler.h b/source/blender/compositor/intern/COM_WorkScheduler.h
index 758bd8feb26..85b1d7e2ebf 100644
--- a/source/blender/compositor/intern/COM_WorkScheduler.h
+++ b/source/blender/compositor/intern/COM_WorkScheduler.h
@@ -36,10 +36,8 @@ struct WorkScheduler {
    * when ExecutionGroup.get_flags().open_cl is set the work will be handled by a OpenCLDevice
    * otherwise the work is scheduled for an CPUDevice
    * \see ExecutionGroup.execute
-   * \param group: the execution group
-   * \param chunkNumber: the number of the chunk in the group to be executed
    */
-  static void schedule(ExecutionGroup *group, int chunkNumber);
+  static void schedule(WorkPackage *package);
 
   /**
    * \brief initialize the WorkScheduler