perf: fast wasm streaming

[littledivy]

Ref denoland/deno_core#1243

[Copilot]

Pull Request Overview

This PR optimizes WASM streaming performance by replacing manual chunk-by-chunk feeding with a more efficient stream-based approach. The changes align with deno_core PR #1243 and move WASM data streaming from JavaScript to a native operation.

• Refactored handleWasmStreaming to use op_wasm_streaming_stream_feed instead of manually reading and feeding chunks
• Added logic to detect and reuse resource-backed streams when available
• Updated dependencies to use a local deno_core patch for testing

Reviewed Changes

Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.

File	Description
ext/fetch/26_fetch.js	Refactored WASM streaming to use the new op_wasm_streaming_stream_feed operation, eliminating the manual reader loop and adding resource backing detection
Cargo.toml	Added path-based patch for local deno_core development
Cargo.lock	Updated dependency entries to reflect the local deno_core patch and related dependency changes (proc-macro-rules removed, syn-match added)

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

This Cargo.toml patch should not be committed to production code. Path-based patches are typically used for local development and testing. This should either be removed before merging or replaced with a proper git or version-based dependency once the corresponding deno_core changes are published.

Suggested change

deno_core = { path = "../deno_core/core" }

[coderabbitai]

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

A patch override redirects deno_core to a specific git revision instead of using crates.io. The WebAssembly streaming implementation in the fetch module is refactored to use a new op_wasm_streaming_stream_feed operation with updated resource and autoClose parameter handling.

Changes

Cohort / File(s)	Summary
Dependency management Cargo.toml	Added [patch.crates-io] section with deno_core override pointing to a specific git revision from the denoland/deno_core repository.
WebAssembly streaming refactor ext/fetch/26_fetch.js	Replaced per-chunk manual streaming loop with op_wasm_streaming_stream_feed operation. Updated imports: removed op_wasm_streaming_feed, added op_wasm_streaming_stream_feed. Added logic to determine stream resource RID and autoClose flag, preferring existing readable stream backing when available or creating a new resource with autoClose defaulting to true.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

• Cargo.toml patch override: Straightforward dependency redirection; verify git revision is valid and intentional.
• ext/fetch/26_fetch.js streaming logic: Confirm the RID determination logic (preference for existing readable stream) is correct; verify the new op signature and parameter passing align with the underlying streaming implementation; ensure cleanup and error paths remain intact.

Poem

🐰 A patch from deno_core now flows,
WebAssembly streams in cleaner rows,
No chunky loops, just streamlined feed,
Resource RIDs fulfill the need! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title 'perf: fast wasm streaming' directly and clearly summarizes the main change—optimizing WebAssembly streaming performance by replacing manual chunk-feeding with a faster streaming feed implementation.
Description check	✅ Passed	The description references the relevant upstream PR in deno_core, providing context for the changes even though it is minimal; it is clearly related to the changeset.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment

---

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 0

♻️ Duplicate comments (1)

Cargo.toml (1)

533-535: Treat the deno_core git patch as temporary and clean it up before/after landing

The [patch.crates-io] override is fine for cross-repo development, but it should not be left as a long‑term override in main. It silently replaces the pinned crates.io version in [workspace.dependencies] and can surprise downstream consumers.

Once the deno_core changes are released, please either:

• Bump deno_core in [workspace.dependencies] to the new published version and remove this patch, or
• Otherwise ensure this patch is strictly temporary and not kept in production builds.

A follow‑up PR or a TODO comment pointing to the cleanup would also help avoid it sticking around.

-[patch.crates-io]
-deno_core = { git = "https://github.com/denoland/deno_core", rev = "70c2762" }
+## Temporary override for in-flight deno_core changes used by fast wasm streaming.
+## Remove once a crates.io release including op_wasm_streaming_stream_feed is available.
+#[patch.crates-io]
+#deno_core = { git = "https://github.com/denoland/deno_core", rev = "70c2762" }

🧹 Nitpick comments (1)

ext/fetch/26_fetch.js (1)

580-605: Wasm streaming resource wiring looks correct; consider unrefable backing parity

The new handleWasmStreaming logic correctly:

• Validates the Response,
• Reuses an existing readable stream resource backing when available (getReadableStreamResourceBacking),
• Falls back to resourceForReadableStream with autoClose = true when there is no backing, and
• Lets op_wasm_streaming_stream_feed(rid, streamRid, autoClose) manage the feed and cleanup via close/abort.

This looks functionally sound and avoids the per‑chunk JS loop.

If you want to mirror ext/web/06_streams.js more closely and also reuse unrefable stream backings, you could optionally:

• Import getReadableStreamResourceBackingUnrefable from ext:deno_web/06_streams.js, and
• Use the combined pattern:

-import {
-  errorReadableStream,
-  getReadableStreamResourceBacking,
-  readableStreamForRid,
-  ReadableStreamPrototype,
-  resourceForReadableStream,
-} from "ext:deno_web/06_streams.js";
+import {
+  errorReadableStream,
+  getReadableStreamResourceBacking,
+  getReadableStreamResourceBackingUnrefable,
+  readableStreamForRid,
+  ReadableStreamPrototype,
+  resourceForReadableStream,
+} from "ext:deno_web/06_streams.js";

-      const resourceBacking = getReadableStreamResourceBacking(
-        innerResponse.body?.stream,
-      );
+      const stream = innerResponse.body?.stream;
+      const resourceBacking =
+        getReadableStreamResourceBacking(stream) ??
+        getReadableStreamResourceBackingUnrefable(stream);

This is a minor parity tweak; the current implementation is otherwise solid.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b0cfbf5 and adc185d.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (2)

• Cargo.toml (1 hunks)
• ext/fetch/26_fetch.js (2 hunks)

🧰 Additional context used

🧬 Code graph analysis (1)

ext/fetch/26_fetch.js (1)

ext/web/06_streams.js (3)

• res (793-793)
• resourceBacking (1095-1096)
• rid (899-901)

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (10)

• GitHub Check: test release linux-x86_64
• GitHub Check: test debug linux-aarch64
• GitHub Check: test debug macos-aarch64
• GitHub Check: test debug linux-x86_64
• GitHub Check: test debug windows-x86_64
• GitHub Check: test debug macos-x86_64
• GitHub Check: build libs
• GitHub Check: lint debug macos-x86_64
• GitHub Check: lint debug windows-x86_64
• GitHub Check: lint debug linux-x86_64

🔇 Additional comments (1)

ext/fetch/26_fetch.js (1)

15-20: Import of op_wasm_streaming_stream_feed matches the new streaming path

Switching the import to op_wasm_streaming_stream_feed aligns with the new streaming implementation below; no issues from this side.