Cache usdz archive traversal for subsequent lookup/queries on external references. #1578
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
marsupial
force-pushed
the
PR/1577-usdz-cache-linear-traversal
branch
from
8ef1053
to
2d30f0e
Compare
|
Filed as internal issue #USD-6817 |
|
Just wanted to mention I'm looking at merging this PR in now. Thanks for your patience! |
|
Thanks for your patience. I ended up reworking some of these changes and separating the fix for #1577 and #1579 into different commits. I also pulled the performance test into an internal test suite instead of committing them, as I'm hesitant to add a semi-large-ish file to the repo. We could certainly revisit that if necessary. I'm going to close this PR out in favor of those separate commits, which will land with the next dev push. Thanks again! |
pixar-oss
pushed a commit
that referenced
this pull request
Dec 18, 2021
…l references. The idea for this change and the initial implementation were contributed by @marsupial in PR #1578. I did some further simplifications, cleanup to conform to our standard style, and added a C++ unit test to exercise iterator behavior. From the original submission: While linearly traversing the **usdz** archive, cache the path and header information so that subsequent lookups can be accelerated. This brings down load time of a **usdz** file that was taking ~3 minutes to ~16 seconds, and can often wind up performing better than parsing the original **usdc** file that generated the **usdz** (likely because IO is avoided on the references as the files are _in memory_ from the archive). Fixes #1577 (Internal change: 2205950)
pixar-oss
pushed a commit
that referenced
this pull request
Dec 18, 2021
where default-constructed structs may incorrectly be detected as valid if their signature field happened to be initialized with a value that matched the expected signature. This initial fix for this issue was provided by @marsupial in PR #1578. Fixes #1579 (Internal change: 2206828)
lkerley
pushed a commit
to imageworks/USD
that referenced
this pull request
Jan 7, 2022
…l references. The idea for this change and the initial implementation were contributed by @marsupial in PR PixarAnimationStudios#1578. I did some further simplifications, cleanup to conform to our standard style, and added a C++ unit test to exercise iterator behavior. From the original submission: While linearly traversing the **usdz** archive, cache the path and header information so that subsequent lookups can be accelerated. This brings down load time of a **usdz** file that was taking ~3 minutes to ~16 seconds, and can often wind up performing better than parsing the original **usdc** file that generated the **usdz** (likely because IO is avoided on the references as the files are _in memory_ from the archive). Fixes PixarAnimationStudios#1577 (Internal change: 2205950)
lkerley
pushed a commit
to imageworks/USD
that referenced
this pull request
Jan 7, 2022
where default-constructed structs may incorrectly be detected as valid if their signature field happened to be initialized with a value that matched the expected signature. This initial fix for this issue was provided by @marsupial in PR PixarAnimationStudios#1578. Fixes PixarAnimationStudios#1579 (Internal change: 2206828)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Change(s)
While linearly traversing the usdz archive, cache the path and header information so that subsequent lookups can be accelerated.
Fixes Issue(s)
#1577, #1579
This brings down load time of a usdz file that was taking ~3 minutes to ~16 seconds, and can often wind up performing better than parsing the original usdc file that generated the usdz (likely because IO is avoided on the references as the files are in memory from the archive).
The test case is an example that takes more than 15 seconds to load currently, but with caching goes down to 2 seconds. It has 25000 entries, chosen to demonstrate the time difference in a way that can be tested against regression. The amount of entries is also good to test against any race conditions filling in the cache (though per the comments in code, a relatively simplistic filling is used to as it performs well enough and keeps the code simple)