Skip to content

refactor(fspy): simplify shm_io to a plain frame arena - #578

Open
wan9chi wants to merge 1 commit into
fspy-close-gatefrom
fspy-plain-frame-arena
Open

refactor(fspy): simplify shm_io to a plain frame arena#578
wan9chi wants to merge 1 commit into
fspy-close-gatefrom
fspy-plain-frame-arena

Conversation

@wan9chi

@wan9chi wan9chi commented Jul 28, 2026

Copy link
Copy Markdown
Member

Motivation

The gated API reads shared memory only when the gate is closed and nothing was in flight, which proves every claimed frame was fully written and published — the gate guard is released only after the frame drops. The three-state frame header (0 / +size / -size), the atomic header stores and the release fences in shm_io existed so a reader could tolerate crashed or in-flight writers: states that are unreachable through the gated API.

This deletes them. The arena keeps exactly one atomic, its end-offset word; a frame becomes | size: u32 | content | padding |, written once at claim time with a plain store, because the claim already made the range exclusive and publication is the gate release rather than the header store. Crash exclusion lives entirely in the gate, where a leaked count fails closed — the run is not cached — instead of being parsed around at read time. The arena also stops being a surface of its own and becomes an internal of the gated module.

No public API change: GatedShmWriter, GatedShmReceiver, GatedShmReader, FrameMut, ClaimError and StillWriting keep their signatures, so the channel layer, the fspy crate and both preload clients are untouched.

🤖 Generated with Claude Code

wan9chi commented Jul 28, 2026

Copy link
Copy Markdown
Member Author

Warning

This pull request is not mergeable via GitHub because a downstack PR is open. Once all requirements are satisfied, merge this PR as a stack on Graphite.
Learn more

This stack of pull requests is managed by Graphite. Learn more about stacking.

@github-actions

github-actions Bot commented Jul 28, 2026

Copy link
Copy Markdown

fspy benchmark

linux

dynamic/launch             change  -1.37%  [ -8.81% ..  +7.65%]  overhead  +271.55%
dynamic/access             change  -1.90%  [ -4.51% ..  +0.86%]  overhead   +62.49%
static/launch              change  -0.31%  [ -7.92% ..  +8.43%]  overhead  +691.76%
static/access              change  +0.39%  [ -3.42% ..  +2.20%]  overhead  +842.29%

macos

dynamic/launch             change  -0.11%  [ -2.13% ..  +1.96%]  overhead  +217.35%
dynamic/access             change  -3.16%  [ -5.93% ..  -0.62%]  overhead   +26.51%

windows

dynamic/launch             change  +0.69%  [ -6.13% ..  +5.95%]  overhead   +24.38%
dynamic/access             change  +0.00%  [ -1.07% ..  +0.88%]  overhead    +9.02%

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: b03e50b979

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +150 to +153
let serialized_size =
usize::try_from(T::serialized_size(value)?).expect("serialized size exceeds usize");
let Some(size) = NonZeroUsize::new(serialized_size) else {
return Err(WriteEncodedError::ZeroSizedFrame);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Check the closed gate before sizing encoded values

When write_encoded is called after the receiver has closed with a zero-sized value such as () or a schema whose size calculation fails, these early returns produce ZeroSizedFrame or Encode instead of Claim(Closed). The previous implementation entered the gate before calculating the serialized size, and the documented post-close contract requires writes to be reported as Claim(Closed) so callers can silently discard them; perform the gate check first and hold its guard through serialization.

Useful? React with 👍 / 👎.

@wan9chi
wan9chi force-pushed the fspy-plain-frame-arena branch from 9485184 to a815712 Compare July 28, 2026 11:18
@wan9chi
wan9chi force-pushed the fspy-close-gate branch 2 times, most recently from 2788743 to e910409 Compare July 28, 2026 11:20
@wan9chi
wan9chi force-pushed the fspy-plain-frame-arena branch 2 times, most recently from 98eff10 to 4deba7a Compare July 29, 2026 01:22
The gated API reads shared memory only when the gate is closed and nothing
was in flight, which proves every claimed frame was fully written and
published. The three-state frame header (`0` / `+size` / `-size`), the
atomic header stores and the release fences existed so a reader could
tolerate crashed or in-flight writers — states that are unreachable through
the gate.

Delete them. A frame is now `| size: u32 | content | padding |`, written
once at claim time with a plain store, and the arena keeps exactly one
atomic: its end offset. Crash exclusion lives entirely in the gate, where a
leaked count fails closed — the run is not cached — instead of being parsed
around. The arena also stops being a surface of its own and becomes an
internal of the gated module.

No public API change: `GatedShmWriter`, `GatedShmReceiver`,
`GatedShmReader`, `FrameMut`, `ClaimError` and `StillWriting` keep their
signatures, so the channel layer, the fspy crate and both preload clients
are untouched.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@wan9chi
wan9chi force-pushed the fspy-plain-frame-arena branch from 4deba7a to d5fee65 Compare July 29, 2026 02:14
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.

1 participant