[WIP] Cache RNA_path_resolve in dependency graph #77124

New Issue

Jeroen Bakker · 2020-05-28T14:53:23+02:00

Jeroen Bakker commented

2020-05-28 14:53:23 +02:00

State: Study to come to a proposal

Based on the information from #75208 (Dependency Graph Evaluation Optimizations)

During build time not all RNA_path_resolves can be determined.

Investigation

Current Code

depsgraph_query.c

RNA_path_resolve is used in DEG_get_evaluated_rna_pointer what isn't being used. Should we remove this function?

depsgraph_builder_cache
Introduced by c8f3377d03 which is already adding a cache around RNA_path_resolve_property

deg_builder_relations.c

there seems like an unneeded second lookup in build_driver_data; property_entry_key is the same as property_exit_key, but uses a different source parameter. [D7872: Depsgraph: Remove unneeded RNA_path_resolve`](https://archive.blender.org/developer/D7872)

Cache hits

A cache only works if we can get a mechanism working with has cache hits. If a cache items is only used once (when created) it will add overhead. If a cache item can be shared across frames and or objects it is potentially better to cache. There is a potential that some rna_paths can be cached in the IDType. They could even by const evaluators as they can be determined at compile time and don't need to be cached at all. examples are 'Object.location/rotation/scale`.

The core of the dependency graph does all RNA_path_resolves during loading. When playing back the animation it doesn't call any RNA_path_resolve* (P1434: Research: DEG RNA path resolve

diff --git a/source/blender/depsgraph/intern/builder/deg_builder_cache.cc b/source/blender/depsgraph/intern/builder/deg_builder_cache.cc
index ba0238b43c7..4d4671c472a 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_cache.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder_cache.cc
@@ -54,6 +54,7 @@ AnimatedPropertyID::AnimatedPropertyID(const PointerRNA &pointer_rna,
 AnimatedPropertyID::AnimatedPropertyID(ID *id, StructRNA *type, const char *property_name)
     : data(id)
 {
+  std::cout << __func__ << "\n";
   property_rna = RNA_struct_type_find_property(type, property_name);
 }
 
@@ -63,6 +64,7 @@ AnimatedPropertyID::AnimatedPropertyID(ID * /*id*/,
                                        const char *property_name)
     : data(data)
 {
+  std::cout << __func__ << "\n";
   property_rna = RNA_struct_type_find_property(type, property_name);
 }
 
@@ -94,6 +96,7 @@ void animated_property_cb(ID * /*id*/, FCurve *fcurve, void *data_v)
   /* Resolve property. */
   PointerRNA pointer_rna;
   PropertyRNA *property_rna = nullptr;
+  std::cout << __func__ << "\n";
   if (!RNA_path_resolve_property(
           &data->pointer_rna, fcurve->rna_path, &pointer_rna, &property_rna)) {
     return;
diff --git a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
index 9230fa19c32..85ee82fc17c 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
@@ -1007,7 +1007,7 @@ void DepsgraphNodeBuilder::build_driver_id_property(ID *id, const char *rna_path
   PropertyRNA *prop;
   int index;
   RNA_id_pointer_create(id, &id_ptr);
-  if (!RNA_path_resolve_full(&id_ptr, rna_path, &ptr, &prop, &index)) {
+  if (!RNA_path_resolve_full_log(__func__, &id_ptr, rna_path, &ptr, &prop, &index)) {
     return;
   }
   if (prop == nullptr) {
diff --git a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc
index c98aab51654..4dd4b9be33f 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc
@@ -1295,7 +1295,7 @@ void DepsgraphRelationBuilder::build_animdata_curves_targets(ID *id,
     PointerRNA ptr;
     PropertyRNA *prop;
     int index;
-    if (!RNA_path_resolve_full(&id_ptr, fcu->rna_path, &ptr, &prop, &index)) {
+    if (!RNA_path_resolve_full_log(__func__, &id_ptr, fcu->rna_path, &ptr, &prop, &index)) {
       continue;
     }
     Node *node_to = rna_node_query_.find_node(&ptr, prop, RNAPointerSource::ENTRY);
@@ -1492,7 +1492,7 @@ void DepsgraphRelationBuilder::build_driver_data(ID *id, FCurve *fcu)
       PointerRNA id_ptr;
       PointerRNA ptr;
       RNA_id_pointer_create(id, &id_ptr);
-      if (RNA_path_resolve_full(&id_ptr, fcu->rna_path, &ptr, nullptr, nullptr)) {
+      if (RNA_path_resolve_full_log(__func__, &id_ptr, fcu->rna_path, &ptr, nullptr, nullptr)) {
         if (id_ptr.owner_id != ptr.owner_id) {
           ComponentKey cow_key(ptr.owner_id, NodeType::COPY_ON_WRITE);
           add_relation(cow_key, driver_key, "Driven CoW -> Driver", RELATION_CHECK_BEFORE_ADD);
@@ -1593,7 +1593,7 @@ void DepsgraphRelationBuilder::build_driver_id_property(ID *id, const char *rna_
   PropertyRNA *prop;
   int index;
   RNA_id_pointer_create(id, &id_ptr);
-  if (!RNA_path_resolve_full(&id_ptr, rna_path, &ptr, &prop, &index)) {
+  if (!RNA_path_resolve_full_log(__func__, &id_ptr, rna_path, &ptr, &prop, &index)) {
     return;
   }
   if (prop == nullptr) {
diff --git a/source/blender/depsgraph/intern/builder/deg_builder_relations_keys.cc b/source/blender/depsgraph/intern/builder/deg_builder_relations_keys.cc
index 6c449900f03..4ebcab488f2 100644
--- a/source/blender/depsgraph/intern/builder/deg_builder_relations_keys.cc
+++ b/source/blender/depsgraph/intern/builder/deg_builder_relations_keys.cc
@@ -174,7 +174,7 @@ RNAPathKey::RNAPathKey(ID *id, const char *path, RNAPointerSource source) : id(i
   RNA_id_pointer_create(id, &id_ptr);
   /* Try to resolve path. */
   int index;
-  if (!RNA_path_resolve_full(&id_ptr, path, &ptr, &prop, &index)) {
+  if (!RNA_path_resolve_full_log(__func__, &id_ptr, path, &ptr, &prop, &index)) {
     ptr = PointerRNA_NULL;
     prop = nullptr;
   }
diff --git a/source/blender/makesrna/RNA_access.h b/source/blender/makesrna/RNA_access.h
index d2e27bdbcad..13dbeaec98d 100644
--- a/source/blender/makesrna/RNA_access.h
+++ b/source/blender/makesrna/RNA_access.h
@@ -1073,6 +1073,12 @@ bool RNA_path_resolve(PointerRNA *ptr, const char *path, PointerRNA *r_ptr, Prop
 
 bool RNA_path_resolve_full(
     PointerRNA *ptr, const char *path, PointerRNA *r_ptr, PropertyRNA **r_prop, int *r_index);
+bool RNA_path_resolve_full_log(const char *log_name,
+                               PointerRNA *ptr,
+                               const char *path,
+                               PointerRNA *r_ptr,
+                               PropertyRNA **r_prop,
+                               int *r_index);
 
 /* path_resolve_property() variants ensure that pointer + property both exist */
 bool RNA_path_resolve_property(PointerRNA *ptr,
diff --git a/source/blender/makesrna/intern/rna_access.c b/source/blender/makesrna/intern/rna_access.c
index f14e81a38df..546e0df5bf0 100644
--- a/source/blender/makesrna/intern/rna_access.c
+++ b/source/blender/makesrna/intern/rna_access.c
@@ -5334,6 +5334,25 @@ bool RNA_path_resolve_full(
   return r_ptr->data != NULL;
 }
 
+bool RNA_path_resolve_full_log(const char *log_name,
+                               PointerRNA *ptr,
+                               const char *path,
+                               PointerRNA *r_ptr,
+                               PropertyRNA **r_prop,
+                               int *r_index)
+{
+  bool result = RNA_path_resolve_full(ptr, path, r_ptr, r_prop, r_index);
+  printf("%s, %p, %s, %s, ->, %p, %p, %d\n",
+         log_name,
+         ptr->data,
+         ptr->owner_id->name,
+         path,
+         r_ptr,
+         r_prop ? *r_prop : NULL,
+         r_index ? *r_index : -999);
+  return result;
+}
+
 /**
  * Resolve the given RNA Path to find both the pointer AND property
  * indicated by fully resolving the path.

does not output anything when playing back animation). This proofs that adding a dependency specific rna resolve cache wouldn't be a solution we would be needed.

deg_* seems to be evaluated once when loading so caching this won't improve playback performance. A reason why we would want to cache this might be that the cache will be populated at this time so it reduces synchronization overhead during dependency graph evaluation.

This points us more in the direction to look into other areas where RNA_path_resolve is used inside a Node.

TODO

- create data that would help us check the best caching strategy

Research

Is it possible to use a non-blocking lazy retrieval of the RNA_path_resolve during eval time.
Add an evaluation expression cache on ID block similar to Python Expression.
many rna paths are animated in groups (location.x, y z) by caching intermediates might also increase performance. as a single call will set the right pointer, and the second and third would only need to set the index. There the longer the paths are the more performance we get.
FCurve: Could we add a FCurve_Runtime what holds a List of RNAProperties that can be evaluated when needed so it doesn't need to be constructed every time.

Proposal

This proposal is still heavily influenced by any progress in this ticket.

CLEANUP: Should we remove DEG_get_evaluated_rna_pointer as it isn't used anymore?

**State: Study to come to a proposal** Based on the information from #75208 (Dependency Graph Evaluation Optimizations) During build time not all RNA_path_resolves can be determined. ## Investigation ### Current Code **depsgraph_query.c** * `RNA_path_resolve` is used in `DEG_get_evaluated_rna_pointer` what isn't being used. Should we remove this function? **depsgraph_builder_cache** Introduced by c8f3377d03 which is already adding a cache around `RNA_path_resolve_property` **deg_builder_relations.c** * there seems like an unneeded second lookup in `build_driver_data`; `property_entry_key` is the same as `property_exit_key`, but uses a different `source parameter. [D7872: Depsgraph: Remove unneeded `RNA_path_resolve`](https://archive.blender.org/developer/D7872) ### Cache hits A cache only works if we can get a mechanism working with has cache hits. If a cache items is only used once (when created) it will add overhead. If a cache item can be shared across frames and or objects it is potentially better to cache. There is a potential that some rna_paths can be cached in the IDType. They could even by const evaluators as they can be determined at compile time and don't need to be cached at all. examples are 'Object.location/rotation/scale`. The core of the dependency graph does all RNA_path_resolves during loading. When playing back the animation it doesn't call any RNA_path_resolve* ([P1434: Research: DEG RNA path resolve](https://archive.blender.org/developer/P1434.txt) ``` diff --git a/source/blender/depsgraph/intern/builder/deg_builder_cache.cc b/source/blender/depsgraph/intern/builder/deg_builder_cache.cc index ba0238b43c7..4d4671c472a 100644 --- a/source/blender/depsgraph/intern/builder/deg_builder_cache.cc +++ b/source/blender/depsgraph/intern/builder/deg_builder_cache.cc @@ -54,6 +54,7 @@ AnimatedPropertyID::AnimatedPropertyID(const PointerRNA &pointer_rna, AnimatedPropertyID::AnimatedPropertyID(ID *id, StructRNA *type, const char *property_name) : data(id) { + std::cout << __func__ << "\n"; property_rna = RNA_struct_type_find_property(type, property_name); } @@ -63,6 +64,7 @@ AnimatedPropertyID::AnimatedPropertyID(ID * /*id*/, const char *property_name) : data(data) { + std::cout << __func__ << "\n"; property_rna = RNA_struct_type_find_property(type, property_name); } @@ -94,6 +96,7 @@ void animated_property_cb(ID * /*id*/, FCurve *fcurve, void *data_v) /* Resolve property. */ PointerRNA pointer_rna; PropertyRNA *property_rna = nullptr; + std::cout << __func__ << "\n"; if (!RNA_path_resolve_property( &data->pointer_rna, fcurve->rna_path, &pointer_rna, &property_rna)) { return; diff --git a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc index 9230fa19c32..85ee82fc17c 100644 --- a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc +++ b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc @@ -1007,7 +1007,7 @@ void DepsgraphNodeBuilder::build_driver_id_property(ID *id, const char *rna_path PropertyRNA *prop; int index; RNA_id_pointer_create(id, &id_ptr); - if (!RNA_path_resolve_full(&id_ptr, rna_path, &ptr, &prop, &index)) { + if (!RNA_path_resolve_full_log(__func__, &id_ptr, rna_path, &ptr, &prop, &index)) { return; } if (prop == nullptr) { diff --git a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc index c98aab51654..4dd4b9be33f 100644 --- a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc +++ b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc @@ -1295,7 +1295,7 @@ void DepsgraphRelationBuilder::build_animdata_curves_targets(ID *id, PointerRNA ptr; PropertyRNA *prop; int index; - if (!RNA_path_resolve_full(&id_ptr, fcu->rna_path, &ptr, &prop, &index)) { + if (!RNA_path_resolve_full_log(__func__, &id_ptr, fcu->rna_path, &ptr, &prop, &index)) { continue; } Node *node_to = rna_node_query_.find_node(&ptr, prop, RNAPointerSource::ENTRY); @@ -1492,7 +1492,7 @@ void DepsgraphRelationBuilder::build_driver_data(ID *id, FCurve *fcu) PointerRNA id_ptr; PointerRNA ptr; RNA_id_pointer_create(id, &id_ptr); - if (RNA_path_resolve_full(&id_ptr, fcu->rna_path, &ptr, nullptr, nullptr)) { + if (RNA_path_resolve_full_log(__func__, &id_ptr, fcu->rna_path, &ptr, nullptr, nullptr)) { if (id_ptr.owner_id != ptr.owner_id) { ComponentKey cow_key(ptr.owner_id, NodeType::COPY_ON_WRITE); add_relation(cow_key, driver_key, "Driven CoW -> Driver", RELATION_CHECK_BEFORE_ADD); @@ -1593,7 +1593,7 @@ void DepsgraphRelationBuilder::build_driver_id_property(ID *id, const char *rna_ PropertyRNA *prop; int index; RNA_id_pointer_create(id, &id_ptr); - if (!RNA_path_resolve_full(&id_ptr, rna_path, &ptr, &prop, &index)) { + if (!RNA_path_resolve_full_log(__func__, &id_ptr, rna_path, &ptr, &prop, &index)) { return; } if (prop == nullptr) { diff --git a/source/blender/depsgraph/intern/builder/deg_builder_relations_keys.cc b/source/blender/depsgraph/intern/builder/deg_builder_relations_keys.cc index 6c449900f03..4ebcab488f2 100644 --- a/source/blender/depsgraph/intern/builder/deg_builder_relations_keys.cc +++ b/source/blender/depsgraph/intern/builder/deg_builder_relations_keys.cc @@ -174,7 +174,7 @@ RNAPathKey::RNAPathKey(ID *id, const char *path, RNAPointerSource source) : id(i RNA_id_pointer_create(id, &id_ptr); /* Try to resolve path. */ int index; - if (!RNA_path_resolve_full(&id_ptr, path, &ptr, &prop, &index)) { + if (!RNA_path_resolve_full_log(__func__, &id_ptr, path, &ptr, &prop, &index)) { ptr = PointerRNA_NULL; prop = nullptr; } diff --git a/source/blender/makesrna/RNA_access.h b/source/blender/makesrna/RNA_access.h index d2e27bdbcad..13dbeaec98d 100644 --- a/source/blender/makesrna/RNA_access.h +++ b/source/blender/makesrna/RNA_access.h @@ -1073,6 +1073,12 @@ bool RNA_path_resolve(PointerRNA *ptr, const char *path, PointerRNA *r_ptr, Prop bool RNA_path_resolve_full( PointerRNA *ptr, const char *path, PointerRNA *r_ptr, PropertyRNA **r_prop, int *r_index); +bool RNA_path_resolve_full_log(const char *log_name, + PointerRNA *ptr, + const char *path, + PointerRNA *r_ptr, + PropertyRNA **r_prop, + int *r_index); /* path_resolve_property() variants ensure that pointer + property both exist */ bool RNA_path_resolve_property(PointerRNA *ptr, diff --git a/source/blender/makesrna/intern/rna_access.c b/source/blender/makesrna/intern/rna_access.c index f14e81a38df..546e0df5bf0 100644 --- a/source/blender/makesrna/intern/rna_access.c +++ b/source/blender/makesrna/intern/rna_access.c @@ -5334,6 +5334,25 @@ bool RNA_path_resolve_full( return r_ptr->data != NULL; } +bool RNA_path_resolve_full_log(const char *log_name, + PointerRNA *ptr, + const char *path, + PointerRNA *r_ptr, + PropertyRNA **r_prop, + int *r_index) +{ + bool result = RNA_path_resolve_full(ptr, path, r_ptr, r_prop, r_index); + printf("%s, %p, %s, %s, ->, %p, %p, %d\n", + log_name, + ptr->data, + ptr->owner_id->name, + path, + r_ptr, + r_prop ? *r_prop : NULL, + r_index ? *r_index : -999); + return result; +} + /** * Resolve the given RNA Path to find both the pointer AND property * indicated by fully resolving the path. ``` does not output anything when playing back animation). This proofs that adding a dependency specific rna resolve cache wouldn't be a solution we would be needed. deg_* seems to be evaluated once when loading so caching this won't improve playback performance. A reason why we would want to cache this might be that the cache will be populated at this time so it reduces synchronization overhead during dependency graph evaluation. This points us more in the direction to look into other areas where RNA_path_resolve is used inside a Node. **TODO** * - [ ] create data that would help us check the best caching strategy ## Research * Is it possible to use a non-blocking lazy retrieval of the RNA_path_resolve during eval time. * Add an evaluation expression cache on ID block similar to Python Expression. * many rna paths are animated in groups (location.x, y z) by caching intermediates might also increase performance. as a single call will set the right pointer, and the second and third would only need to set the index. There the longer the paths are the more performance we get. * FCurve: Could we add a FCurve_Runtime what holds a List of RNAProperties that can be evaluated when needed so it doesn't need to be constructed every time. ## Proposal This proposal is still heavily influenced by any progress in this ticket. * CLEANUP: Should we remove `DEG_get_evaluated_rna_pointer` as it isn't used anymore?

Jeroen Bakker self-assigned this 2020-05-28 14:53:23 +02:00

Jeroen Bakker commented

2020-05-28 14:53:23 +02:00

Added subscriber: @Jeroen-Bakker

Jeroen Bakker commented

2020-05-28 15:01:45 +02:00

Changed status from 'Needs Triage' to: 'Confirmed'

Jeroen Bakker changed title from ~~Cache `RNA_path_resolve` in dependency graph~~ to [WIP] Cache `RNA_path_resolve` in dependency graph

2020-05-29 07:06:44 +02:00

Jeroen Bakker commented

2022-02-14 15:46:59 +01:00

Changed status from 'Confirmed' to: 'Archived'

Jeroen Bakker closed this issue

2022-02-14 15:46:59 +01:00

Sign in to join this conversation.

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

[WIP] Cache RNA_path_resolve in dependency graph #77124

Investigation

Current Code

Cache hits

Research

Proposal

[WIP] Cache `RNA_path_resolve` in dependency graph #77124