Page MenuHome
Create Task
Maniphest T77124

[WIP] Cache `RNA_path_resolve` in dependency graph
Confirmed, NormalPublicTO DO
Actions

Description

State: Study to come to a proposal

Based on the information from T75208: 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 rBc8f3377d0353: Depsgraph: Add generic animated properties cache which is already adding a cache around RNA_path_resolve_property

deg_builder_relations.c

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
1diff --git a/source/blender/depsgraph/intern/builder/deg_builder_cache.cc b/source/blender/depsgraph/intern/builder/deg_builder_cache.cc
2index ba0238b43c7..4d4671c472a 100644
3--- a/source/blender/depsgraph/intern/builder/deg_builder_cache.cc
4+++ b/source/blender/depsgraph/intern/builder/deg_builder_cache.cc
5@@ -54,6 +54,7 @@ AnimatedPropertyID::AnimatedPropertyID(const PointerRNA &pointer_rna,
6 AnimatedPropertyID::AnimatedPropertyID(ID *id, StructRNA *type, const char *property_name)
7 : data(id)
8 {
9+ std::cout << __func__ << "\n";
10 property_rna = RNA_struct_type_find_property(type, property_name);
11 }
12
13@@ -63,6 +64,7 @@ AnimatedPropertyID::AnimatedPropertyID(ID * /*id*/,
14 const char *property_name)
15 : data(data)
16 {
17+ std::cout << __func__ << "\n";
18 property_rna = RNA_struct_type_find_property(type, property_name);
19 }
20
21@@ -94,6 +96,7 @@ void animated_property_cb(ID * /*id*/, FCurve *fcurve, void *data_v)
22 /* Resolve property. */
23 PointerRNA pointer_rna;
24 PropertyRNA *property_rna = nullptr;
25+ std::cout << __func__ << "\n";
26 if (!RNA_path_resolve_property(
27 &data->pointer_rna, fcurve->rna_path, &pointer_rna, &property_rna)) {
28 return;
29diff --git a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
30index 9230fa19c32..85ee82fc17c 100644
31--- a/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
32+++ b/source/blender/depsgraph/intern/builder/deg_builder_nodes.cc
33@@ -1007,7 +1007,7 @@ void DepsgraphNodeBuilder::build_driver_id_property(ID *id, const char *rna_path
34 PropertyRNA *prop;
35 int index;
36 RNA_id_pointer_create(id, &id_ptr);
37- if (!RNA_path_resolve_full(&id_ptr, rna_path, &ptr, &prop, &index)) {
38+ if (!RNA_path_resolve_full_log(__func__, &id_ptr, rna_path, &ptr, &prop, &index)) {
39 return;
40 }
41 if (prop == nullptr) {
42diff --git a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc
43index c98aab51654..4dd4b9be33f 100644
44--- a/source/blender/depsgraph/intern/builder/deg_builder_relations.cc
45+++ b/source/blender/depsgraph/intern/builder/deg_builder_relations.cc
46@@ -1295,7 +1295,7 @@ void DepsgraphRelationBuilder::build_animdata_curves_targets(ID *id,
47 PointerRNA ptr;
48 PropertyRNA *prop;
49 int index;
50- if (!RNA_path_resolve_full(&id_ptr, fcu->rna_path, &ptr, &prop, &index)) {
51+ if (!RNA_path_resolve_full_log(__func__, &id_ptr, fcu->rna_path, &ptr, &prop, &index)) {
52 continue;
53 }
54 Node *node_to = rna_node_query_.find_node(&ptr, prop, RNAPointerSource::ENTRY);
55@@ -1492,7 +1492,7 @@ void DepsgraphRelationBuilder::build_driver_data(ID *id, FCurve *fcu)
56 PointerRNA id_ptr;
57 PointerRNA ptr;
58 RNA_id_pointer_create(id, &id_ptr);
59- if (RNA_path_resolve_full(&id_ptr, fcu->rna_path, &ptr, nullptr, nullptr)) {
60+ if (RNA_path_resolve_full_log(__func__, &id_ptr, fcu->rna_path, &ptr, nullptr, nullptr)) {
61 if (id_ptr.owner_id != ptr.owner_id) {
62 ComponentKey cow_key(ptr.owner_id, NodeType::COPY_ON_WRITE);
63 add_relation(cow_key, driver_key, "Driven CoW -> Driver", RELATION_CHECK_BEFORE_ADD);
64@@ -1593,7 +1593,7 @@ void DepsgraphRelationBuilder::build_driver_id_property(ID *id, const char *rna_
65 PropertyRNA *prop;
66 int index;
67 RNA_id_pointer_create(id, &id_ptr);
68- if (!RNA_path_resolve_full(&id_ptr, rna_path, &ptr, &prop, &index)) {
69+ if (!RNA_path_resolve_full_log(__func__, &id_ptr, rna_path, &ptr, &prop, &index)) {
70 return;
71 }
72 if (prop == nullptr) {
73diff --git a/source/blender/depsgraph/intern/builder/deg_builder_relations_keys.cc b/source/blender/depsgraph/intern/builder/deg_builder_relations_keys.cc
74index 6c449900f03..4ebcab488f2 100644
75--- a/source/blender/depsgraph/intern/builder/deg_builder_relations_keys.cc
76+++ b/source/blender/depsgraph/intern/builder/deg_builder_relations_keys.cc
77@@ -174,7 +174,7 @@ RNAPathKey::RNAPathKey(ID *id, const char *path, RNAPointerSource source) : id(i
78 RNA_id_pointer_create(id, &id_ptr);
79 /* Try to resolve path. */
80 int index;
81- if (!RNA_path_resolve_full(&id_ptr, path, &ptr, &prop, &index)) {
82+ if (!RNA_path_resolve_full_log(__func__, &id_ptr, path, &ptr, &prop, &index)) {
83 ptr = PointerRNA_NULL;
84 prop = nullptr;
85 }
86diff --git a/source/blender/makesrna/RNA_access.h b/source/blender/makesrna/RNA_access.h
87index d2e27bdbcad..13dbeaec98d 100644
88--- a/source/blender/makesrna/RNA_access.h
89+++ b/source/blender/makesrna/RNA_access.h
90@@ -1073,6 +1073,12 @@ bool RNA_path_resolve(PointerRNA *ptr, const char *path, PointerRNA *r_ptr, Prop
91
92 bool RNA_path_resolve_full(
93 PointerRNA *ptr, const char *path, PointerRNA *r_ptr, PropertyRNA **r_prop, int *r_index);
94+bool RNA_path_resolve_full_log(const char *log_name,
95+ PointerRNA *ptr,
96+ const char *path,
97+ PointerRNA *r_ptr,
98+ PropertyRNA **r_prop,
99+ int *r_index);
100
101 /* path_resolve_property() variants ensure that pointer + property both exist */
102 bool RNA_path_resolve_property(PointerRNA *ptr,
103diff --git a/source/blender/makesrna/intern/rna_access.c b/source/blender/makesrna/intern/rna_access.c
104index f14e81a38df..546e0df5bf0 100644
105--- a/source/blender/makesrna/intern/rna_access.c
106+++ b/source/blender/makesrna/intern/rna_access.c
107@@ -5334,6 +5334,25 @@ bool RNA_path_resolve_full(
108 return r_ptr->data != NULL;
109 }
110
111+bool RNA_path_resolve_full_log(const char *log_name,
112+ PointerRNA *ptr,
113+ const char *path,
114+ PointerRNA *r_ptr,
115+ PropertyRNA **r_prop,
116+ int *r_index)
117+{
118+ bool result = RNA_path_resolve_full(ptr, path, r_ptr, r_prop, r_index);
119+ printf("%s, %p, %s, %s, ->, %p, %p, %d\n",
120+ log_name,
121+ ptr->data,
122+ ptr->owner_id->name,
123+ path,
124+ r_ptr,
125+ r_prop ? *r_prop : NULL,
126+ r_index ? *r_index : -999);
127+ return result;
128+}
129+
130 /**
131 * Resolve the given RNA Path to find both the pointer AND property
132 * 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?

Revisions and Commits

rB Blender
Needs ReviewD7875 T77124: Remapping `RNA_path_resolve` to other ID Blocks for FCurves

Related Objects
Search...

StatusSubtypeAssignedTask
 ConfirmedTO DONoneT68908 Faster Animation Playback
 ConfirmedDESIGNNoneT75208 Dependency Graph Evaluation Optimizations
 ConfirmedTO DOJeroen Bakker (jbakker)T77124 [WIP] Cache `RNA_path_resolve` in dependency graph

Event Timeline

Jeroen Bakker (jbakker) created this task.May 28 2020, 2:53 PM
Jeroen Bakker (jbakker) changed the task status from Needs Triage to Confirmed.May 28 2020, 3:01 PM
Jeroen Bakker (jbakker) updated the task description. (Show Details)
Jeroen Bakker (jbakker) updated the task description. (Show Details)May 28 2020, 4:01 PM
Jeroen Bakker (jbakker) updated the task description. (Show Details)May 28 2020, 4:06 PM
Jeroen Bakker (jbakker) updated the task description. (Show Details)May 28 2020, 4:09 PM
Jeroen Bakker (jbakker) updated the task description. (Show Details)
Jeroen Bakker (jbakker) renamed this task from Cache `RNA_path_resolve` in dependency graph to [WIP] Cache `RNA_path_resolve` in dependency graph.May 29 2020, 7:06 AM
Jeroen Bakker (jbakker) updated the task description. (Show Details)
Jeroen Bakker (jbakker) updated the task description. (Show Details)May 29 2020, 7:28 AM
Jeroen Bakker (jbakker) updated the task description. (Show Details)May 29 2020, 9:28 AM
Jeroen Bakker (jbakker) updated the task description. (Show Details)May 29 2020, 9:37 AM
Jeroen Bakker (jbakker) mentioned this in D7872: Depsgraph: Remove unneeded `RNA_path_resolve`.May 29 2020, 9:57 AM
Jeroen Bakker (jbakker) updated the task description. (Show Details)May 29 2020, 10:12 AM
Jeroen Bakker (jbakker) added a revision: D7875: T77124: Remapping `RNA_path_resolve` to other ID Blocks for FCurves.May 29 2020, 2:19 PM
Sybren A. Stüvel (sybren) moved this task from Backlog to TODOs on the Animation & Rigging board.Jun 2 2020, 2:17 PM
Jeroen Bakker (jbakker) mentioned this in rB0749cff957c3: Depsgraph: Remove unneeded `RNA_path_resolve`.Jun 2 2020, 4:04 PM
Jeroen Bakker (jbakker) mentioned this in T78250: Performance: AnimationSystem - Action Evaluation Performance .Jun 25 2020, 4:15 PM
Sybren A. Stüvel (sybren) moved this task from TODOs to Needs Investigation on the Animation & Rigging board.Nov 16 2020, 3:54 PM