Memory optimisation #8382
Memory optimisation #8382
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe changes involve refactoring the dataset manager bindings in Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- cvat/apps/dataset_manager/bindings.py (4 hunks)
Additional comments not posted (3)
cvat/apps/dataset_manager/bindings.py (3)
540-540: Refactor: Centralize point conversion logic.The addition of
_ensure_points_converted_to_floatsmethod centralizes the conversion of points to floats, which is a significant improvement in terms of maintainability and readability. This method checks if the points are already in a suitable format before performing the conversion, which helps in optimizing memory usage.
- The method is correctly placed within the
CommonDataclass, which seems to handle various common functionalities related to data handling.- The method is called within
_import_shapeand_import_trackmethods to ensure all points are converted before further processing.The changes are well-implemented and align with the PR's objective of memory optimization.
Also applies to: 566-566, 570-586
540-540: Ensure consistent data processing in_import_shape.The inclusion of
_ensure_points_converted_to_floatsin_import_shapeensures that all points are converted to floats before any further processing. This change is crucial for maintaining consistency and accuracy in data handling.The modification is correctly implemented and contributes to the overall goal of memory optimization.
566-566: Ensure consistent data processing in_import_track.The inclusion of
_ensure_points_converted_to_floatsin_import_trackensures that all points are converted to floats before any further processing. This change is crucial for maintaining consistency and accuracy in data handling.The modification is correctly implemented and contributes to the overall goal of memory optimization.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #8382 +/- ##
===========================================
+ Coverage 83.46% 83.56% +0.10%
===========================================
Files 395 396 +1
Lines 41826 41891 +65
Branches 3881 3881
===========================================
+ Hits 34909 35008 +99
+ Misses 6917 6883 -34
|
|
<!-- Raise an issue to propose your change (https://github.com/cvat-ai/cvat/issues). It helps to avoid duplication of efforts from multiple independent contributors. Discuss your ideas with maintainers to be sure that changes will be approved and merged. Read the [Contribution guide](https://docs.cvat.ai/docs/contributing/). --> <!-- Provide a general summary of your changes in the Title above --> ### Motivation and context By using tuple as a container for points when dealing with import from datumaro, we can achieve 2 things: - Reduce memory needed for copying shapes and tracks during import (running `deepcopy` on `tuple[int]` will return the same object, as opposed to `list[int]`) - Guarantee type safety during later stages of data pipeline and skip additional conversion added in cvat-ai#1898 Same thing arguable should be done for CVAT format as well. Benchmarks: [memray_reports.zip](https://github.com/user-attachments/files/16849509/memray_reports.zip) ### How has this been tested? <!-- Please describe in detail how you tested your changes. Include details of your testing environment, and the tests you ran to see how your change affects other areas of the code, etc. --> ### Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply. If an item isn't applicable for some reason, then ~~explicitly strikethrough~~ the whole line. If you don't do that, GitHub will show incorrect progress for the pull request. If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [x] I submit my changes into the `develop` branch - [ ] I have created a changelog fragment <!-- see top comment in CHANGELOG.md --> - [ ] I have updated the documentation accordingly - [ ] I have added tests to cover my changes - [ ] I have linked related issues (see [GitHub docs]( https://help.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword)) - [ ] I have increased versions of npm packages if it is necessary ([cvat-canvas](https://github.com/cvat-ai/cvat/tree/develop/cvat-canvas#versioning), [cvat-core](https://github.com/cvat-ai/cvat/tree/develop/cvat-core#versioning), [cvat-data](https://github.com/cvat-ai/cvat/tree/develop/cvat-data#versioning) and [cvat-ui](https://github.com/cvat-ai/cvat/tree/develop/cvat-ui#versioning)) ### License - [x] I submit _my code changes_ under the same [MIT License]( https://github.com/cvat-ai/cvat/blob/develop/LICENSE) that covers the project. Feel free to contact the maintainers if that's a concern. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Improved handling of shape points during the import process for enhanced data accuracy. - Centralized conversion of shape points to floats, optimizing memory usage and performance. - **Refactor** - Enhanced code readability and maintainability by restructuring the point conversion logic. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Motivation and context
By using tuple as a container for points when dealing with import from datumaro, we can achieve 2 things:
deepcopyontuple[int]will return the same object, as opposed tolist[int])Same thing arguable should be done for CVAT format as well.
Benchmarks: memray_reports.zip
How has this been tested?
Checklist
developbranch(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit
New Features
Refactor