Repository navigation
Conversation
Review r1 of #1133 at 4a3aa62ArchitectureThe owner is right. I checked
BLOCKERsdk/libc/arch/src/lib.rs:224–237 — the "page distance" this test sweeps is not the copy's distance. The implementer's own negative control shows it. In Consequence: a defect specific to a distance, or to the 512 boundary, can land green. The test's claim, the module header's added line (lib.rs:5, "a forward copy at every page distance below 1 KiB") and the PR's Checks bullet are all false as stated. Cheapest fix: take both ends from one buffer, aligned to 64 by construction. For example, allocate Evidence gap — routed copies above cache size are unmeasured. Failure scenario: a 1 MiB or 64 MiB copy at distance 16 or 256 moves from Cheapest check: add SUGGESTION
Net: about +35 production lines, +44 test lines and +29 issue lines. The edit to the six-tenths issue is accurate: SEND BACK |
|
Review r1, answered at 15a8464:
|
|
A note from #1139 (the kernel's The issue's premise is wrong. It says the kernel's Its exit is met by #1139. #1139's metal A/B ran four boots per bench. Base kernel reads were flat across distances, so the old sequence did not stall at short distances. #1139 replaces it with one
Whichever of #1133 and #1139 lands second deletes the issue file. #1139 never added it. If #1133 lands second, it should drop the file from its branch. |
…osts What #1133 leaves, from round 5 (C1, C3 and C4 missed): the T14's routed copies at 1.4-1.7x of distance 0 and its heap pair at 1.56x, the AMD's stall above 1 MiB (4 MiB 3.4x, 64 MiB 2.0x), and one T14 boot's unexplained mem.memcpy.64. Exit: round 5's rules as written. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… down memcpy was one rep movsb at every distance. With the destination 16 bytes above the source modulo 4096, an 8 KiB copy took 2595 ns on the AMD bench against 90 ns at distance 0, and the T14 lost up to twelvefold from 16 to 256 bytes (osbench copyat, measurement only). Two heap Vecs dlmalloc places one after the other sit at exactly 16: copyat's heap row read 2525 ns on the AMD bench, the rate #1121's IPC copy split saw. AMD's Family 17h judges store-to-load forwarding eligibility by address bits 11:0 alone (Software Optimization Guide for AMD EPYC 7002, 56305 rev. 3.02, section 2.12), so each load of a forward copy at such a distance matches an older store it does not depend on. Copied highest first, the store a load could match is 4 KiB minus the distance behind it. copy_forward now takes an SSE2 loop from the top for 64 bytes or more at a distance under 512 between disjoint ranges, and rep movsb otherwise. The overlap guard keeps memmove's forward branch lowest first. Host tests: a forward copy at every distance below 1 KiB and four line offsets, and an overlapping one a page apart; the latter reds without the guard. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…its test exact The base-against-fix A/B (four boots per bench) showed the SSE2 loop at 136-151 ns per 8 KiB on the T14 and 140 on the AMD against rep movsb's 62 and 90 at distance 0: twice the T14's good copy, and on the AMD slower than the rep movsb it replaced at distances 32 to 384, where that bench does not stall. Both benches retire about one store a cycle there, so the loop is now 32 bytes a store where CPUID and XCR0 say AVX runs, asked once, with SSE2 left for a CPU without it. The A/B also read mem.memcpy.64 16% slower on the T14 (18690 to 15767 MB/s): every copy of 64 bytes or more paid the route's distance and overlap arithmetic. A copy now pays a length and one distance compare; the overlap check and the AVX choice are out of line. The page-distance test took its two ends from separate allocations, so the distance it named was off by the allocator's gap (the head-store mutation first failed at "distance 109"). Both ends now come from one buffer from a 64-byte boundary, and the mutation's first failure is "n 64 at 0 distance 1". A second test calls both loops directly; the AVX one only where the host has AVX, which the arm64 host's Rosetta does not, so the guest's copyat rows check it under QEMU (whose CPU declares +avx). Comments: Intel's documented 4K aliasing in place of a bench reading, SHORT_ALIAS's comment cut to what it is. The kernel issue cites the evidence repository instead of a branch that will be deleted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ng copies to ERMS The round-4 pair (evidence round-4-ab): - The AVX loop read 80 ns per 8 KiB on the AMD where its destination stores fell on 32-byte boundaries and 140 where they straddled them; the T14 read 71-78 against 109-124. A store split across two lines cost as much as the stall. Each loop now stores its top 64 bytes unaligned once and every other store aligned to its width. - Past the caches the T14's copies at a short distance were not stalled by rep movsb (1 MiB at d16: 37.7 us against 40.9 at d0), and the loop was slower there (64 MiB at d256: 10.1 ms against 7.2), while the AMD, which has no ERMS, stalled at 64 MiB (21.8 ms against 10.9) and the loop beat it at every size. A copy over 1 MiB keeps rep movsb where the CPU has ERMS. Host tests: the loops at every destination offset in a 32-byte store. A mutation misplacing a tail store reds three tests; the guard and head mutations still red theirs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
#1139 Round 5 (four boots per bench, evidence round-5-ab): - The AMD's routed copies read 80-86 ns per 8 KiB against 2615 before. - At 4 MiB with d256, a distance its rep movsb does not stall at, the AMD loop read 597 us against rep movsb's 385. With the cap on every CPU, a copy over 1 MiB is base's rep movsb everywhere, measured in base3, and the ERMS reading goes. - What stays is the AMD's stall above 1 MiB, as before (64 MiB at d16: 2.0x of d0), and the T14's routed copies at 1.1-1.4x of rep movsb's best. The kernel-copy issue's premise was false (the old kernel copy was rep movsb; rep movsq; rep movsb and did not stall at those distances), and #1139 meets its exit; it never reached main, so it goes from this branch. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…osts What #1133 leaves, from round 5 (C1, C3 and C4 missed): the T14's routed copies at 1.4-1.7x of distance 0 and its heap pair at 1.56x, the AMD's stall above 1 MiB (4 MiB 3.4x, 64 MiB 2.0x), and one T14 boot's unexplained mem.memcpy.64. Exit: round 5's rules as written. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The comment cited 56305 by the wrong title: it is the Software Optimization Guide for AMD Family 17h Models 30h and Greater Processors, and its §2.12 (p. 39) says the processor uses address bits 11:0 to determine STLF eligibility. The comment also implied every distance under the 512 window stalls, which no bench showed on AMD. The issue now separates the two benches. On Zen 2, #1139's in-process rows stall at d = 8..31 and at nothing tried from 32 up, at 4 to 64 KiB. On the T14 the 512 window comes from copyat's rows alone; a heap pair 64 KiB + 16 apart and #1139's in-process copy at d = 16 did not stall there, so the Tiger Lake mechanism is recorded as not established. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Round 7: does #1133 still claim something? Yes, on the AMDChecked after #1139, #1150's round 7 and main 15c26bf:
Correction to the record: the T14's 512 window rests on Rebased (no conflicts). Host CI exit 0, and Rosetta x86-64 tests exit 0. The AMD A/B on today's main is staged (rule D1–D6, stated before the run; D1 closes the PR if main no longer stalls): https://github.com/ToyOSOrg/evidence/tree/eeac5483dfdaaebade0685885018f9b098d40186/amd-memcpy/round-7 |
Round 7's AMD A/B showed every small copy paying 5-7% (mem.memcpy.64 0.93, .256 0.93, .1024 0.95 of base). The disassembly shows why: memcpy no longer inlined copy_forward and called it, with a second frame, and the distance arithmetic and two branches ran on every copy. copy_forward is now inlined into memcpy and memmove as main's rep movsb behind one length compare. Copies of ROUTED_LEAST (4 KiB) or more go out of line to copy_long, which alone computes the distance, the 1 MiB cap and the overlap guard. 4 KiB is the shortest length a stall has been measured at (AMD copyat 4096 d16 1312 ns against 60); whether shorter copies at those distances stall is unmeasured, and the issue records it. The tests now cover lengths across 4096 and to 8192, and the overlap test's every length reaches copy_long. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With copy_long called, memcpy had to keep dest across the call: its prologue saved rbx and its epilogue restored it on every copy, short ones included. copy_forward now returns dest on both architectures, so memcpy and memmove end in a jump to copy_long and a copy under 4 KiB runs main's instructions plus one compare. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… there Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review, round 8: head abfe08b against main 15c26bfArchitecture. The fix belongs in libc, and that is where it is. libc owns userland's I checked the disassembly in Net change: about +150 lines of production code in The libc copy goes down on every x86-64 CPU over a 512-byte window. The kernel's copy uses a per-vendor rule: AMD below 32 bytes, Correctness (checked by reading the code)
Evidence
FindingsBLOCKER
SUGGESTION
Round 8's D3 / 256 KiB gap is excluded from this review, as instructed. SEND BACK (one blocker: the issue's exit for copies under 4 KiB) |
…hat ends Round 8 measured base copies at d = 16 against d = 0: 256 B 90 against 30 ns, 1 KiB 270 against 31, 2 KiB 511 against 40. They stall and are not routed. Exit C6 asks for them routed without any mem.memcpy row losing more than 10%, or a recorded decision to keep the stall. Review r8's suggestions: ROUTED_LEAST's comment no longer implies shorter lengths were measured clean, and the T14's 512 window is recorded as a choice within a taper. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ed one jumps Round 8 measured copies under 4 KiB stalling on the AMD at d = 16 (256 B 3x, 1 KiB 8.7x, 2 KiB 12.8x) and left unrouted. Routing them through round 8's out-of-line copy_long would have cost every such copy a call (4 KiB read 0.97 of main's). The distance compare now sits inline in copy_forward, so memcpy and memmove ask it for every copy of 256 bytes or more without a call, and only a copy at a short distance jumps to copy_short_alias, which keeps the 1 MiB cap and the overlap guard. A copy under 256 bytes is main's rep movsb behind one compare. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review, round 9: head 97ae8d6 (changes since abfe08b)Architecture. The shape is unchanged and still right: libc owns userland's Correctness (checked by reading the code and the disassembly)
Evidence
FindingsBLOCKER: none. SUGGESTION
LAND (no blocker. Round 9's metal judgement and the queued guest run are outside this review and remain the orchestrator's landing conditions.) |
…nd C6 about short copies The overlap test's `over` of 64 could not catch a missing guard: the loops load the first 64 bytes before storing, so an overlap of 64 or less is never read clobbered. 200 makes n 3800-4295; with the guard removed the test now first fails at gap 3600 n 3800. The issue no longer names a pending round as a measurement, and C6 judges only the mem.memcpy rows whose copies ask the distance (256 B, 1 KiB, 4 KiB), so the open 256 KiB question cannot fail it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… ruling on it Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…fer addresses For #1133 round 11: the copyat metal row runs osbench_copyat and osbench_mem, and copyat notes memcpy's address, its first rep movsb, and each placement's buffer addresses. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Closed by round 12's binding outcomeRound 12 ran twenty AMD boots in five rotated blocks of base B, head H, head with AVX off N, and base with
So by the rule this closes. The stall it measured stays real: AMD forward copies 1–31 bytes above their source modulo 4 KiB take 3× at 256 B, 22× at 8 KiB and 3.5× at 1 MiB (56305 rev. 3.02 §2.12). #1198 records the stall, this PR's fix and its unexplained 256 KiB cost as a finding, with an exit for a future approach. Branches kept for reference: |
Round 11 (the last bench round) found a regression in 5 of 5 pairs: the head's 256 KiB placement rows read 0.93 of base (0.80 in one pair). By the rule the drop is the PR's. Its cause is not yet found, and what follows is the orchestrator's decision.
Problem
A userland
memcpyof 8 KiB took 2615 ns on the AMD bench (Ryzen 7 4800U, Zen 2) against 90 ns, depending only on where the copy's two ends sit. #1121's IPC copy split first showed it.rep movsbfrom 4 to 64 KiB stalls at d = 8, 16, 24 and 31 and at no distance tried from 32 to 2048 (kernel: memcpy goes highest first where a forward copy would stall #1139's in-process rows).osbench copyat(libc'smemcpy) reads 2615 ns at 8 KiB and d = 16, 90 at every other distance, and stalls at d = 16 up to 64 MiB.copyat, with the ends 1 MiB + d apart, reads 769 ns at d = 16, 714 at 64 and 206 at 256, against 74 at d = 0. Intel documents 4K aliasing on bits 11:0. But the same bench read 124 ns for a heap pair 64 KiB + 16 apart and 136 ns for kernel: memcpy goes highest first where a forward copy would stall #1139's in-process copy at d = 16. So the window's extent on Tiger Lake is measured, but its mechanism is not established (recorded).Vecs exactly 16 apart. On the AMD that pair's copy took 2570 ns.Change
sdk/libc's x86-64copy_forward, inlined intomemcpyandmemmove's forward branch, returnsdest:rep movsbbehind one compare. In the disassembly that is main's instructions pluscmp rdx,0xff; jbeand one register move.rep movsb.copy_short_alias. It copies highest first where the ranges are disjoint and the copy is at most 1 MiB, so any store a load could match is 4 KiB minus the distance behind it. Overlapping ranges (which keeps an overlappingmemmovelowest first) and longer copies get onerep movsb.The loop:
The kernel's copy is #1139's, which also took the kernel issue this PR once carried. #1150's round 7 found that the paged read's user copies run at distance 0, so the kernel has no part in this.
Since the last review
memcpycalledcopy_forward. Round 8 moved the route out of line from 4 KiB: small copies recovered (0.985–0.99), but D3 still failed onmem.memcpy.262144(0.84). It also measured copies under 4 KiB stalling 3–13× at d16.ROUTED_LEAST's comment is corrected.Checks
cargo run -- --ci hostat 97ae8d6: 108 steps green, exit 0 (log).toyos-libc-copiesat 97ae8d6, x86-64 under Rosetta and arm64 natively: exit 0 each.gap 3600 n 3800, exit 101; restored. The tests pass at 7c67bbf, exit 0.copyat_runsat fb4ca4a (guest-rows run 42024430030, x86-64 QEMU with AVX): PASS, 5 steps green. That covers every routed copy, now from 256 B, checked against its source, and the placement rows running to exit 0. logcopyat_runschecked every routed copy against its source at round 5: PASS.ipc.reqrep.*0.99–1.00 on both benches.amd-memcpy-ab4; four boots base, head, head, base, all passed; rule, judgement):mem.memcpy.262144read 36905 MB/s against base's 40756, 0.906 against the 0.909 bound. Both pairs agree. That copy is at distance 0 and never routed, and its only new code is two compares, so the cause is unexplained. The row is on record as unstable on this bench (round 5's base 19390/29365, the scoreboard's 45% range). Small rows lose 5–7% (64 B 0.93, 256 B 0.93, 1 KiB 0.95), as round 5 measured: the hot path's cost, within the bound.rep movsb.ipc.reqrep.*1.00–1.01 in pair 1; base boot 4 was the slow one in pair 2.mem.memcpy.640.985,.2560.987,.10240.99..4096reads 0.97, because those copies takecopy_long's call.mem.memcpy.2621440.84. Copyat's unrouted 256 KiB d0 copy is 1.13× slower too, while 64 KiB and 1 MiB are equal. The 256 KiB copy runs about 15 instructions more before the samerep movsb, which cannot explain 1–1.5 µs. Unexplained.mem.memcpy.256,.1024and.4096read 0.98–0.99.ipcrow faster, the 8-byte row included.mem.memcpy.2621440.858. P fails: the placement spread (base 1.12, head 1.09) is under the gap, and head's median is 0.90 of base's. So the gap does not follow buffer placement. Reported, not judged: head's two boots differ (placement rows 0.97 and 0.81 of base), while base's agree.memcpyis byte-identical to B's but sits at the head's address, and head H; rule, judgement):mem.memcpy.262144.mem.memcpy.262144S reads 0.80 of B, lower than H; the clause failed because S fell below H's range.mem.memcpy.262144, in a separate process that never runs the AVX loop, reads 0.84.Why metal: the stall is in the CPU's load/store unit, and QEMU's timing cannot show it. Supported: the T14 and the AMD bench.
Evidence: rounds 1–6 at https://github.com/ToyOSOrg/evidence/tree/782346f21f39f6b2e87329546febaf433a8f7c2f/amd-memcpy, round 7 at https://github.com/ToyOSOrg/evidence/tree/459eb1230c2458f815dd74886eb14a5d6d1304d1/amd-memcpy/round-7, round 8 at https://github.com/ToyOSOrg/evidence/tree/675b23c48eeeeea932715455eca793b4284edb78/amd-memcpy/round-8, round 9 at https://github.com/ToyOSOrg/evidence/tree/dfd53b1e0d6f481c0812ad62ece2febc1c1d7e0b/amd-memcpy/round-9, round 10 at https://github.com/ToyOSOrg/evidence/tree/6d717e2ad8683be5d02947ddc57dd3576e40ada6/amd-memcpy/round-10.
Recorded limitations and risks
Recorded in
issues/a-user-copy-a-short-way-above-its-source-modulo-4-kib-still-costs.md(exit: round 5's C1, C3 and C4 as written; the 64 B outlier explained):mem.memcpy.262144on the AMD: 0.906, 0.84 and 0.858 in rounds 7–9 (D3). Round 9's placement test rules out buffer placement. About 6 extra instructions cannot explain 1 µs, so the cause is unexplained; code placement is untested.rep movsb's best (10× before). Where aligned stores leave the loads 16 bytes off a 32-byte boundary, the T14 pays for split loads.copyat's rows.mem.memcpy.64outlier boot is unexplained.🤖 Generated with Claude Code