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

Flamegraph Analysis #447

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

Flamegraph Analysis #447

wants to merge 4 commits into from

Conversation

ReagentX
Copy link
Owner

@ReagentX ReagentX commented Jan 25, 2025

Changes:

  • Attachment::from_row()
    • Double attachment query performance (+3% overall)
  • Message::get_replies()
    • No changes needed
  • TypedStreamReader::parse()
  • Message::from_row()
    • Set SQLITE_OPEN_NOMUTEX (+40% overall)

Original flamegraph (calls in pink were removed or improved):

image

Final flamegraph:

image

@ReagentX ReagentX added crate: database Related to the database crate enhancement Requires updating an existing feature labels Jan 25, 2025
@ReagentX ReagentX self-assigned this Jan 25, 2025
@ReagentX
Copy link
Owner Author

ReagentX commented Jan 25, 2025

Attachment::from_row() can benefit from selecting specific columns, falling back to using * if the column names are not present in the provided table.

Before:

image

After:

image

@ReagentX
Copy link
Owner Author

Message::get_replies() is already quite fast on a per-call basis: Status: OK. Duration: 0.000900s. Likely, we just make this query a lot because there are a lot of threaded messages.

@ReagentX
Copy link
Owner Author

ReagentX commented Jan 25, 2025

Message::from_row() performs a lot of allocations, but it is designed to own all of the data from a row, and there are a lot of rows to go through. The query is fast and we already use a cursor to lazily evaluate it.

However, looking at the call stack closely, we can see that we are using some very slow mutex operations on the SQLite side:

image

This can be resolved by setting SQLITE_OPEN_NOMUTEX, which leads to a massive 40% speedup:

image

@ReagentX
Copy link
Owner Author

ReagentX commented Jan 25, 2025

Removing calls to TypedStreamReader::parse() leads to about a 30% speedup, but loses a ton of important data stored in the TypedStream blobs. Most of the time is spent in read_types(), which performs a lot of allocations and copies that can likely be optimized with some careful lifetime management.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate: database Related to the database crate enhancement Requires updating an existing feature
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

1 participant