[kixelated]

Finishes quest/m2/perf/lite-request-allocs.md, the two follow-ups from moq-dev/moq#6952.

Problem

• Path::concat (so Path::join for any result over 22 bytes) built a String, then copied it into the path's Arc<str>: two allocations where one is needed.
• The lite request serve (TRACK, SUBSCRIBE, FETCH) copied the track name twice: once into the owned message, and again into RequestServe::track for its logs.

Approach

• concat joins a long path in an arrayvec::ArrayString on the stack (uninitialized, like ietf::namespace::from_tuple), so the shared Arc<str> is the only allocation. The inline branch (22 bytes or less) is unchanged.
• RequestServe no longer copies the name. While the request resolves, its logs read the name from the message (RequestServe::track()). As the request starts, Request::take_track moves the message's Cow into the log context, and Request::start takes the name as &str for the track lookup. Nothing reads the message's track after start. A graceful request (SUBSCRIBE) whose requester FINs before it starts moves the name into the log context the same way, so its complete log still names the track.
• moq-net-tests/tests/announce_allocs.rs gains two tests:
  • a_joined_path_allocates_only_its_buffer: 0 allocations joining a short path, 1 for a long one.
  • a_lite_request_keeps_one_copy_of_its_track_name: a full mock session subscribes on moq-lite 03 and 07. It counts the allocations sized exactly like an odd-length track name, only while the publisher's driver polls (a Watched wrapper), so the subscriber's own copies don't count.

Measurements

All deterministic: allocation counts, and user-space instruction counts pinned to one E-core. Before is main at 2d13685.

announce_allocs (the new tests, run against both):

	main	this PR
Path::join, 10 / 47 byte result	0 / 2	0 / 1
publisher's track-name copies, moq-lite 03 (SUBSCRIBE)	2	1
publisher's track-name copies, moq-lite 07 (TRACK + SUBSCRIBE)	4	2

cargo bench -p moq-net --features fuzz --bench path (allocations per op; rows that changed):

	main	this PR
path/join/medium (26 bytes)	2.00	1.00
path/join/long	2.00	1.00
decode/announce_lite07/medium	4.19	3.60
decode/announce_lite07/long	6.00	5.01
origin/request_rooted_str/long	348	340

The other 28 rows are unchanged.

Instructions per join (perf stat -e cpu_atom/instructions/u, 10M joins, net of an empty loop, using a standalone copy of Repr and both concats):

	main	this PR
short (10 bytes)	80	82
medium (22 bytes)	78	81
long (47 bytes)	352	233

Alternatives and choices

• The inline branch stays hand-written. Routing every join through the stack buffer and Repr::copy_bounded was smaller code, but cost about 42 more instructions per inline join (122 vs 80). Only the shared branch uses the ArrayString.
• The name moves at start, not at decode. Moving it out at decode would leave the message's track empty while RequestState::Resolve holds it, so the started log's request = ?msg would print track: "". Until start, the message keeps the one copy and RequestServe::track() reads it there.
• Request::start takes a fourth argument (track: &str). The trait is private, and a struct for one call site would add more than it saves.
• The session test does its own handshake instead of using support::harness::connect_mock, so it can wrap the server's driver in Watched. The harness has no hook for that, and adding one for a single test seemed worse.
• No just bench BASE run. The host is shared and loaded, so wall-clock relay benches would be noise. The perf epic's before/after is the deterministic allocation and instruction counts above.

Impact

• Public API: none. Path::concat is pub(crate), and Request is private to lite::publisher.
• Wire: none.

Checks

On 0b2bec95e5471624feb40401da0f53863164f8df, after merging main at 62ccb69bac845563dc798a27b4665056e9e56a56:

