Details

Reviewers

Maniphest Tasks

T77124: [WIP] Cache `RNA_path_resolve` in dependency graph

Summary

This patch stores the done evaluation steps for an FCurve RNA path lookup inside the runtime of the FCurve.
This reduces the evaluation of fcurves. Multiple solutions have been tested including storing multiple
cache items at the end the simplest solution (single cache result) got the best performance.

This patch also introduces a more generic solution that can be applied to other locations (rna_access_cache.c)
Spring 020_02A.anim.blend went from 12.4 to 12.7 fps in AMD Ryzen 1700.

The current implementation uses a global lock as the building process during playback is most likely to be
performed from the main thread. The mutex is a just in case scenario. We might also want to remove the
mutex at all and make sure that the cache is populated on a single thread only.

There are still some clean ups to do before this patch will be send for review.

Before:

After:

The difference is that the lookup of the RNA_path in the drivers takes less time most noticeable in (BKE_animsys_eval_driver).

Diff Detail

Repository

rB Blender

Branch

arcpatch-D7875_1 (branched from master)

Build Status

Buildable 8929
Build 8929: arc lint + arc unit

Event Timeline

Jeroen Bakker (jbakker) created this revision.May 29 2020, 2:19 PM

Harbormaster completed remote builds in B8286: Diff 25233.May 29 2020, 2:19 PM

Jeroen Bakker (jbakker) requested review of this revision.May 29 2020, 2:19 PM

Jeroen Bakker (jbakker) planned changes to this revision.May 29 2020, 2:20 PM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)

First try on caching rna paths in the animation system

Harbormaster completed remote builds in B8321: Diff 25369.Jun 2 2020, 7:59 AM

Attached to the animation system.

Harbormaster completed remote builds in B8324: Diff 25386.Jun 2 2020, 2:50 PM

Rebased with latest master to continue working from office

Harbormaster completed remote builds in B8357: Diff 25459.Jun 3 2020, 5:34 PM

Fixing ASAN related issues

Harbormaster completed remote builds in B8372: Diff 25484.Jun 4 2020, 3:42 PM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Jun 4 2020, 3:44 PM

Fixed read from unallocated memory

Harbormaster completed remote builds in B8373: Diff 25485.Jun 4 2020, 4:37 PM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Jun 4 2020, 4:40 PM

Jeroen Bakker (jbakker) retitled this revision from T77124: Research for caching RNA_path_resolve to T77124: Research for remapping `RNA_path_resolve` to other ID Blocks.Jun 4 2020, 4:46 PM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)

Moved code to more generic places

Harbormaster completed remote builds in B8402: Diff 25535.Jun 5 2020, 4:47 PM

Jeroen Bakker (jbakker) added inline comments.Jun 5 2020, 8:29 PM

