Animation: Disable NLA mapping if no NLA tracks are present #110306

Christoph Lendenfeld · 2023-07-20T16:25:32+02:00

2023-07-20 16:25:32 +02:00

The drawing code executed NLA mapping code even though there were no tracks to be mapped.

Disabling this takes the draw_fcurve function from ~1130μs to ~985μs (heavy example scene with dense data)
Of course this only applies when not using the NLA.
And the performance benefit is larger, the more curves are on screen

The drawing code executed NLA mapping code even though there were no tracks to be mapped. Disabling this takes the `draw_fcurve` function from ~1130μs to ~985μs (heavy example scene with dense data) Of course this only applies when not using the NLA. And the performance benefit is larger, the more curves are on screen

Christoph Lendenfeld added 1 commit 2023-07-20 16:25:41 +02:00

fb01fdd2be nla mapping optional

Christoph Lendenfeld requested review from Nate Rupsis 2023-07-20 16:38:25 +02:00

Christoph Lendenfeld added the

Module

Animation & Rigging

label 2023-07-20 16:38:33 +02:00

Christoph Lendenfeld requested review from Sybren A. Stüvel 2023-07-20 16:39:33 +02:00

Christoph Lendenfeld added this to the Animation & Rigging project 2023-07-20 16:39:40 +02:00

Nate Rupsis commented

2023-07-20 20:13:20 +02:00

Curious to know how you profiled.

Nate Rupsis approved these changes 2023-07-20 20:14:28 +02:00

Nate Rupsis left a comment

What a nice little improvement :)

Christoph Lendenfeld commented

2023-07-21 09:42:34 +02:00

Curious to know how you profiled.

to find the slowdowns I used Hotspot on linux
to get actual performance times I used simple delta time prints using std::chrono::high_resolution_clock::now()

> Curious to know how you profiled. to find the slowdowns I used Hotspot on linux to get actual performance times I used simple delta time prints using `std::chrono::high_resolution_clock::now()`

Sybren A. Stüvel reviewed 2023-07-21 10:39:50 +02:00

Sybren A. Stüvel left a comment

Now I'm curious. What are the timings you get when you move the check into ANIM_nla_mapping_apply_fcurve()? The current optimisation creates a stronger binding between the caller and that function's implementation, which IMO is not the best way forward. My hypothesis is that putting this at the start of the function would be enough to get the speedup. At least it should get rid of the cost per displayed FCurve.

void ANIM_nla_mapping_apply_fcurve(AnimData *adt, FCurve *fcu, bool restore, bool only_keys)
{
  /* Don't bother iterating all FCurves if this function is not going to do anything anyway. */
  if (adt == nullptr || BLI_listbase_is_empty(&adt->nla_tracks)) {
    return;
  }
  // ... rest of the function 
}

Now I'm curious. What are the timings you get when you move the check into `ANIM_nla_mapping_apply_fcurve()`? The current optimisation creates a stronger binding between the caller and that function's implementation, which IMO is not the best way forward. My hypothesis is that putting this at the start of the function would be enough to get the speedup. At least it should get rid of the cost per displayed FCurve. ```cpp void ANIM_nla_mapping_apply_fcurve(AnimData *adt, FCurve *fcu, bool restore, bool only_keys) { /* Don't bother iterating all FCurves if this function is not going to do anything anyway. */ if (adt == nullptr || BLI_listbase_is_empty(&adt->nla_tracks)) { return; } // ... rest of the function } ```

Christoph Lendenfeld commented

2023-07-21 16:04:40 +02:00

@dr.sybren very good idea! shaved off another 70μs per curve
There were 2 more calls to that function below that I missed, having the if within the function itself of course also included them

@dr.sybren very good idea! shaved off another 70μs per curve There were 2 more calls to that function below that I missed, having the if within the function itself of course also included them

🚀 1

Christoph Lendenfeld added 1 commit 2023-07-21 16:06:19 +02:00

e1efa80608 move check for nla tracks into mapping function

Sybren A. Stüvel approved these changes 2023-07-24 11:42:07 +02:00

Christoph Lendenfeld added 2 commits 2023-07-27 09:27:17 +02:00

5b3a997462 Merge branch 'main' into performance_nla_mapping

360cc27911 resolve merge issues

Christoph Lendenfeld referenced this issue from a commit

2023-07-27 09:31:50 +02:00

Animation: Disable NLA mapping if no NLA tracks are present

Christoph Lendenfeld merged commit f7570c5ef4 into main

2023-07-27 09:31:50 +02:00