• The selected just ci origin/main Rust gate passed 9,477 tests, with 109 skipped, plus clippy, formatting, docs and dependency checks. Both allocation tests, malformed complete-frame and SUBSCRIBE-update tests, and path length-boundary tests passed.
• backend::quic_client_close_drains_finished_track passed on its first test run. moq-dev/moq#7634 remains separate and untouched; this run does not prove that shared race is fixed.
• Quest validation, drill sensitivity, shell, TOML, Nix, Justfile, large-file and GitHub tooling checks passed.
• The full invocation exited unsuccessfully because the clean Nix environment omitted the Nix executable from PATH, stopping the Markdown lane's libav setup. The isolated Markdown lane then passed with PATH retained. Earlier launcher attempts stopped before tests because of the inherited Cargo shim environment and stale dev-shell tools. No failed test was retried.
• cargo bench --locked -p moq-net --features fuzz --bench path passed: joins allocate 0 for short paths and 1 for medium/long paths; lite-07 announce decoding uses 3.60 / 5.01 allocations for medium/long paths.
• CPU control: compiled each revision's actual Repr and concat into an optimized standalone harness, with 10M joins pinned to CPU 0 and cpu_core/instructions/u, subtracting an empty loop. Base / current instructions per join: 70 / 71 (short), 59 / 59 (22-byte inline boundary), 340 / 211 (long). This isolates join CPU cost from host scheduling noise.
• quest check --base origin/main and git diff --check: passed.
• No API or wire changes, so cross-language interop is not required by this change.

Merge from main

• Iteration on 2026-11-02 merged main at 62ccb69 without textual conflicts. The one-allocation join, single request-name ownership, FIN logging fix, and borrowed wire decoding are preserved. Public API and wire formats remain unchanged.

• No textual conflicts. One semantic conflict: moq-dev/moq#7027 moved moq-clock into kio::clock, so the new session test now uses #[kio::test] instead of #[moq_clock::test].

Review

• Original reviewed head: 5844c41152ec1f013f73557432b817f5c206f1c0. Codex (quest review), gpt-6.1-sol, medium effort, session 01a19806-fabb-75f1-8c4b-60ed4ee58f93, round 1: no findings.
• Current head: 0b2bec95e5471624feb40401da0f53863164f8df. The only new commit is a conflict-free merge from origin/main, so Quest's review exemption carries the existing clean review forward. No implementation changes or new review round.
• Online: Pullfrog's FIN-during-resolve logging finding is fixed in the original reviewed head. Both subsequent posted reviews found no new issues.

Follow-ups

• The lite subscriber also copies the track name twice per subscription, in SubscriberDriver::poll and TrackServe::prepare_establish. The new test's backtraces showed both. This is already covered by Small hot-path waste.

(Written by GPT-6.1 Sol)

[coderabbitai]

No actionable comments were generated in the recent review.

Recent review info

Run configuration

• Source mode: Auto
• Repository: Unavailable
• Branch: Unavailable
• File: No configuration file from this repository applied
• Commit: Unavailable
• Layers: Organization UI
• Review profile: CHILL
• Plan: Advanced
• Run ID: 7ab98469-1bf1-474b-9eff-a9f4fca0023b

Commits

Reviewing files that changed from the base of the PR and between 62ccb69 and 0b2bec9.

Files selected for processing: 5

• quest/m2/perf/README.md
• quest/m2/perf/lite-request-allocs.md
• rs/moq-net-tests/tests/announce_allocs.rs
• rs/moq-net/src/lite/publisher.rs
• rs/moq-net/src/path/mod.rs

Files with no reviewable changes: 2

• quest/m2/perf/README.md
• quest/m2/perf/lite-request-allocs.md

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 0 remain after this review.

Walkthrough

Path joining now uses a stack-backed buffer for long results. Lite publisher requests extract track names from decoded messages and pass them separately to request handlers. Allocation tests cover path joins and track-name-sized allocations. The related performance quest entry and document were removed.

Merge Risk: Minimal · up to 0b2be

The allocation changes have no identified merge-blocking behavior regression.

Pre-merge checks: 4 · 1

Failed checks: 1 warning

Check name	Status	Explanation	Resolution
Docstring Coverage		Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta…	Write docstrings for the functions missing them to satisfy the coverage threshold.

Passed checks: 4 passed

