-
Notifications
You must be signed in to change notification settings - Fork 43
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 #488
Conversation
AbortableSource is slow because it races promises against every chunk causing extra async work. It's only really necessary if we're going to pass the source off to another component. Here we don't do that so it's simpler to just add a listener for the abort event and close the stream.
// similar to pushable.end() but clear the internal buffer | ||
await this.pushable.return() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think this is necessary since the pushable
is only referenced by this stream class so everything will be garbage collected shortly after the close
method is called on this OutboundStream
instance.
// similar to pushable.end() but clear the internal buffer | |
await this.pushable.return() |
It's also worth noting that any queued messages will not be sent if you call .return
instead of .end
but this method is only called after the remote peer has already disconnected so it's not a big deal.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #488 +/- ##
==========================================
- Coverage 81.40% 81.39% -0.02%
==========================================
Files 48 48
Lines 12325 12333 +8
Branches 1301 1302 +1
==========================================
+ Hits 10033 10038 +5
- Misses 2292 2295 +3 ☔ View full report in Codecov by Sentry. |
#434 can probably be closed if this is merged. |
AbortableSource is slow because it races promises against every chunk causing extra async work.
It's only really necessary if we're going to pass the source off to another component.
Here we don't do that so it's simpler to just add a listener for the abort event and close the stream.