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

Fix potential memory leak for cache ram #13525

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

longtran1904
Copy link

@longtran1904 longtran1904 commented Mar 5, 2025

I encountered a memory leak when customizing Yolov5 cache ram. The plan was to allowing caching partial dataset on RAM if there's not enough memory for the whole dataset.

The change ensures the ThreadPool used for caching data is closed when it's done their duty.
This improves code extensibility, ensures threads are closed when they finished their tasks.

I have read the CLA Document and I sign the CLA

🛠️ PR Summary

Made with ❤️ by Ultralytics Actions

🌟 Summary

Enhanced the image caching process for improved efficiency and resource management during dataset loading. 🛠️💾

📊 Key Changes

  • Replaced manual ThreadPool instantiation and usage with a context manager (with ThreadPool), ensuring cleaner and safer thread lifecycle management. 🔄
  • Retained the core functionality for image caching to RAM or disk, but made the implementation more robust.

🎯 Purpose & Impact

  • Simpler and Safer Code: Using a with context ensures threads are properly closed, reducing the chance of resource leaks or crashes. 🙌
  • Improved Stability: Provides better memory and thread management, especially for large datasets during training or inference. 🚀
  • User Benefit: Ensures smoother and more reliable caching, leading to fewer interruptions or unexpected behavior during intensive tasks. ⚡

Copy link
Contributor

github-actions bot commented Mar 5, 2025

All Contributors have signed the CLA. ✅
Posted by the CLA Assistant Lite bot.

@UltralyticsAssistant UltralyticsAssistant added bug Something isn't working enhancement New feature or request labels Mar 5, 2025
@UltralyticsAssistant
Copy link
Member

👋 Hello @longtran1904, thank you for submitting a ultralytics/yolov5 🚀 Pull Request! To ensure a seamless integration of your work, please review the following checklist:

  • Define a Purpose: You've made great strides in enhancing the caching mechanism! Please ensure your PR description clearly explains the purpose of your changes and how they address the issue (e.g., memory leak and thread management). If this relates to any relevant issues, please link them.
  • Include a Minimum Reproducible Example (MRE): If applicable, please attach an MRE demonstrating the specific issue of the memory leak to validate and reproduce the bug and confirm this PR resolves it effectively.
  • Synchronize with Source: Confirm your PR is up-to-date with the ultralytics/yolov5 main branch. If needed, update it via the 'Update branch' button or locally using git pull and git merge main.
  • Ensure CI Checks Pass: Verify all Ultralytics Continuous Integration (CI) checks are passing. Let us know if you encounter any issues so we can assist you. 🔍
  • Update Documentation: Ensure that the relevant documentation is updated to reflect these changes if they impact end-user functionality.
  • Add Tests: Adding or updating tests to verify these changes would help ensure future stability. Please confirm that all tests are passing locally.
  • Sign the CLA: Confirm you've signed our Contributor License Agreement (CLA) by writing "I have read the CLA Document and I sign the CLA" as a comment in this PR (thank you for already doing this!). ✅
  • Minimize Changes: Focus changes on addressing the identified memory leak, limiting any unrelated edits for a more concise and impactful PR.

For more guidance, please see our Contributing Guide. Your contribution, which improves resource management for caching, is highly appreciated! 🎉

This is an automated response 🤖, but an Ultralytics engineer will review your PR and provide feedback soon. If you have any questions, feel free to comment below. Thank you again for contributing to Ultralytics! 🚀

@longtran1904
Copy link
Author

I have read the CLA Document and I sign the CLA

@longtran1904 longtran1904 marked this pull request as ready for review March 5, 2025 13:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants