Skip to content

stream: add sync iterable fast path to pipeTo#63318

Open
trivikr wants to merge 3 commits into
nodejs:mainfrom
trivikr:stream-iter-pipeto-sync-source
Open

stream: add sync iterable fast path to pipeTo#63318
trivikr wants to merge 3 commits into
nodejs:mainfrom
trivikr:stream-iter-pipeto-sync-source

Conversation

@trivikr
Copy link
Copy Markdown
Member

@trivikr trivikr commented May 15, 2026

Description

This adds a pipeTo() fast path for synchronous iterable sources when there
are no transforms, no abort signal, and the writer supports writeSync().

Previously, pipeTo() always normalized through from(), which routes sync
iterables through the async normalization path. The new guarded path writes
incrementally from the sync iterator while preserving async fallback behavior
for values that still require async normalization.

Benchmark

                                                                               confidence improvement accuracy (*)    (**)   (***)
streams/iter-throughput-pipeto.js n=5 datasize=1048576 api='iter-sync-source'         ***     18.03 %       ±3.54%  ±4.86%  ±6.64%
streams/iter-throughput-pipeto.js n=5 datasize=16777216 api='iter-sync-source'                 7.09 %      ±19.35% ±27.78% ±40.79%
streams/iter-throughput-pipeto.js n=5 datasize=67108864 api='iter-sync-source'        ***     10.86 %       ±3.77%  ±5.23%  ±7.26%

Assisted-by: openai:gpt-5.5

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/performance
  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem. labels May 15, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 15, 2026

Codecov Report

❌ Patch coverage is 84.90566% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.05%. Comparing base (8c6e39e) to head (5bac588).
⚠️ Report is 2 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/streams/iter/pull.js 84.90% 8 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##             main   #63318   +/-   ##
=======================================
  Coverage   90.05%   90.05%           
=======================================
  Files         714      714           
  Lines      225564   225612   +48     
  Branches    42660    42668    +8     
=======================================
+ Hits       203134   203184   +50     
- Misses      14209    14216    +7     
+ Partials     8221     8212    -9     
Files with missing lines Coverage Δ
lib/internal/streams/iter/pull.js 85.14% <84.90%> (-0.10%) ⬇️

... and 28 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@trivikr trivikr self-assigned this May 15, 2026
Comment thread lib/internal/streams/iter/pull.js Outdated
Copy link
Copy Markdown
Member

@jasnell jasnell left a comment

Choose a reason for hiding this comment

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

LGTM with a question

@trivikr trivikr requested a review from jasnell May 15, 2026 15:18
@trivikr trivikr added request-ci Add this label to start a Jenkins CI on a PR. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. labels May 15, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label May 15, 2026
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot
Copy link
Copy Markdown
Collaborator

trivikr added 3 commits May 16, 2026 07:54
Avoid normalizing sync iterable sources through from() when pipeTo()
has no transforms or signal and the writer can accept sync writes.
This keeps writes incremental while preserving async fallback for
values that still need it.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
@trivikr trivikr force-pushed the stream-iter-pipeto-sync-source branch from 42d7184 to 5bac588 Compare May 16, 2026 14:55
@trivikr trivikr requested a review from Ethan-Arrowood May 16, 2026 14:55
@trivikr trivikr removed the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label May 16, 2026
@trivikr
Copy link
Copy Markdown
Member Author

trivikr commented May 16, 2026

Re-requested reviews because of rebase which involved resolving conflicts.

@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label May 16, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. request-ci Add this label to start a Jenkins CI on a PR. stream Issues and PRs related to the stream subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants