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

Value Clips memory usage grows with the square of the number of "times" entries #1774

Closed
marktucker opened this issue Feb 18, 2022 · 6 comments

Comments

@marktucker
Copy link
Contributor

Description of Issue

If you create a value clip with 15000 "times" entries, the Usd_Clip data structures consume about 5GB of data, regardless of the size of the data in the actual clip files.

This memory if consumed by the fact that each Usd_Clip object makes a copy of the 15000-entry timeMapping array created in the Usd_ClipSet constructor. I think each Usd_Clip applies the same sorting procedure to the data, and makes it's own copy. Seems like the obvious solution would be to have the Usd_ClipSet constructor massage the data, then pass a shared pointer to each Usd_Clip constructor. I'd be happy to make a PR for this if you think this is the correct approach to fixing this.

Steps to Reproduce

  1. Unzip the attached file:
    value_clip_memory.zip
  2. Load the box_loop.usd file into usdview. Memory usage will be 5-6GB, even though this is just a box flipping back and forth between two positions.

Package Versions

Tested in 20.08, 21.08, and 21.11

@jilliene
Copy link

Filed as internal issue #USD-7222

@spiffmon
Copy link
Member

Hey @marktucker - oof, we've never exercised the explicit form to that scale; good find! Your approach seems to us like the right thing, and we'd be happy to get a PR.

@LucaScheller
Copy link

Hey @jilliene / @spiffmon,
hope you are doing well!

We (at RISE | Visual Effects Studios) were wondering what the current state of the attached PR from Mark is? Has it been reviewed/are there plans to merge it soon (#1777)? Thanks again Mark for submitting the PR :)

We've been running into this issue quite often for heavy FX scenes, the fix would help us quite a bit if it gets released soon :)

Cheers,
Luca

@spiffmon
Copy link
Member

Hi @LucaScheller , although it hasn't been pushed out to gitHub yet, I believe we merged the PR just last week, so it will be in the dev branch soon, and in the next release, tentatively 22.08

@LucaScheller
Copy link

Thanks for the update, that's great to hear :)

@sunyab
Copy link
Contributor

sunyab commented Jul 22, 2022

PR #1777 was merged and released in 22.08, closing this out. Thanks!

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

5 participants