Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Performance issue for UsdImagingDelegate::SetTime / UsdImagingDelegate::ApplyPendingUpdates #1813

Closed
csyshing opened this issue Mar 24, 2022 · 4 comments

Comments

@csyshing
Copy link

Description of Issue

Hi,

I was working on a performance issue for production and found that UsdImagingDelegate::SetTime() takes a bit longer than expected to run for the first time when user interact the prim, for example, the first time moving a prim in Maya viewport.

The USD trace shows that UsdImagingDelegate::_GatherDependencies() takes about 1.x~10ms to finish, in our production shot, this method is being called around 12k times, which resulted in 1.2 minutes in total (first image below: USD-20.x) / 39 second (second image below: USD-22.03):

USD-20 x_usdImaging_delegate_gather_dependencies_current

USD-22 03_usdImaging_delegate_gather_dependencies

We have tried to make improvement to speed up UsdImagingDelegate::SetTime() call:

  • Replace the result container type to be SdfPathSet in _GatherDependencies()
    • The existing code logic copies paths, then sorts all items, then removes duplicates, looks like using a SdfPathSet would be more efficient:
    // Propose changes below:
    SdfPathSet affectedPaths;
    for (_DependencyMap::const_iterator it = start; it != end; ++it) {
        affectedPaths.emplace(it->second);
    }
    affectedCachePaths.reserve(affectedCachePaths.size() + affectedPaths.size());
    affectedCachePaths.insert(affectedCachePaths.end(),
                              affectedPaths.begin(), affectedPaths.end());
  • Pre-cache path dependencies in parallel and calling _GatherDependencies() later would be much faster: this brings in a significant speed up and the total time drops to 10.x second:

usdImaging_delegate_gather_dependencies_in_parallel

As a comparison, here is the breakdown matrix based on one of our production shots with 12k prim paths to run:

_GatherDependencies (called from _ResyncUsdPrim and _RefreshUsdObject) _ProcessRemoval _Populate _ExecuteWorkForVariabilityUpdate SetTime (Total)
USD 20.x ~62 sec ~3.x sec ~5.5 sec ~0.5 sec 1.2 min
USD 22.03 ~30 sec ~3.x sec ~5.5 sec ~0.5 sec 39.x sec
Run in Parallel ~1.x sec ~3.x sec ~5.5 sec ~0.5 sec 10.x sec

We are happy to submit the changes for review, but I am creating this issue to share the idea of the improvement, in case there is anything I missed.

Thanks,
Zhicheng

Steps to Reproduce

  1. Load any USD scene
  2. Enable the tracing and observe the log

System Information (OS, Hardware)

OS: CentOS 7.8

Package Versions

Tested in USD-20.11, USD-22.03.

@jilliene
Copy link

Filed as internal issue #USD-7290

@tcauchois
Copy link
Contributor

Hey Zhicheng, do you know if the SdfPathSet change by itself sped things up? We've found that sets aren't always faster than sorted arrays, but if it's faster in this case it seems like a good change.

If your CLA is all set it would be great to see a PR for this work.

Thanks!
Tom

@csyshing
Copy link
Author

Hi @tcauchois , I have submitted the PR here: #1815

I have a few tests against our production shot, the SdfPathSet change has a very tiny difference comparing with using SdfPathVector, I don't think our production shot is a good example for benchmarking this change, most of the usd path <--> cache path are 1 to 1 mapping so it's not as obvious as running in parallel v.s. running sequentially. I did not include the SdfPathSet change in my PR, I guess we will need to revisit that at another time. Thanks!

@sunyab
Copy link
Contributor

sunyab commented Nov 4, 2022

Closing this out, PR #1815 was merged and released with v22.11. Thanks!

@sunyab sunyab closed this as completed Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants