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

fix: remove abortable iterator from muxer #63

Merged
merged 2 commits into from
Nov 12, 2023

Conversation

achingbrain
Copy link
Collaborator

@achingbrain achingbrain commented Nov 11, 2023

We use an abortable source for every connection - this introduces a lot of latency.

This PR removes abortable iterator to just listen on the abort event on the shutdown controller's abort signal and cause the connection's iterator to return.

This is the same thing that abortable source does except we don't Promise.race every .next() from the source.

I've added a simple benchmark - in my testing it increases throughput by 30% or so, this seems to translate to 10-15% in "real world" performance using the @libp2p/perf testing protocol.

We use an abortable source for every connection - this introduces
a lot of latency.

This PR removes abortable iterator to just listen on the abort event
on the shutdown controller's abort signal and close the connection's
iterator.

This is the same thing that abortable source does except we don't
race every `next` from the source.

I've added a simple benchmark - in my testing it increases
throughput by 30% or so, this seems to translate to 10-15% in "real world"
performance using the `@libp2p/perf` testing protocol.
@achingbrain achingbrain requested a review from a team as a code owner November 11, 2023 17:44
@codecov-commenter
Copy link

codecov-commenter commented Nov 11, 2023

Codecov Report

Attention: 40 lines in your changes are missing coverage. Please review.

Files Coverage Δ
src/encode.ts 100.00% <100.00%> (ø)
test/codec.spec.ts 100.00% <100.00%> (ø)
test/compliance.spec.ts 100.00% <100.00%> (ø)
test/decode.spec.ts 96.01% <100.00%> (-0.02%) ⬇️
test/muxer.spec.ts 100.00% <100.00%> (ø)
test/stream.spec.ts 100.00% <100.00%> (+0.90%) ⬆️
test/util.ts 100.00% <100.00%> (ø)
src/decode.ts 97.22% <88.88%> (+2.08%) ⬆️
src/stream.ts 98.01% <99.08%> (+2.52%) ⬆️
test/codec.util.ts 94.28% <66.66%> (ø)
... and 2 more

📢 Thoughts on this report? Let us know!

@wemeetagain wemeetagain merged commit 064bf1c into master Nov 12, 2023
17 checks passed
@wemeetagain wemeetagain deleted the perf/remove-abortable-iterator branch November 12, 2023 06:26
github-actions bot pushed a commit that referenced this pull request Nov 12, 2023
## [5.0.2](v5.0.1...v5.0.2) (2023-11-12)

### Bug Fixes

* remove abortable iterator from muxer ([#63](#63)) ([064bf1c](064bf1c))
Copy link

🎉 This PR is included in version 5.0.2 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

3 participants