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 #13524

Closed
wants to merge 1 commit into 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.

🛠️ PR Summary

Made with ❤️ by Ultralytics Actions

🌟 Summary

Enhanced image caching process by improving resource management with a ThreadPool context manager.

📊 Key Changes

  • Replaced manual ThreadPool handling with a with ThreadPool context manager for efficient worker thread management.
  • Simplified and refined the image caching process while preserving the existing functionality.

🎯 Purpose & Impact

  • 🛠 Improved Code Maintainability: Using the with statement ensures proper resource cleanup and reduces the potential for thread-related issues.
  • Enhanced Stability: Prevents resource leaks by ensuring threads are closed cleanly after use.
  • 😊 Better Usability: Maintains the existing behavior but ensures more robust handling, benefitting both developers and users working with large datasets.

Copy link
Contributor

github-actions bot commented Mar 5, 2025


Thank you for your submission, we really appreciate it. Like many open-source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just posting a Pull Request Comment same as the below format.


I have read the CLA Document and I sign the CLA


Long Trần seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You can retrigger this bot by commenting recheck in this Pull Request. 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 an ultralytics/yolov5 🚀 PR! Your contribution to improving memory management and thread handling for image caching is greatly appreciated! To ensure a seamless integration of your work, please review the following checklist:

  • Define a Purpose: Your PR description clearly outlines the purpose of your change, which is to enhance resource management and avoid memory leaks during dataset caching. If there are any related issues this PR addresses, please link to them for better traceability.
  • Synchronize with Source: Confirm your PR is synchronized with the ultralytics/yolov5 main branch. If it's behind, update it by clicking the 'Update branch' button or by running git pull and git merge main locally.
  • Ensure CI Checks Pass: Verify all Ultralytics Continuous Integration (CI) checks are passing. If any checks fail, please address them to maintain compatibility with the codebase.
  • Update Documentation: If relevant, update the documentation to reflect the improved dataset caching mechanism you introduced.
  • Add Tests: Include or update tests to ensure that your changes are fully covered and verified for stability. This is especially crucial for functionality changes involving dataset loading and caching.
  • Sign the CLA: Please ensure you have signed our Contributor License Agreement if this is your first Ultralytics PR by writing "I have read the CLA Document and I sign the CLA" in a new message.
  • Minimize Changes: Based on your diff, the changes seem concise and precisely targeted at resolving the issue. Excellent work keeping the modifications minimal and focused. "Simplicity is the ultimate sophistication." — Leonardo da Vinci

If you haven't already, please share a minimum reproducible example (MRE) that demonstrates the memory leak issue under the current implementation. Providing an MRE will help ensure the improvement's impact is thoroughly validated.

For more guidance, please refer to our Contributing Guide. An Ultralytics engineer will review your PR soon to provide further feedback. Thank you for contributing to Ultralytics! 🚀✨

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