Skip to content
New issue

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

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

Already on GitHub? # to your account

Optimize performance of large assets #84

Merged
merged 23 commits into from
Jul 21, 2022
Merged

Optimize performance of large assets #84

merged 23 commits into from
Jul 21, 2022

Conversation

mauricefisher64
Copy link
Collaborator

@mauricefisher64 mauricefisher64 commented Jul 20, 2022

Changes in this pull request

This is a series of optimizations that primarily benefit large assets;

  • Interleaved IO for Sha2 and Blake3 hashing
  • patch API for asset handlers to directly insert manifest without needing to copy files
  • remove unnecessary file copies
  • no more memory buffers for large assets

Checklist

  • This PR represents a single feature, fix, or change.
  • All applicable changes have been documented.
  • Any TO DO items (or similar) have been entered as GitHub issues and the link to that issue has been included in a comment.

@codecov-commenter
Copy link

codecov-commenter commented Jul 20, 2022

Codecov Report

Merging #84 (db0fbd8) into main (26f9ed1) will decrease coverage by 0.48%.
The diff coverage is 49.26%.

@@            Coverage Diff             @@
##             main      #84      +/-   ##
==========================================
- Coverage   74.44%   73.96%   -0.49%     
==========================================
  Files          66       66              
  Lines       13565    13745     +180     
==========================================
+ Hits        10099    10167      +68     
- Misses       3466     3578     +112     
Impacted Files Coverage Δ
sdk/src/error.rs 14.28% <ø> (ø)
sdk/src/utils/hash_utils.rs 45.91% <15.30%> (-18.20%) ⬇️
sdk/src/jumbf_io.rs 84.44% <70.00%> (-2.47%) ⬇️
sdk/src/asset_handlers/bmff_io.rs 75.27% <80.89%> (+0.62%) ⬆️
sdk/src/asset_io.rs 50.00% <100.00%> (+30.00%) ⬆️
sdk/src/ingredient.rs 89.00% <100.00%> (-0.02%) ⬇️
sdk/src/manifest.rs 89.65% <100.00%> (ø)
make_test_images/src/make_test_images.rs 67.50% <0.00%> (-0.42%) ⬇️
sdk/src/store.rs 76.57% <0.00%> (-0.39%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 26f9ed1...db0fbd8. Read the comment docs.

Copy link
Collaborator

@gpeacock gpeacock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments and one requested change for manifest.embed()

@mauricefisher64 mauricefisher64 requested a review from gpeacock July 21, 2022 11:48
@mauricefisher64 mauricefisher64 merged commit 29bb633 into main Jul 21, 2022
@mauricefisher64 mauricefisher64 deleted the optimize branch July 21, 2022 16:18
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants