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

Publish - avoid async state machine when possible #373

Merged
merged 2 commits into from
Feb 8, 2024

Conversation

caleblloyd
Copy link
Collaborator

Brings back optimizations from prior to #346

  • Logic to skip async state machine when possible
  • Locks switched back from channel-based to semaphore-based, but it appears they are faster
  • Selective send memory consolidation
  • Bigger send buffer sizes

Performed some benchmarks on Linux:

Send 10 Million Messages Serially:

main:

Completed: 9999701, Awaited: 299

1,412,795 msgs/sec ~ 172.46 MB/sec

This PR:

Completed: 9999980, Awaited: 20

2,688,705 msgs/sec ~ 328.21 MB/sec

So around 1.9x more throughput.

Publish Serial Microbench

main:

Method Iter Mean Error StdDev Allocated
PublishAsync 64 126.0 μs 246.1 μs 13.49 μs 360 B
PublishAsync 512 584.0 μs 2,328.1 μs 127.61 μs 333 B
PublishAsync 1024 839.2 μs 365.5 μs 20.03 μs 358 B

This PR:

Method Iter Mean Error StdDev Allocated
PublishAsync 64 127.1 μs 258.8 μs 14.19 μs 321 B
PublishAsync 512 377.8 μs 677.4 μs 37.13 μs 336 B
PublishAsync 1024 750.7 μs 1,845.1 μs 101.14 μs 339 B

Looks pretty similar

Publish Parallel Microbench

main:

Method Concurrency Mean Error StdDev Gen0 Gen1 Allocated
PublishParallelAsync 1 1.708 s 4.9311 s 0.2703 s 10000.0000 - 30.52 MB
PublishParallelAsync 2 1.486 s 1.6187 s 0.0887 s 17000.0000 2000.0000 105.66 MB
PublishParallelAsync 4 2.079 s 0.9243 s 0.0507 s 79000.0000 2000.0000 256.4 MB

This PR:

Method Concurrency Mean Error StdDev Gen0 Allocated
PublishParallelAsync 1 1.112 s 1.2659 s 0.0694 s 10000.0000 30.52 MB
PublishParallelAsync 2 1.426 s 2.9107 s 0.1595 s 61000.0000 180.45 MB
PublishParallelAsync 4 2.096 s 0.4181 s 0.0229 s 216000.0000 640.77 MB

Faster at concurrency = 1, similar timing at 2 and 4. Allocations jump in this PR quite a bit at concurrency = 4, although they don't escape Gen0. Might be worth investigating why.

@caleblloyd caleblloyd requested a review from mtmk February 4, 2024 17:01
@caleblloyd caleblloyd marked this pull request as draft February 4, 2024 17:02
@caleblloyd
Copy link
Collaborator Author

Marking as draft as there are quite a few failing tests I need to look into before it's ready for review

@mtmk
Copy link
Collaborator

mtmk commented Feb 4, 2024

Semaphore here is giving us better performance clearly. Main gain is probably not having to manage timeouts with cancellation tokens.

@caleblloyd
Copy link
Collaborator Author

Could also be seeing gains from skipping the async state machinery

@caleblloyd caleblloyd force-pushed the command-writer-avoid-state-machine branch from d005186 to 3f8cb7b Compare February 8, 2024 05:09
@caleblloyd caleblloyd marked this pull request as ready for review February 8, 2024 05:09
@caleblloyd
Copy link
Collaborator Author

Ready for review

Copy link
Collaborator

@mtmk mtmk left a comment

Choose a reason for hiding this comment

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

LGTM

@mtmk mtmk merged commit bac5173 into main Feb 8, 2024
10 checks passed
@mtmk mtmk deleted the command-writer-avoid-state-machine branch February 8, 2024 11:25
mtmk added a commit that referenced this pull request Feb 8, 2024
* Send buffer fix (#380)
* Publish - avoid async state machine when possible (#373)
* Reject payloads over the threshold set by server (#378)
* Fix NatsSvcServer start time format so that it's no longer culture aware (#374)
* Expose Headers on NatsSvcMsg (#371)
@mtmk mtmk mentioned this pull request Feb 8, 2024
mtmk added a commit that referenced this pull request Feb 8, 2024
* Send buffer fix (#380)
* Publish - avoid async state machine when possible (#373)
* Reject payloads over the threshold set by server (#378)
* Fix NatsSvcServer start time format so that it's no longer culture aware (#374)
* Expose Headers on NatsSvcMsg (#371)
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