Check name	Status	Explanation
Title check		The title clearly identifies both primary performance changes: reducing allocations for long path joins and lite request track names.
Description check		The description directly explains the allocation reductions, implementation approach, measurements, tests, and impact described by the changeset.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 36.36% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 11 functions across 3 files.

Resolution

Write docstrings for the functions missing them to satisfy the coverage threshold.

Finishing Touches

Simplify code

• Commit to this branch
• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

Share

• X
• Mastodon
• Reddit
• LinkedIn

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: a7a6f79

No actionable bugs found. The stack-buffer join retains the existing byte bound and inline path; moving the request's track name at start preserves lookup inputs without another owned copy. Subsequent SUBSCRIBE/FETCH processing does not depend on the emptied message field.

Direction: these are appropriately scoped ownership optimizations. Reusing ArrayString and keeping the change inside the private request machinery avoids a new public API or wire change; the focused allocation tests are useful regression coverage.

Verification: static inspection of the changes and relevant callers/state transitions only. I did not run tests or benchmarks or independently reproduce the reported allocation/instruction measurements.

(Written by OpenAI)

[pullfrog]

ℹ️ No critical issues — one minor, log-only suggestion below.

Reviewed changes — full diff at a7a6f79, plus the two new allocation tests run locally.

• Path::concat long branch — joins on a stack arrayvec::ArrayString::<Path::MAX_BYTES> instead of a String, so the shared Arc<str> is the only allocation; inline branch untouched. Correct: len <= MAX_BYTES is checked before the pushes, so nothing can overflow the buffer.
• Lite request track name — Request gains take_track and a track: &str arg to start; RequestServe::track and the track() reader move the name from the message into the log context at start instead of copying it at decode. All four Request impls and the one unit-test call site are updated; the message's emptied track is never read after start.
• announce_allocs.rs — a_joined_path_allocates_only_its_buffer and a_lite_request_keeps_one_copy_of_its_track_name (the Watched driver scopes the count to the publisher's poll). Both pass; moq-net lib tests compile.
• Quest bookkeeping — lite-request-allocs.md deleted and its README bullet removed; no dangling references.

ℹ️ RequestServe::track loses the name when a graceful request FINs mid-resolve

track() reads the name from the message only while the state is Resolve, and from self.track (populated by take_track at start) otherwise. But poll_serve can leave Resolve without starting: when the requester FINs while resolving, line 1730 replaces the state with Finish and drops the message, so self.track stays empty. SubscribeServe is the only GRACEFUL impl, so a subscriber that closes before its broadcast resolves now logs track="" in {} complete (and any later line), where before this PR the name was captured at decode. No behavior beyond logging: track() is the sole reader of self.track.

Technical details

# Preserve the track name across the FIN-during-resolve path

## Affected sites
- `rs/moq-net/src/lite/publisher.rs:1667-1671` — `track()` returns `&self.track` for any non-`Resolve` state.
- `rs/moq-net/src/lite/publisher.rs:1730` — `self.state = RequestState::Finish { finished: false };` drops the `Resolve` message without calling `take_track`.

## Required outcome
- After a graceful request FINs before `start`, the request's log context still names the track, matching the pre-PR behavior and the JS publisher (`js/net/src/lite/publisher.ts:1510` keeps logging `msg.track`).

## Suggested approach (optional)
- Before overwriting the state at line 1730, when it is `Resolve { msg, .. }`, set `self.track = R::take_track(msg)` so the one copy moves into the log context exactly as it does at `start`.

  ^{｜ Fix it ➔ ｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[kixelated]

Fixed Pullfrog's log-only finding in 5844c41: when a graceful request FINs while still resolving, poll_serve now moves the name out of the message (R::take_track) before dropping it, so the complete log names the track, as start already did.

(Written by Claude Opus 5.5)

[kixelated]

Automated review by review (OpenAI)

Reviewed commit: 5844c41

No new actionable bugs found. Compared the base-relative changes with my review at a7a6f79, separating the main merge from the follow-up fix.

Fixed: Pullfrog's FIN-during-resolve logging finding. In rs/moq-net/src/lite/publisher.rs:1731–1735, the transition to Finish now moves the unresolved message's track name into the log context; an already-serving request retains its existing name. The allocation test also correctly uses the current kio test harness.

Direction: the small ownership transfer addresses the missing transition without adding another allocation or abstraction. The original scoped optimization remains sound in the changed areas.

Verification: static inspection of the old/new base-relative diffs, request state transitions, and test harness only. I did not run tests or benchmarks; the FIN logging fix has not been runtime-verified by me.

(Written by OpenAI)

[pullfrog]

✅ No new issues found.

Reviewed changes — the commits since the prior pullfrog review at a7a6f79.

• Closed the FIN-during-resolve log gap — poll_serve now moves an unresolved Resolve message's track name into self.track before switching to Finish, so a SubscribeServe that FINs mid-resolve still logs its track name. This is the only Resolve→non-Resolve transition that bypassed take_track; the other error exits already logged through track(), which reads the message while the state is Resolve.
• Renamed the new session test's harness attribute — #[moq_clock::test] → #[kio::test], matching main moving moq-clock into kio::clock.
• Merged main — a perf-quest README bullet swap; the finished quest file stays deleted.

  ^{｜ View workflow run ｜ Using deepseek-v4.1-flash (free via Pullfrog for OSS) ｜ 𝕏}

[moq-bot]

codex / Codex land fixer note:

The batch failed at js/net/src/lite/connection.test.ts:394 in a peer close drains the overtaken group over moq-lite-02. This PR changes Rust code and quest documents, not that JavaScript test or its implementation. The full net suite passed unchanged on rerun. The first local run instead failed the unchanged test a version without a boundary syncs once the request is out; both tests merit flake follow-up. Rust validation passed all 3248 tests under CI's Nextest runner. An initial plain cargo test run encountered a global tracing subscriber collision in setup::tests::credentials_stay_out_of_traces; Nextest's process isolation passed it. No changes were made.

(written by codex / Codex)

[moq-bot]

codex / Codex land fixer: This maintainer pick already used its mechanical fi·x attempt. Run /quest-iterate to diagnose the repeated failure, then pick it again. Recommendation: run /quest-iterate before picking it again.

+    "reason": "Healthy unchanged; passed quest check; bun run --cwd js/net build; bun run --cwd js/net test (2656 passed). Batch flaky test: a peer close drains the overtaken group over moq-lite-02.",
+    "comment": "This PR changes only quest documents. The document check and full JavaScript net build and test suite passed. The batch failure in `a peer close drains the overtaken group over moq-lite-02` did not recur. No changes were made."
+  },
+  {
+    "number": 7553,
+    "action": "requeue",
+    "reason": "Healthy unchanged; passed bun run --cwd js/net build; bun run --cwd js/net check; bun run --cwd js/net test (2661 passed); quest check. Batch flaky test: a peer close drains the overtaken group over moq-lite-02.",
+    "comment": "The expanded IETF join tests and full JavaScript net suite passed, along with the build, type checks, and quest document check. The batch failure was in an unchanged lite connection test, `a peer close drains the overtaken group over moq-lite-02`, and did not recur. No changes were made."
+  },
+  {
+    "number": 7509,
+    "action": "requeue",
+    "reason": "Healthy unchanged; passed cargo build --locked -p moq-net --benches; cargo nextest run --locked -p moq-net -p moq-net-tests --tests (3247 passed); bun run --cwd js/net build; bun run --cwd js/net test (2656 passed); quest check. Batch flaky test: a peer close drains the overtaken group over moq-lite-02.",
+    "comment": "The Rust benchmark build and all 3247 Nextest tests passed, along with the full JavaScript net build and test suite and quest document check. This PR does not change the JavaScript implementation or test implicated in the batch failure, `a peer close drains the overtaken group over moq-lite-02`, which passed locally. No changes were made."
+  }
+]

exec
/bin/bash -lc "git switch base; python3 -c 'import json, pathlib, subprocess; p=pathlib.Path(\"quest-land-result.json\"); r=json.loads(p.read_text()); expected={7500:\"5844c41152ec1f013f73557432b817f5c206f1c0\",7572:\"5217954b816bb0a4d801c4793fe0235d08fe58cb\",7553:\"378a07b7d0c390b6044c0802e57e1c80e7f56e56\",7509:\"94a1d2be089a2738e8f7c412921d1833a87cf334\"}; assert len(r)==4 and {e[\"number\"] for e in r}==set(expected); allowed={\"number\",\"action\",\"reason\",\"comment\"}; assert all(e[\"action\"]==\"requeue\" and set(e)<=allowed and len(e.get(\"comment\",\"\"))<=4000 for e in r); assert all(subprocess.check_output([\"git\",\"rev-parse\",f\"pr-{n}\"], text=True).strip()==sha for n,sha in expected.items()); print(\"Result validated; all four PR heads unchanged.\")'; git status --short" in /work
 succeeded in 650ms:
Switched to branch 'base'
Result validated; all four PR heads unchanged.
?? quest-land-result.json

diff --git a/quest-land-result.json b/quest-land-result.json
new file mode 100644
index 0000000000000000000000000000000000000000..228e22e02dd4ef7d6543404427e4b7d4d3bbd649
--- /dev/null
+++ b/quest-land-result.json
@@ -0,0 +1,26 @@
+[
+  {
+    "number": 7500,
+    "action": "requeue",
+    "reason": "Healthy unchanged; passed cargo build --locked -p moq-net -p moq-net-tests; cargo nextest run --locked -p moq-net -p moq-net-tests --tests (3248 passed); bun run --cwd js/net build; bun run --cwd js/net test (2656 passed on rerun); quest check. Flaky tests: a peer close drains the overtaken group over moq-lite-02 (batch log), a version without a boundary syncs once the request is out (first local run).",
+    "comment": "The batch failed at js/net/src/lite/connection.test.ts:394 in `a peer close drains the overtaken group over moq-lite-02`. This PR changes Rust code and quest documents, not that JavaScript test or its implementation. The full net suite passed unchanged on rerun. The first local run instead failed the unchanged test `a version without a boundary syncs once the request is out`; both tests merit flake follow-up. Rust validation passed all 3248 tests under CI's Nextest runner. An initial plain cargo test run encountered a global tracing subscriber collision in `setup::tests::credentials_stay_out_of_traces`; Nextest's process isolation passed it. No changes were made."
+  },
+  {
+    "number": 7572,
+    "action": "requeue",
+    "reason": "Healthy unchanged; passed quest check; bun run --cwd js/net build; bun run --cwd js/net test (2656 passed). Batch flaky test: a peer close drains the overtaken group over moq-lite-02.",
+    "comment": "This PR changes only quest documents. The document check and full JavaScript net build and test suite passed. The batch failure in `a peer close drains the overtaken group over moq-lite-02` did not recur. No changes were made."
+  },
+  {
+    "number": 7553,
+    "action": "requeue",
+    "reason": "Healthy unchanged; passed bun run --cwd js/net build; bun run --cwd js/net check; bun run --cwd js/net test (2661 passed); quest check. Batch flaky test: a peer close drains the overtaken group over moq-lite-02.",
+    "comment": "The expanded IETF join tests and full JavaScript net suite passed, along with the build, type checks, and quest document check. The batch failure was in an unchanged lite connection test, `a peer close drains the overtaken group over moq-lite-02`, and did not recur. No changes were made."
+  },
+  {
+    "number": 7509,
+    "action": "requeue",
+    "reason": "Healthy unchanged; passed cargo build --locked -p moq-net --benches; cargo nextest run --locked -p moq-net -p moq-net-tests --tests (3247 passed); bun run --cwd js/net build; bun run --cwd js/net test (2656 passed); quest check. Batch flaky test: a peer close drains the overtaken group over moq-lite-02.",
+    "comment": "The Rust benchmark build and all 3247 Nextest tests passed, along with the full JavaScript net build and test suite and quest document check. This PR does not change the JavaScript implementation or test implicated in the batch failure, `a peer close drains the overtaken group over moq-lite-02`, which passed locally. No changes were made."
+  }
+]

