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

refactor: Optimize memory fetching by moving sorting and slicing to DB #1531

Open
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

nicky-ru
Copy link
Contributor

@nicky-ru nicky-ru commented Dec 28, 2024

Relates to:

No


Risks

Low

  • Adjustments to database queries improve performance by moving sorting and limiting logic to the database side.
  • Even though this change adds improvements to the Postgres adapter and required a change in the global IDatabaseAdapter, other database adapters are still working because the limit parameter is optional.
  • Other plugins relying on the getMemoriesByRoomIds function can opt into the limit functionality, but it is not necessary.
  • No breaking changes introduced.

Background

What does this PR do?

This PR improves the performance of memory-fetching functionality by:

  • Moving sorting and slicing logic from the application layer to the database layer.
  • Adding an optional limit parameter to restrict the number of results fetched from the database.
  • Adjusting the runtime logic to leverage the new database query functionality.

Why was this change made?

In our database with 60,000 memories, fetching memories by room IDs took 40 seconds, significantly affecting the performance of the agent. After applying this fix, the query execution time dropped to just 0.02 seconds. This improvement was observed using Supabase analytics.


What kind of change is this?

Improvements

  • Changes to existing functionality to improve database query performance.

Documentation changes needed?

My changes do not require a change to the project documentation.


Testing

Where should a reviewer start?

  • Review the changes in packages/adapter-postgres/src/index.ts to understand how sorting and limiting were implemented in the database query.
  • Look at the adjusted runtime logic in packages/core/src/runtime.ts for the integration of the limit parameter.

Detailed testing steps

  1. Ensure all existing tests for memory fetching pass.
  2. Manually test the memory-fetching feature by:
    • Querying with and without the limit parameter.
    • Verifying that the sorting order (createdAt DESC) is maintained.
  3. Monitor query performance with larger datasets to confirm improvements.

Database changes

  • Updated query logic to include ORDER BY createdAt DESC and optional LIMIT in the PostgreSQL adapter.

Discord username

nikita_zhou


Screenshots

I will attach screenshots of:

  • Successful working run with Postgres and SQLite.
  • Successful linting results.

Using Postgres adapter

Screenshot 2024-12-28 at 7 13 52 PM Screenshot 2024-12-28 at 7 15 14 PM

Using SQLite adapter

Screenshot 2024-12-28 at 7 16 57 PM Screenshot 2024-12-28 at 7 18 52 PM

Linting

Screenshot 2024-12-28 at 7 28 45 PM Screenshot 2024-12-28 at 7 29 47 PM

@nicky-ru nicky-ru changed the title Performance: Improve memory fetching performance by moving sorting and slicing to db side Performance: Optimize memory fetching by moving sorting and slicing to DB Dec 28, 2024
Copy link
Collaborator

@odilitime odilitime left a comment

Choose a reason for hiding this comment

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

These changes look fine but the question is there any other part of the system that also used them, we need to check

@odilitime odilitime changed the title Performance: Optimize memory fetching by moving sorting and slicing to DB refactor: Optimize memory fetching by moving sorting and slicing to DB Dec 28, 2024
@nicky-ru
Copy link
Contributor Author

These changes look fine but the question is there any other part of the system that also used them, we need to check

as of my knowledge it's also used in twitter client. I can try later this week if it will work with twitter client enabled as well

@nicky-ru
Copy link
Contributor Author

nicky-ru commented Jan 2, 2025

@odilitime works with twitter on 0.1.7-alpha.1 just fine https://x.com/Bino_io

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants