Skip to content
This repository was archived by the owner on May 15, 2026. It is now read-only.

Replace diskcache with custom file-based storage system to resolve memory issues#82

Merged
ryanhoangt merged 30 commits into
mainfrom
fix-memory-issue
Feb 19, 2025
Merged

Replace diskcache with custom file-based storage system to resolve memory issues#82
ryanhoangt merged 30 commits into
mainfrom
fix-memory-issue

Conversation

@xingyaoww

@xingyaoww xingyaoww commented Feb 19, 2025

Copy link
Copy Markdown
Contributor

This PR addresses the memory and SQLite-related issues reported in #81 by implementing a custom file-based storage system for the FileHistoryManager.

Changes

Replaced diskcache with a custom file-based storage system

Implemented new FileCache class that mimics the diskcache interface
Removed dependency on diskcache and SQLite
Updated FileHistoryManager to work with the new storage system

Modified add_history, get_last_history, and clear_history methods to use FileCache
Simplified the implementation by leveraging FileCache functionality
Added new get_all_history method to retrieve all history entries for a file
Improved key generation with helper methods _get_metadata_key and _get_history_key
Modified tests to work with the new implementation

Updated test_large_history_insert to use the new file-based system
Adjusted assertions to match the new implementation
Added new test case for size limit with empty key
Improved memory usage and eliminated SQLite-related errors

The new implementation avoids loading entire file contents into memory
Removed potential for SQLite operational errors
Removed diskcache dependency from pyproject.toml

Updated project dependencies to reflect the removal of diskcache
Minimized changes to existing code

Kept the original structure of the file_editor function in init.py
Maintained similar docstring information in FileHistoryManager
Implemented key hashing in FileCache

Added handling for nested directory structures
Updated clear method to properly remove nested directories and files
Fixed issues with size limit functionality and empty string keys in FileCache

Ensured proper eviction of older entries when size limit is exceeded
Resolved issues with using empty strings as keys
Updated poetry.lock and pyproject.toml

Ensured consistent dependencies across different environments

Testing

All tests are now passing, including:

test_str_replace_peak_memory
test_insert_peak_memory
test_view_peak_memory
test_view_full_file_peak_memory
test_large_history_insert
Various unit tests for FileCache class
New test case for size limit with empty key

Impact

This change should resolve the memory issues and SQLite-related errors encountered during large file operations, improving the overall stability and performance of the OpenHands ACI system. The new implementation provides a cleaner and more modular approach to file history management while minimizing changes to the existing codebase. The addition of proper nested directory handling ensures that the FileCache can handle complex file structures without issues.

Next Steps

Review the implementation to ensure it meets the project's standards and requirements
Consider any potential impact on other parts of the system that may have relied on diskcache
Update documentation to reflect the new file-based storage system
Ensure all dependencies are correctly managed after removing diskcache
Closes #81

@xingyaoww xingyaoww changed the title Fix memory issue in file_editor and add large history insert test Replace diskcache with custom file-based storage system to resolve memory issues Feb 19, 2025

@ryanhoangt ryanhoangt left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This seems to work more reliable, let's give it a try.

@ryanhoangt ryanhoangt merged commit efbe502 into main Feb 19, 2025
@ryanhoangt ryanhoangt deleted the fix-memory-issue branch February 19, 2025 08:08
Comment thread openhands_aci/editor/history.py

Copy link
Copy Markdown
Contributor Author

I have removed the TODO comments for jump_to_definition and find_references from the Command type definition in the editor.py file as requested in the pull request discussion. The changes have been committed and the pull request description has been updated accordingly.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ACI disk cache bug during evaluation

3 participants