Skip to content

Conversation

juntae6942
Copy link

@juntae6942 juntae6942 commented Aug 7, 2025

Title:
Refactor(memory): Improve logic and efficiency of Chat Memory

Body:
Closes: #3945

This pull request addresses the performance overhead in MessageWindowChatMemory by refactoring its internal logic and abstracting the storage layer. The previous implementation replaced the entire message list on every update, which is inefficient for long-running conversations.

Key Changes
Introduced ChatMemoryRepository: A new interface is introduced to abstract the storage mechanism, decoupling the chat memory logic from the in-memory implementation. This design allows for future persistent implementations (e.g., Redis, JDBC).

Efficient InMemoryChatMemoryRepository: The default InMemoryChatMemoryRepository now uses an efficient refresh method to apply only deltas (additions/deletions) instead of replacing the entire list, significantly reducing overhead.

Updated MessageWindowChatMemory: The class is refactored to use the new ChatMemoryRepository, delegating all state mutations to the repository layer.

Updated Tests: Comprehensive unit tests have been added and updated to ensure the reliability and correctness of the new implementation.

@juntae6942 juntae6942 force-pushed the fix/chat-memory-efficiency-3945 branch from 6becb80 to 6bf4f28 Compare August 7, 2025 17:00
@YunKuiLu
Copy link
Contributor

YunKuiLu commented Aug 8, 2025

This might need some discussion. Some NoSQL databases (like MongoDB?) only support full message overwrite and do not support incremental updates or deletions.

And after ChatMemoryRepository added new methods, spring-ai-model-chat-memory-repository-jdbc, spring-ai-model-chat-memory-repository-cassandra, spring-ai-model-chat-memory-repository-neo4j should all have corresponding updates and integration tests.

@juntae6942
Copy link
Author

@YunKuiLu Thank you for the insightful feedback! Both points you raised are important, and I had them in mind as well.

Regarding the concern for NoSQL databases that only support full overwrites, that is a very valid point. My intention with this PR was to first establish the foundational, more efficient repository pattern with the new interface and the default in-memory implementation. For specific database implementations that don't support incremental updates, we can ensure they rely on a saveAll-style full overwrite to maintain compatibility, while other databases can leverage the benefits of the new refresh method.

I was also fully aware of the need to update the other ChatMemoryRepository implementations (jdbc, cassandra, neo4j). My plan was to first get this core refactoring of the interface and the base implementation merged.

I believed that separating the changes this way would keep the pull requests smaller and more focused, making them easier to review and discuss.

If this approach sounds reasonable to you, I plan to submit follow-up PRs for the other modules once this foundational PR is merged.

Thank you again for the thoughtful review.

Signed-off-by: potato <65760583+juntae6942@users.noreply.github.com>
Signed-off-by: potato <65760583+juntae6942@users.noreply.github.com>
Signed-off-by: potato <65760583+juntae6942@users.noreply.github.com>
Signed-off-by: potato <65760583+juntae6942@users.noreply.github.com>
Signed-off-by: potato <65760583+juntae6942@users.noreply.github.com>
Signed-off-by: potato <65760583+juntae6942@users.noreply.github.com>
Signed-off-by: potato <65760583+juntae6942@users.noreply.github.com>
Signed-off-by: potato <65760583+juntae6942@users.noreply.github.com>
@juntae6942 juntae6942 force-pushed the fix/chat-memory-efficiency-3945 branch from 5d876c3 to 5d882ae Compare September 5, 2025 03:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants