Skip to content

stream: cache minimum cursor count in share#63262

Open
trivikr wants to merge 4 commits into
nodejs:mainfrom
trivikr:streams-iter-share
Open

stream: cache minimum cursor count in share#63262
trivikr wants to merge 4 commits into
nodejs:mainfrom
trivikr:streams-iter-share

Conversation

@trivikr
Copy link
Copy Markdown
Member

@trivikr trivikr commented May 12, 2026

Description

This updates share() and shareSync() to avoid recomputing the minimum
consumer cursor on every buffer trim attempt.

Instead of scanning all consumers each time, share now caches the current
minimum cursor and tracks how many consumers are positioned at that
cursor. The minimum is recomputed only when the last consumer at the
cached minimum advances or detaches.

The shared getMinCursor() utility now returns both the minimum cursor
and the number of consumers at that cursor.

Benchmark

                                                                                     confidence improvement accuracy (*)   (**)  (***)
streams/iter-throughput-share.js n=5 backpressure='block' batches=10000 consumers=2         ***      6.06 %       ±2.01% ±2.76% ±3.76%
streams/iter-throughput-share.js n=5 backpressure='block' batches=10000 consumers=32        ***     69.76 %       ±1.56% ±2.15% ±2.96%
streams/iter-throughput-share.js n=5 backpressure='block' batches=10000 consumers=8         ***     20.96 %       ±1.71% ±2.38% ±3.35%

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 12, 2026
@trivikr trivikr force-pushed the streams-iter-share branch from 9d05a7a to a0ce67e Compare May 12, 2026 06:14
Track the number of consumers at the cached minimum cursor in share()
so the minimum is only recomputed when the last consumer at that cursor
advances or detaches.

This avoids scanning every consumer on each trim attempt when multiple
consumers advance through a shared buffer.

Signed-off-by: Kamat, Trivikram <16024985+trivikr@users.noreply.github.com>
Assisted-by: openai:gpt-5.5
@trivikr trivikr force-pushed the streams-iter-share branch from a0ce67e to 8a32f1c Compare May 12, 2026 06:51
@trivikr
Copy link
Copy Markdown
Member Author

trivikr commented May 12, 2026

I'd attempted making the same changes in broadcast. They were reverted since benchmarks showed regression

                                                                                       confidence improvement accuracy (*)    (**)   (***)
streams/iter-throughput-broadcast.js n=5 datasize=1048576 consumers=1 api='classic'                   -4.72 %       ±8.35% ±11.66% ±16.39%
streams/iter-throughput-broadcast.js n=5 datasize=1048576 consumers=1 api='iter'              ***    -17.21 %       ±8.78% ±12.06% ±16.50%
streams/iter-throughput-broadcast.js n=5 datasize=1048576 consumers=1 api='webstream'         ***    -31.77 %      ±12.75% ±17.99% ±25.68%
streams/iter-throughput-broadcast.js n=5 datasize=1048576 consumers=2 api='classic'                   -2.32 %       ±5.56%  ±7.76% ±10.90%
streams/iter-throughput-broadcast.js n=5 datasize=1048576 consumers=2 api='iter'               **    -11.18 %       ±6.34%  ±8.72% ±11.95%
streams/iter-throughput-broadcast.js n=5 datasize=1048576 consumers=2 api='webstream'         ***    -22.39 %       ±6.13%  ±8.68% ±12.44%
streams/iter-throughput-broadcast.js n=5 datasize=1048576 consumers=4 api='classic'                   -5.51 %       ±7.07%  ±9.78% ±13.55%
streams/iter-throughput-broadcast.js n=5 datasize=1048576 consumers=4 api='iter'                      -1.68 %       ±6.46%  ±8.92% ±12.30%
streams/iter-throughput-broadcast.js n=5 datasize=1048576 consumers=4 api='webstream'         ***    -26.03 %       ±4.97%  ±6.88%  ±9.54%
streams/iter-throughput-broadcast.js n=5 datasize=16777216 consumers=1 api='classic'            *     -4.21 %       ±3.90%  ±5.36%  ±7.33%
streams/iter-throughput-broadcast.js n=5 datasize=16777216 consumers=1 api='iter'              **    -10.27 %       ±7.18% ±10.03% ±14.07%
streams/iter-throughput-broadcast.js n=5 datasize=16777216 consumers=1 api='webstream'         **    -20.80 %      ±11.70% ±16.63% ±24.05%
streams/iter-throughput-broadcast.js n=5 datasize=16777216 consumers=2 api='classic'                  -4.53 %       ±5.56%  ±7.70% ±10.66%
streams/iter-throughput-broadcast.js n=5 datasize=16777216 consumers=2 api='iter'             ***    -11.46 %       ±5.39%  ±7.40% ±10.13%
streams/iter-throughput-broadcast.js n=5 datasize=16777216 consumers=2 api='webstream'         **    -12.59 %       ±9.01% ±12.36% ±16.87%
streams/iter-throughput-broadcast.js n=5 datasize=16777216 consumers=4 api='classic'                  -4.09 %       ±6.01%  ±8.35% ±11.61%
streams/iter-throughput-broadcast.js n=5 datasize=16777216 consumers=4 api='iter'               *     -8.16 %       ±6.54%  ±8.98% ±12.27%
streams/iter-throughput-broadcast.js n=5 datasize=16777216 consumers=4 api='webstream'        ***     -8.95 %       ±4.03%  ±5.55%  ±7.60%

Comment thread lib/internal/streams/iter/broadcast.js Outdated
Comment thread lib/internal/streams/iter/share.js Outdated
Comment thread lib/internal/streams/iter/share.js Outdated
Comment thread lib/internal/streams/iter/share.js
@trivikr trivikr marked this pull request as ready for review May 12, 2026 08:02
@codecov
Copy link
Copy Markdown

codecov Bot commented May 12, 2026

Codecov Report

❌ Patch coverage is 85.15625% with 19 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.05%. Comparing base (78bbee3) to head (2eda9ba).
⚠️ Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/streams/iter/share.js 83.33% 19 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##             main   #63262    +/-   ##
========================================
  Coverage   90.04%   90.05%            
========================================
  Files         713      714     +1     
  Lines      225003   225333   +330     
  Branches    42536    42605    +69     
========================================
+ Hits       202606   202915   +309     
- Misses      14177    14189    +12     
- Partials     8220     8229     +9     
Files with missing lines Coverage Δ
lib/internal/streams/iter/broadcast.js 84.66% <100.00%> (+0.02%) ⬆️
lib/internal/streams/iter/utils.js 100.00% <100.00%> (ø)
lib/internal/streams/iter/share.js 84.12% <83.33%> (-0.06%) ⬇️

... and 38 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 12, 2026
@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label May 12, 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.

3 participants