Playback performance issue due to proto prim lookup in instanceAdapter

[csyshing]

Description of Issue

Hi,

We received a request from our production that the playback speed in Maya was slow, by doing some investigation, besides the viewport rendering time, we found that UsdImagingInstanceAdapter::_GetProtoPrim() seems to be related, although each individual call was about 0.14 ms but consider our production shot has thousands of instances, the total amount of time (within UsdImagingDelegate::SetTime()) resulted in about 38.5 ms, that caused significant performance impact during playback.

When looking into the code of UsdImagingInstanceAdapter::_GetProtoPrim(), we realized that this is a known issue and a note is there as well:
https://github.com/PixarAnimationStudios/USD/blob/faed18ce62c8736b02413635b584a2f637156bad/pxr/usdImaging/usdImaging/instanceAdapter.cpp#L2149-L2167

We implemented the necessary changes as described in the comment above internally, the time on UsdImagingInstanceAdapter::_GetProtoPrim(), dropped from 0.14 ms to ~2 us, total time on UsdImagingDelegate::SetTime() dropped from 38 ms to 2.x ms.

We have also received good feedback from production that the FPS in Maya got increased from ~10 FPS to ~17 FPS.

Before sending PR straight away, we would like to log the issue first, see if there's anything we need to be aware of?

Thanks,
Zhicheng

Steps to Reproduce

1. Open any USD scene with animation in Maya
2. Enable the tracing and do the playback

System Information (OS, Hardware)

OS: CentOS 7.8

Package Versions

USD-20.11, USD-21.8

[tcauchois]

Hi Zhicheng,

That sounds like a great PR! The only thing I wonder about is whether we already have that data cached somewhere; I noticed that we only call _GetProtoPrim if _IsChildPrim returns true, and _IsChildPrim checks if the name is "/foo.bar", meaning its instancer is "/foo", or if the name "/baz" is an instance of instancer "/foo". _GetProtoPrim doesn't check the _instanceToInstancerMap. What kind of prims were you finding that took the slow path of _GetProtoPrim? Maybe they can check that map.

If not, it's worth making a new table; it would be great to get rid of that loop you're calling out, and the performance data speaks for itself.

Thanks! Looking forward to the PR!
Tom

[jilliene]

Filed as internal issue #USD-7141

[csyshing]

Hi @tcauchois , I have submitted the PR #1767 , let me know how it looks, thanks!

[tcauchois]

Thanks! I'll take a look.

[spiffmon]

Thanks, @csyshing - before we can consider pulling in the request, we require a signed CLA. Can you send us one as described here, please?

[fabal]

Hi @spiffmon , @csyshing works with us at Animal Logic. Let me check if our CLA needs an update.

[csyshing]

Hi @tcauchois @spiffmon , our (Animal Logic) CLA should have been updated so this PR is ready for review, thanks!

[sunyab]

PR #1767 was merged and included in the 22.08 release. Closing this out, thanks!