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

Release convolution weightsMat after usage #25181

Merged
merged 4 commits into from
Mar 25, 2024

Conversation

dkurt
Copy link
Member

@dkurt dkurt commented Mar 7, 2024

Pull Request Readiness Checklist

related (but not resolved): #24134

Minor memory footprint improvement. Also, adds a test for VmHWM.

RAM top memory usage (-230MB)

YOLOv3 (237MB file) 4.x PR
no winograd 808 MB 581 MB
winograd 1985 MB 1750 MB

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

  • I agree to contribute to the project under Apache 2 License.
  • To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
  • The PR is proposed to the proper branch
  • There is a reference to the original bug report and related work
  • There is accuracy test, performance test and test data in opencv_extra repository, if applicable
    Patch to opencv_extra has the same branch name.
  • The feature is well documented and sample code can be built with the project CMake

@dkurt dkurt force-pushed the release_conv_weights branch 2 times, most recently from d92d6e9 to c531352 Compare March 7, 2024 18:26
@opencv-alalek
Copy link
Contributor

opencv-alalek commented Mar 7, 2024

BTW,
DNN redesign is necessary to resolve problem with storing of stalled unused buffers.

We need 2 entities of DL model at least:

  • "imported model"
  • "compiled model" for the specific target
  • imported model could be released with its buffers after compilation (if user won't to change the target device).
  • user infers using compiled model only.

@dkurt
Copy link
Member Author

dkurt commented Mar 7, 2024

@opencv-alalek, agree. For now something intermediate is used - we clean buffers during the import trying not to store two instances of the weights (protobuf does not let more memory ownership).

If you mean current OpenVINO workflow with model/compiled_model - they have an advantage with offline IR converter. But yes, after the compilation (CompiledModel) a model reference (Model) can be removed.

Not sure if we need introduce two instances in public API, maybe add two private implementations: LayerProto (with original weights, shapes logic, backends initialization) and LayerOCV, which is a separate "default backend" implementation. And need an agreement or option to let us clean original weights (means after OpenCV forward user won't set a different backend).

@asmorkalov asmorkalov added this to the 4.10.0 milestone Mar 8, 2024
@fengyuentau
Copy link
Member

Gemm, MatMul and Attention layers also have this problem as constant weight needs to be prepacked and stored while leaving the original one unused.

@asmorkalov asmorkalov merged commit 0b6c9a2 into opencv:4.x Mar 25, 2024
27 checks passed
@dkurt dkurt deleted the release_conv_weights branch March 25, 2024 06:16
@asmorkalov asmorkalov mentioned this pull request Apr 1, 2024
klatism pushed a commit to klatism/opencv that referenced this pull request May 17, 2024
Release convolution weightsMat after usage opencv#25181

### Pull Request Readiness Checklist

related (but not resolved): opencv#24134

Minor memory footprint improvement. Also, adds a test for VmHWM.

RAM top memory usage (-230MB)

| YOLOv3 (237MB file) |   4.x   |    PR   |
|---------------------|---------|---------|
| no winograd         | 808 MB  | 581 MB  |
| winograd            | 1985 MB | 1750 MB |

See details at https://github.com/opencv/opencv/wiki/How_to_contribute#making-a-good-pull-request

- [x] I agree to contribute to the project under Apache 2 License.
- [x] To the best of my knowledge, the proposed patch is not based on a code under GPL or another license that is incompatible with OpenCV
- [x] The PR is proposed to the proper branch
- [x] There is a reference to the original bug report and related work
- [x] There is accuracy test, performance test and test data in opencv_extra repository, if applicable
      Patch to opencv_extra has the same branch name.
- [x] The feature is well documented and sample code can be built with the project CMake
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants