[Bf-blender-cvs] [d48735cca2c] master: Functions: speedup multi-function procedure executor

Jacques Lucke noreply at git.blender.org
Sun Jun 19 14:26:08 CEST 2022


Commit: d48735cca2c69cefa7d4cdf3128cfdcd1beeb909
Author: Jacques Lucke
Date:   Sun Jun 19 14:25:21 2022 +0200
Branches: master
https://developer.blender.org/rBd48735cca2c69cefa7d4cdf3128cfdcd1beeb909

Functions: speedup multi-function procedure executor

This improves performance of the procedure executor on secondary metrics
(i.e. not for the main use case when many elements are processed together,
but for the use case when a single element is processed at a time).

In my benchmark I'm measuring a 50-60% improvement:
* Procedure with a single function (executed many times): `5.8s -> 2.7s`.
* Procedure with 1000 functions (executed many times): `2.4 -> 1.0s`.

The speedup is mainly achieved in multiple ways:
* Store an `Array` of variable states, instead of a map. The array is indexed
  with indices stored in each variable. This also avoids separately allocating
  variable states.
* Move less data around in the scheduler and use a `Stack` instead of `Map`.
  `Map` was used before because it allows for some optimizations that might
  be more important in the future, but they don't matter right now (e.g. joining
  execution paths that diverged earlier).
* Avoid memory allocations by giving the `LinearAllocator` some memory
  from the stack.

===================================================================

M	source/blender/functions/FN_multi_function_procedure.hh
M	source/blender/functions/intern/multi_function_procedure.cc
M	source/blender/functions/intern/multi_function_procedure_executor.cc

===================================================================

diff --git a/source/blender/functions/FN_multi_function_procedure.hh b/source/blender/functions/FN_multi_function_procedure.hh
index 75a54992a48..da269b08155 100644
--- a/source/blender/functions/FN_multi_function_procedure.hh
+++ b/source/blender/functions/FN_multi_function_procedure.hh
@@ -87,7 +87,7 @@ class MFVariable : NonCopyable, NonMovable {
   MFDataType data_type_;
   Vector<MFInstruction *> users_;
   std::string name_;
-  int id_;
+  int index_in_graph_;
 
   friend MFProcedure;
   friend MFCallInstruction;
@@ -101,7 +101,7 @@ class MFVariable : NonCopyable, NonMovable {
   StringRefNull name() const;
   void set_name(std::string name);
 
-  int id() const;
+  int index_in_procedure() const;
 };
 
 /** Base class for all instruction types. */
@@ -376,9 +376,9 @@ inline StringRefNull MFVariable::name() const
   return name_;
 }
 
-inline int MFVariable::id() const
+inline int MFVariable::index_in_procedure() const
 {
-  return id_;
+  return index_in_graph_;
 }
 
 /** \} */
diff --git a/source/blender/functions/intern/multi_function_procedure.cc b/source/blender/functions/intern/multi_function_procedure.cc
index bc045bfd44b..7ad324a9574 100644
--- a/source/blender/functions/intern/multi_function_procedure.cc
+++ b/source/blender/functions/intern/multi_function_procedure.cc
@@ -173,7 +173,7 @@ MFVariable &MFProcedure::new_variable(MFDataType data_type, std::string name)
   MFVariable &variable = *allocator_.construct<MFVariable>().release();
   variable.name_ = std::move(name);
   variable.data_type_ = data_type;
-  variable.id_ = variables_.size();
+  variable.index_in_graph_ = variables_.size();
   variables_.append(&variable);
   return variable;
 }