source/blender/blenkernel/intern/anim_sys.c
363 ↗	(On Diff #25535)	Merge with `BKE_animsys_store_rna_setting`
576 ↗	(On Diff #25535)	Merge with `animsys_write_orig_anim_rna`
source/blender/editors/space_graph/graph_buttons.c
384	RNAPathCache

Added multiple cache items

Harbormaster completed remote builds in B8523: Diff 25838.Jun 12 2020, 4:33 PM

Fixed cache misses

Harbormaster completed remote builds in B8543: Diff 25907.Jun 15 2020, 1:06 PM

Preparation to shift to interp multiple fcurves at once.

Harbormaster completed remote builds in B8686: Diff 26209.Jun 23 2020, 4:47 PM

Reduced complexity

Harbormaster completed remote builds in B8929: Diff 26660.Jul 9 2020, 3:43 PM

Enable fcurve rna cache in the animation system and dependency graph

Harbormaster completed remote builds in B8931: Diff 26662.Jul 9 2020, 4:11 PM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Jul 9 2020, 4:12 PM

Jeroen Bakker (jbakker) added a project: Performance.

Jeroen Bakker (jbakker) retitled this revision from T77124: Research for remapping `RNA_path_resolve` to other ID Blocks to T77124: Remapping `RNA_path_resolve` to other ID Blocks for FCurves.Jul 9 2020, 4:22 PM

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)

Jeroen Bakker (jbakker) added a project: Animation & Rigging.

Jeroen Bakker (jbakker) edited the summary of this revision. (Show Details)Jul 9 2020, 4:42 PM

Jeroen Bakker (jbakker) added inline comments.Jul 9 2020, 4:47 PM

source/blender/blenkernel/intern/anim_sys.c
576 ↗	(On Diff #25535)	No, it would add another branch that isn't needed.

Jeroen Bakker (jbakker) added inline comments.Jul 9 2020, 4:50 PM

source/blender/makesrna/intern/rna_access_cache.c
70	Use debug log

The patch description reads:

There are still some clean ups to do before this patch will be send for review.

but its status is "needs review".

Phabricator being what it is, I can't set it to "draft" or "planned changes" or anything, so I'll just set it to "request changes" so it's no longer listed as "needs review".

This revision now requires changes to proceed.Nov 1 2021, 5:10 PM

T77124: Remapping `RNA_path_resolve` to other ID Blocks for FCurves
Needs RevisionPublic
Actions

Details

Diff Detail

Event Timeline

Revision Contents
Changeset List

Diff 26660

source/blender/blenkernel/BKE_fcurve.h

source/blender/blenkernel/CMakeLists.txt

source/blender/blenkernel/intern/fcurve.c

source/blender/blenkernel/intern/fcurve_cache.c

source/blender/blenloader/intern/readfile.c

source/blender/depsgraph/CMakeLists.txt

source/blender/depsgraph/intern/builder/deg_builder_relations.cc

source/blender/editors/animation/anim_channels_defines.c

source/blender/editors/animation/anim_deps.c

source/blender/editors/animation/anim_draw.c

source/blender/editors/animation/anim_ipo_utils.c

source/blender/editors/animation/keyframes_general.c

source/blender/editors/animation/keyframing.c

source/blender/editors/armature/pose_slide.c

source/blender/editors/space_graph/graph_buttons.c

source/blender/makesdna/DNA_anim_types.h

source/blender/makesrna/RNA_access.h

source/blender/makesrna/intern/CMakeLists.txt

source/blender/makesrna/intern/rna_access.c

source/blender/makesrna/intern/rna_access_cache.c

T77124: Remapping `RNA_path_resolve` to other ID Blocks for FCurvesNeeds RevisionPublicActions

Details

Diff Detail

Event Timeline

Revision ContentsChangeset List

Diff 26660

source/blender/blenkernel/BKE_fcurve.h

source/blender/blenkernel/CMakeLists.txt

source/blender/blenkernel/intern/fcurve.c

source/blender/blenkernel/intern/fcurve_cache.c

source/blender/blenloader/intern/readfile.c

source/blender/depsgraph/CMakeLists.txt

source/blender/depsgraph/intern/builder/deg_builder_relations.cc

source/blender/editors/animation/anim_channels_defines.c

source/blender/editors/animation/anim_deps.c

source/blender/editors/animation/anim_draw.c

source/blender/editors/animation/anim_ipo_utils.c

source/blender/editors/animation/keyframes_general.c

source/blender/editors/animation/keyframing.c

source/blender/editors/armature/pose_slide.c

source/blender/editors/space_graph/graph_buttons.c

source/blender/makesdna/DNA_anim_types.h

source/blender/makesrna/RNA_access.h

source/blender/makesrna/intern/CMakeLists.txt

source/blender/makesrna/intern/rna_access.c

source/blender/makesrna/intern/rna_access_cache.c

T77124: Remapping `RNA_path_resolve` to other ID Blocks for FCurves
Needs RevisionPublic
Actions

Revision Contents
Changeset List