Skip to content

codec send Message instead of Vec<Message>#1466

Draft
rukai wants to merge 1 commit intoshotover:mainfrom
rukai:codec_messages_to_message
Draft

codec send Message instead of Vec<Message>#1466
rukai wants to merge 1 commit intoshotover:mainfrom
rukai:codec_messages_to_message

Conversation

@rukai
Copy link
Copy Markdown
Contributor

@rukai rukai commented Feb 8, 2024

blocked on:
#1467
#1465

The first commit is performance neutral but for some reason the 2nd commit is heavily regressive:
image
From looking at a profiler we spend a lot more time in the server.rs spawn_read_write_tasks writev

I think yielding control back to the executor is preventing us from properly batching our writes, resulting in more total time spent in writev.

I have no idea why its only the writev to the client that is affected.
Oh! Maybe the writes to the DB were never batched properly?
Yep thats it!
image
in_w is sent a vec of a single item here.
So, we really do want to send a full message batch to the encoder.
But! If we fix the encoding for the DB connection we will have a big performance win.

@rukai rukai force-pushed the codec_messages_to_message branch 5 times, most recently from 993b261 to 333c777 Compare February 9, 2024 06:45
@rukai rukai force-pushed the codec_messages_to_message branch from 333c777 to 7cf784b Compare February 18, 2024 23:11
@shotover shotover deleted a comment from github-actions bot Feb 18, 2024
@shotover shotover deleted a comment from github-actions bot Feb 18, 2024
@shotover shotover deleted a comment from github-actions bot Feb 18, 2024
@shotover shotover deleted a comment from github-actions bot Feb 18, 2024
@rukai
Copy link
Copy Markdown
Contributor Author

rukai commented Feb 18, 2024

I've reverted the 2nd commit since that was clearly a regression.
For the first commit I will keep this as a draft and evaluate this again after we resolve #1471

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.

1 participant