@@ -753,7 +753,7 @@ class MFProcedureDotExport {
       ss << "null";
     }
     else {
-      ss << "$" << variable->id();
+      ss << "$" << variable->index_in_procedure();
       if (!variable->name().is_empty()) {
         ss << "(" << variable->name() << ")";
       }
diff --git a/source/blender/functions/intern/multi_function_procedure_executor.cc b/source/blender/functions/intern/multi_function_procedure_executor.cc
index d852fe19924..a45240dad55 100644
--- a/source/blender/functions/intern/multi_function_procedure_executor.cc
+++ b/source/blender/functions/intern/multi_function_procedure_executor.cc
@@ -157,10 +157,6 @@ class ValueAllocator : NonCopyable, NonMovable {
   {
   }
 
-  template<typename... Args> VariableState *obtain_variable_state(Args &&...args);
-
-  void release_variable_state(VariableState *state);
-
   VariableValue_GVArray *obtain_GVArray(const GVArray &varray)
   {
     return this->obtain<VariableValue_GVArray>(varray);
@@ -294,32 +290,27 @@ class ValueAllocator : NonCopyable, NonMovable {
  * This class keeps track of a single variable during evaluation.
  */
 class VariableState : NonCopyable, NonMovable {
- private:
+ public:
   /** The current value of the variable. The storage format may change over time. */
-  VariableValue *value_;
+  VariableValue *value_ = nullptr;
   /** Number of indices that are currently initialized in this variable. */
-  int tot_initialized_;
+  int tot_initialized_ = 0;
   /* This a non-owning pointer to either span buffer or #GVectorArray or null. */
   void *caller_provided_storage_ = nullptr;
 
- public:
-  VariableState(VariableValue &value, int tot_initialized, void *caller_provided_storage = nullptr)
-      : value_(&value),
-        tot_initialized_(tot_initialized),
-        caller_provided_storage_(caller_provided_storage)
-  {
-  }
-
-  void destruct_self(ValueAllocator &value_allocator, const MFDataType &data_type)
+  void destruct_value(ValueAllocator &value_allocator, const MFDataType &data_type)
   {
     value_allocator.release_value(value_, data_type);
-    value_allocator.release_variable_state(this);
+    value_ = nullptr;
   }
 
   /* True if this contains only one value for all indices, i.e. the value for all indices is
    * the same. */
   bool is_one() const
   {
+    if (value_ == nullptr) {
+      return true;
+    }
     switch (value_->type) {
       case ValueType::GVArray:
         return this->value_as<VariableValue_GVArray>()->data.is_single();
@@ -353,6 +344,7 @@ class VariableState : NonCopyable, NonMovable {
   {
     /* Sanity check to make sure that enough values are initialized. */
     BLI_assert(mask.size() <= tot_initialized_);
+    BLI_assert(value_ != nullptr);
 
     switch (value_->type) {
       case ValueType::GVArray: {
@@ -391,7 +383,7 @@ class VariableState : NonCopyable, NonMovable {
                          const MFDataType &data_type,
                          ValueAllocator &value_allocator)
   {
-    if (ELEM(value_->type, ValueType::Span, ValueType::GVectorArray)) {
+    if (value_ != nullptr && ELEM(value_->type, ValueType::Span, ValueType::GVectorArray)) {
       return;
     }
 
@@ -408,22 +400,24 @@ class VariableState : NonCopyable, NonMovable {
           /* Reuse the storage provided caller when possible. */
           new_value = value_allocator.obtain_Span_not_owned(caller_provided_storage_);
         }
-        if (value_->type == ValueType::GVArray) {
-          /* Fill new buffer with data from virtual array. */
-          this->value_as<VariableValue_GVArray>()->data.materialize_to_uninitialized(
-              full_mask, new_value->data);
-        }
-        else if (value_->type == ValueType::OneSingle) {
-          auto *old_value_typed_ = this->value_as<VariableValue_OneSingle>();
-          if (old_value_typed_->is_initialized) {
-            /* Fill the buffer with a single value. */
-            type.fill_construct_indices(old_value_typed_->data, new_value->data, full_mask);
+        if (value_ != nullptr) {
+          if (value_->type == ValueType::GVArray) {
+            /* Fill new buffer with data from virtual array. */
+            this->value_as<VariableValue_GVArray>()->data.materialize_to_uninitialized(
+                full_mask, new_value->data);
           }
+          else if (value_->type == ValueType::OneSingle) {
+            auto *old_value_typed_ = this->value_as<VariableValue_OneSingle>();
+            if (old_value_typed_->is_initialized) {
+              /* Fill the buffer with a single value. */
+              type.fill_construct_indices(old_value_typed_->data, new_value->data, full_mask);
+            }
+          }
+          else {
+            BLI_assert_unreachable();
+          }
+          value_allocator.release_value(value_, data_type);
         }
-        else {
-          BLI_assert_unreachable();
-        }
-        value_allocator.release_value(value_, data_type);
         value_ = new_value;
         break;
       }
@@ -437,19 +431,21 @@ class VariableState : NonCopyable, NonMovable {
           new_value = value_allocator.obtain_GVectorArray_not_owned(
               *(GVectorArray *)caller_provided_storage_);
         }
-        if (value_->type == ValueType::GVVectorArray) {
-          /* Fill new vector array with data from virtual vector array. */
-          new_value->data.extend(full_mask, this->value_as<VariableValue_GVVectorArray>()->data);
-        }
-        else if (value_->type == ValueType::OneVector) {
-          /* Fill all indices with the same value. */
-          const GSpan vector = this->value_as<VariableValue_OneVector>()->data[0];
-          new_value->data.extend(full_mask, GVVectorArray_For_SingleGSpan{vector, array_size});
-        }
-        else {
-          BLI_assert_unreachable();
+        if (value_ != nullptr) {
+          if (value_->type == ValueType::GVVectorArray) {
+            /* Fill new vector array with data from virtual vector array. */
+            new_value->data.extend(full_mask, this->value_as<VariableValue_GVVectorArray>()->data);
+          }
+          else if (value_->type == ValueType::OneVector) {
+            /* Fill all indices with the same value. */
+            const GSpan vector = this->value_as<VariableValue_OneVector>()->data[0];
+            new_value->data.extend(full_mask, GVVectorArray_For_SingleGSpan{vector, array_size});
+          }
+          else {
+            BLI_assert_unreachable();
+          }
+          value_allocator.release_value(value_, data_type);
         }
-        value_allocator.release_value(value_, data_type);
         value_ = new_value;
         break;
       }
@@ -466,6 +462,7 @@ class VariableState : NonCopyable, NonMovable {
     BLI_assert(mask.size() <= tot_initialized_);
 
     this->ensure_is_mutable(full_mask, data_type, value_allocator);
+    BLI_assert(value_ != nullptr);
 
     switch (value_->type) {
       case ValueType::Span: {
@@ -497,6 +494,7 @@ class VariableState : NonCopyable, NonMovable {
     /* Sanity check to make sure that enough values are not initialized. */
     BLI_assert(mask.size() <= full_mask.size() - tot_initialized_);
     this->ensure_is_mutable(full_mask, data_type, value_allocator);
+    BLI_assert(value_ != nullptr);
 
     switch (value_->type) {
       case ValueType::Span: {
@@ -524,6 +522,7 @@ class VariableState : NonCopyable, NonMovable {
   void add_as_input__one(MFParamsBuilder &params, const MFDataType &data_type) const
   {
     BLI_assert(this->is_one());
+    BLI_assert(value_ != nullptr);
 
     switch (value_->type) {
       case ValueType::GVArray: {
@@ -556,7 +555,7 @@ class VariableState : NonCopyable, NonMovable {
   void ensure_is_mutable__one(const MFDataType &data_type, ValueAllocator &value_allocator)
   {
     BLI_assert(this->is_one());
-    if (ELEM(value_->type, ValueType::OneSingle, ValueType::OneVector)) {
+    if (value_ != nullptr && ELEM(value_->type, ValueType::OneSingle, ValueType::OneVector)) {
       return;
     }
 
@@ -564,38 +563,42 @@ class VariableState : NonCopyable, NonMovable {
       case MFDataType::Single: {
         const CPPType &type = data_type.single_type();
         VariableValue_OneSingle *new_value = value_allocator.obtain_OneSingle(type);
-        if (value_->type == ValueType::GVArray) {
-          this->value_as<VariableValue_GVArray>()->data.get_internal_single_to_uninitialized(
-              new_value->data);
-          new_value->is_initialized = true;
-        }
-        else if (value_->type == ValueType::Span) {
-          

@@ Diff output truncated at 10240 characters. @@



More information about the Bf-blender-cvs mailing list