codex
Wrote [quest-land-result.json](/work/quest-land-result.json): all four PRs requeued unchanged after passing validation.

No PR was blamed. The logged test failure appears flaky; details and commands are recorded in the result. All PR heads remain unchanged, and the result file is uncommitted.
diff --git a/quest-land-result.json b/quest-land-result.json
new file mode 100644
index 0000000000000000000000000000000000000000..228e22e02dd4ef7d6543404427e4b7d4d3bbd649
--- /dev/null
+++ b/quest-land-result.json
@@ -0,0 +1,26 @@
+[
+  {
+    "number": 7500,
+    "action": "requeue",
+    "reason": "Healthy unchanged; passed cargo build --locked -p moq-net -p moq-net-tests; cargo nextest run --locked -p moq-net -p moq-net-tests --tests (3248 passed); bun run --cwd js/net build; bun run --cwd js/net test (2656 passed on rerun); quest check. Flaky tests: a peer close drains the overtaken group over moq-lite-02 (batch log), a version without a boundary syncs once the request is out (first local run).",
+    "comment": "The batch failed at js/net/src/lite/connection.test.ts:394 in `a peer close drains the overtaken group over moq-lite-02`. This PR changes Rust code and quest documents, not that JavaScript test or its implementation. The full net suite passed unchanged on rerun. The first local run instead failed the unchanged test `a version without a boundary syncs once the request is out`; both tests merit flake follow-up. Rust validation passed all 3248 tests under CI's Nextest runner. An initial plain cargo test run encountered a global tracing subscriber collision in `setup::tests::credentials_stay_out_of_traces`; Nextest's process isolation passed it. No changes were made."
+  },
+  {
+    "number": 7572,
+    "action": "requeue",
+    "reason": "Healthy unchanged; passed quest check; bun run --cwd js/net build; bun run --cwd js/net test (2656 passed). Batch flaky test: a peer close drains the overtaken group over moq-lite-02.",
+    "comment": "This PR changes only quest documents. The document check and full JavaScript net build and test suite passed. The batch failure in `a peer close drains the overtaken group over moq-lite-02` did not recur. No changes were made."
+  },
+  {
+    "number": 7553,
+    "action": "requeue",
+    "reason": "Healthy unchanged; passed bun run --cwd js/net build; bun run --cwd js/net check; bun run --cwd js/net test (2661 passed); quest check. Batch flaky test: a peer close drains the overtaken group over moq-lite-02.",
+    "comment": "The expanded IETF join tests and full JavaScript net suite passed, along with the build, type checks, and quest document check. The batch failure was in an unchanged lite connection test, `a peer close drains the overtaken group over moq-lite-02`, and did not recur. No changes were made."
+  },
+  {
+    "number": 7509,
+    "action": "requeue",
+    "reason": "Healthy unchanged; passed cargo build --locked -p moq-net --benches; cargo nextest run --locked -p moq-net -p moq-net-tests --tests (3247 passed); bun run --cwd js/net build; bun run --cwd js/net test (2656 passed); quest check. Batch flaky test: a peer close drains the overtaken group over moq-lite-02.",
+    "comment": "The Rust benchmark build and all 3247 Nextest tests passed, along with the full JavaScript net build and test suite and quest document check. This PR does not change the JavaScript implementation or test implicated in the batch failure, `a peer close drains the overtaken group over moq-lite-02`, which passed locally. No changes were made."
+  }
+]

tokens used
79,565
Wrote [quest-land-result.json](/work/quest-land-result.json): all four PRs requeued unchanged after passing validation.

No PR was blamed. The logged test failure appears flaky; details and commands are recorded in the result. All PR heads remain unchanged, and the result file is uncommitted.
fixer result written

(written by codex / Codex)