-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
stream: remove LazyTransform #50440
stream: remove LazyTransform #50440
Conversation
Review requested:
|
No longer necessary given recent stream and event optimziations. Refs: nodejs#50428 Refs: nodejs#50439 PR-URL: nodejs#50440
9bf65a3
to
efdea70
Compare
Waiting for #50428 which reduces the overhead of registering the |
No longer necessary given recent stream and event optimziations. Refs: nodejs#50428 Refs: nodejs#50439 PR-URL: nodejs#50440
efdea70
to
5fe2816
Compare
No longer necessary given recent stream and event optimziations. Refs: nodejs#50428 Refs: nodejs#50439 PR-URL: nodejs#50440
5fe2816
to
d9f2770
Compare
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.
LGTM in principle
No longer necessary given recent stream and event optimziations. Refs: nodejs#50428 Refs: nodejs#50439 PR-URL: nodejs#50440
d9f2770
to
06c89b4
Compare
No longer necessary given recent stream and event optimziations. Refs: nodejs#50428 Refs: nodejs#50439 PR-URL: nodejs#50440
06c89b4
to
2bad63b
Compare
No longer necessary given recent stream and event optimziations. Refs: nodejs#50428 Refs: nodejs#50439 PR-URL: nodejs#50440
2bad63b
to
3bab912
Compare
It doesn't seem like that's the case, benchmark results:
|
Hm, yea, that's unfortunate |
Strangely enough the benchmark I added is actually "faster":
|
No longer necessary given recent stream and event optimziations. Refs: nodejs#50428 Refs: nodejs#50439 PR-URL: nodejs#50440
3bab912
to
c249549
Compare
The legacy api doesn't need the readable side. Which is why this will never be fast enough. |
No longer necessary given recent stream and event optimziations.
Refs: #50428
Refs: #50439