Skip to content

Conversation

@mcollina
Copy link
Member

@mcollina mcollina commented Dec 1, 2025

Summary

  • Replace setTimeout-based polling in lib/wait.js with Atomics.waitAsync for more efficient, event-driven waiting on SharedArrayBuffer state changes
  • Increase fallback timeout to 10 seconds (from 10ms) since notifications are now properly sent
  • Add Atomics.notify(READ_INDEX) after buffer wraparound to prevent missed notifications
  • Add 1-hour setInterval keepalive in lib/worker.js since Atomics.waitAsync promises don't keep the Node.js event loop alive

Benchmark Results

Benchmark main this PR Improvement
ThreadStreamSync 5.905s 3.885s 34% faster
ThreadStreamAsync 13.905s 7.304s 47% faster
Sonic 4.831s 4.776s ~same
SonicSync 10.476s 10.355s ~same
Core 2.854s 2.831s ~same
Console 11.698s 12.007s ~same

Test plan

  • All existing tests pass
  • Verified overflow scenarios work correctly with setImmediate between writes
  • Verified high-throughput scenario (1GB write) completes in ~3 seconds

🤖 Generated with Claude Code

Replace the setTimeout-based polling in wait.js with Atomics.waitAsync
for more efficient, event-driven waiting on SharedArrayBuffer state changes.

Key changes:
- Use Atomics.waitAsync to wait for value changes with notifications
- Add 10ms fallback timeout for missed notifications
- Add setInterval keepalive in worker.js since waitAsync promises don't
  keep the Node.js event loop alive

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
mcollina and others added 2 commits December 1, 2025 20:09
Add Atomics.notify(READ_INDEX) after resetting indexes to 0 during buffer
wraparound. Without this, the worker would wait for the full timeout when
the buffer wrapped around, causing significant performance degradation.

Also increase WAIT_MS from 10ms to 1000ms as a fallback poll interval,
which is now safe since notifications are properly sent.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@mcollina mcollina marked this pull request as ready for review December 1, 2025 21:59
@mcollina mcollina requested a review from jsumners December 1, 2025 22:00
mcollina and others added 2 commits December 1, 2025 23:01
Since notifications are now properly sent after buffer wraparound,
the timeout is just a safety fallback. Increase to 10s for better
power efficiency when idle.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
The keepalive only needs to prevent worker exit, not run frequently.
1 hour is sufficient and more power-efficient.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
@mcollina mcollina merged commit 6d3f6a8 into main Dec 2, 2025
22 checks passed
@mcollina mcollina deleted the feat/atomics-waitasync branch December 2, 2025 08:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants