Perf/io pipelining followup - #10
Open
nyo16 wants to merge 2 commits into
Open
Conversation
…gnment Second measurement-driven cycle against the code v1.3.0 produced. One liveness bug, one resource leak, one one-byte constant worth 42% of read wall time, and three responsiveness fixes. Harness committed under bench/ so the next cycle does not re-litigate any of it from code reading. Fixed ----- run/2 hung forever on :input larger than ~128 KiB. run_io/3 wrote all of :input to completion before the read loop started, so any filter command (cat, gzip, jq) filled its 64 KiB stdout pipe, blocked in write(2), stopped draining stdin, and left us blocked filling stdin. Default :timeout is nil, i.e. :infinity, so there was no escape. Completed at <= 131072 bytes, hung indefinitely at >= 262144. stream!/2 was never affected -- it has always written from a Task. Writer and reap now live in NetRunner.InputWriter, shared by both entry points so they cannot drift apart again. 16 MiB through cat: run/2 1163 MB/s vs stream!/2 1130 MB/s. run/2 and stream!/2 each leaked a Process GenServer, a Watcher, a UDS socket and three pipe FDs per call. Neither hands the pid to the caller, so nothing could stop them; teardown relied on an owner monitor that only fires when the caller dies -- unbounded for a long-lived caller. Measured 100 leaked processes per 50 calls, now 0. New public NetRunner.Process.stop/1. This also resolves the teardown question cycle 1 left open. Shepherd now sends MSG_CHILD_EXITED before cgroup_cleanup(), which retries rmdir ten times with usleep(100000) between. With --cgroup-path a caller could wait a full second in await_exit for a status the shepherd already held. NOT VERIFIED ON LINUX -- see bench/LINUX_VERIFICATION.md. Changed ------- Default read size 65535 -> 65536. One byte under pipe capacity leaves one byte behind in a saturated pipe, costing a whole extra GenServer round trip: 64 MiB read goes 1773-1834 chunks / 26-31 ms -> 1024 chunks / 18-19 ms. 65536 is also exactly nif_read's stack buffer, so it stays on the allocation-free path; see ADR-9. Pipe.read/2's duplicate default was deleted rather than redirected. consume_stderr/1 bounded at 16 chunks (~1 MiB) per pass with a self-sent resume. Honest scope: worst concurrent handle_call under a 256 MB stderr flood measured 279 us, not the seconds the unbounded recursion suggests -- pipe capacity already capped it. Now 52 us, at the idle jitter floor. Drain throughput unchanged. Daemon.write/2 no longer blocks the Daemon on a backpressured child, and on_output: :log coalesces instead of one Logger call per chunk. Performance ----------- Stream no longer polls the writer with Task.yield(writer, 0) on every stdout chunk -- O(mailbox length) per chunk for a GenServer/LiveView consumer. append_stderr_tail's steady-state branch builds at exactly cap in one pass instead of concat-then-slice. Write and drain loops fold byte and syscall counts through parameters instead of rebuilding state per iteration. Verification ------------ make clean && make all warning-free under -Wall -Wextra -Werror; --warnings-as-errors, format, credo, dialyzer (0 errors), mix docs (0 warnings); 184 tests passing; UBSan suite clean. All three new regression guards were mutation-checked -- notably the stderr drain test, whose obvious form passed the mutation because a 64 KiB pipe never lets a pass reach its budget. Unverified: Linux. Phase 3 is a provable no-op on macOS and the read-size win is macOS-shaped (Linux pipes are 1 MiB). ASan build is clean but its suite cannot run on macOS -- SIP strips DYLD_INSERT_LIBRARIES before beam.smp. Instructions in bench/LINUX_VERIFICATION.md.
Move .claude/{audit,plans,prs} to tmp/, which is already gitignored, and
ignore /.claude/ wholesale. These are local working notes, not part of the
library.
bench/LINUX_VERIFICATION.md cross-referenced those files heavily, which would
have left the Linux handoff pointing at paths absent from a fresh clone. Every
such reference is now inlined instead: the mutation-testing lesson, ADR-9's
upper bound on the read size, and the macOS baseline it says to compare
against. The doc no longer depends on anything outside the repo, and says so.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.