The three the review asked for: the last bare retry, and a bound that was a typical
[flan_dev_reg_at] still went straight round its walk-level retries — a failed scan_open and a failed scan_ok both back to the top. It is the one site where the argument for leaving it holds water, since the agent gates the verb behind a stopped program and there is no writer to lose to. It loses to the other argument: it is the same mistake the slot read was making, three lines under the note explaining why it is a mistake, and a bare retry left next to that note teaches that the rule has exceptions it does not have. And the cost of a refusal was written as a bound when it is a typical. "One writer, so at most one slot of a walk is odd" is true at any instant and false across a walk — flan_reg_compact writes every slot under its own counter, so a repeatedly preempted writer can charge the full patience against several slots of one walk, 4096 of them in the arithmetic worst case. Both docs now say typical and give the worst, and record what the larger figure means for request_lock, which is held across handle_line: a refusing listing delays an abort by that much. Nothing depended on the old number. Also written down, because it is the clearest statement of why this was not a timeout: sixty-four bare re-reads of one word finish in about two microseconds, so against a writer a millisecond from running the old budget was not short, it was zero wall-clock.
This commit is contained in:
parent
a29424a7b6
commit
df2f23609b
42
FIX.org
42
FIX.org
@ -5627,6 +5627,10 @@ test_reload went 55 for 55.
|
|||||||
|
|
||||||
Machine: 16 cores, load average 15-18 through the run.
|
Machine: 16 cores, load average 15-18 through the run.
|
||||||
|
|
||||||
|
And independently, by the reviewer, on their own machine and harness: 9 of 72
|
||||||
|
refused before under 24-way contention, 0 of 120 after, test_reload.exe 20 for
|
||||||
|
20, and 10 for 10 again on the rebased merge.
|
||||||
|
|
||||||
** It also hardens the address root, which was the same spin
|
** It also hardens the address root, which was the same spin
|
||||||
[flan_dev_reg_at] — the inspector's "what block is this address in" — calls
|
[flan_dev_reg_at] — the inspector's "what block is this address in" — calls
|
||||||
the same [flan_reg_snap] and, unlike the listing, steps *past* a slot it could
|
the same [flan_reg_snap] and, unlike the listing, steps *past* a slot it could
|
||||||
@ -5639,6 +5643,44 @@ was not in the table. Nothing was seen failing that way; it is named because
|
|||||||
the fix closes it and the next person should not have to rediscover that this
|
the fix closes it and the next person should not have to rediscover that this
|
||||||
verb shared the defect.
|
verb shared the defect.
|
||||||
|
|
||||||
|
Review caught that the same function still went bare round its *walk*-level
|
||||||
|
retries after that — a failed [flan_reg_scan_open] and a failed
|
||||||
|
[flan_reg_scan_ok] both went straight back to the top. Both wait now. It is
|
||||||
|
the one site where contention is near-impossible, because the agent gates this
|
||||||
|
verb behind a stopped program (vendor/agent/flan_agent.c), so the argument for
|
||||||
|
leaving it is that it cannot matter. The argument against, which won: it is
|
||||||
|
the same mistake the slot read was making, sitting three lines under the note
|
||||||
|
that explains why it is a mistake, and a bare retry left in place next to that
|
||||||
|
note teaches the next reader that the rule has exceptions it does not have.
|
||||||
|
|
||||||
|
** What a refusal costs now, typical and worst, because they are far apart
|
||||||
|
The first pass through this wrote the new cost as "about sixty-six
|
||||||
|
milliseconds" and called it a bound. It is the typical case and not a bound,
|
||||||
|
and the difference is worth having right. The reasoning behind the number is
|
||||||
|
"one writer, so at most one slot of a walk is odd", which is true at any
|
||||||
|
instant and false across a walk: [flan_reg_compact] writes every slot under
|
||||||
|
its own counter, so a writer the scheduler keeps preempting can charge the
|
||||||
|
full 8ms of patience against several slots of the same walk. The arithmetic
|
||||||
|
worst case is 4096 of them. Nothing observed comes near it, and a walk paying
|
||||||
|
it is a walk about to refuse anyway, but the docs now say typical and give the
|
||||||
|
worst rather than presenting one as the other.
|
||||||
|
|
||||||
|
One consequence of the larger figure, traced in review and recorded rather
|
||||||
|
than changed: flan_agent.c holds [request_lock] across [handle_line], so a
|
||||||
|
refusing listing serialises other agent requests — [abort] among them — behind
|
||||||
|
~66ms where it used to be ~2ms. Nothing depends on the old number.
|
||||||
|
[over_socket] reads to close with no deadline, [await] defaults to 5000ms, and
|
||||||
|
[serve]'s two-second timeout is on the read and not on the handling. Written
|
||||||
|
down so the next person does not have to derive it.
|
||||||
|
|
||||||
|
** The sixty-four were not a short budget; there was no timeout at all
|
||||||
|
Also from review, and the sharpest way to say why this is a fix and not a
|
||||||
|
widened timeout. Sixty-four bare re-reads of a single word finish in about two
|
||||||
|
microseconds. Against a writer that will not be scheduled again for a
|
||||||
|
millisecond, the old budget was not small — it was zero wall-clock. There was
|
||||||
|
no timeout in that code to widen. What was added is the first wall-clock
|
||||||
|
patience the slot read ever had.
|
||||||
|
|
||||||
** Not the dev-pause flake, and not the initialiser one either
|
** Not the dev-pause flake, and not the initialiser one either
|
||||||
Two other dev flakes were on the table while this was chased. Neither is this.
|
Two other dev flakes were on the table while this was chased. Neither is this.
|
||||||
|
|
||||||
|
|||||||
@ -5341,10 +5341,20 @@ looking is what keeps the core busy. So the reader burned its whole budget insid
|
|||||||
perfectly readable slot unreadable, and the listing above it refused a table that never moved at all. It showed up
|
perfectly readable slot unreadable, and the listing above it refused a table that never moved at all. It showed up
|
||||||
only under load, which is exactly when a writer gets descheduled: no refusals at all on an idle machine, and one run in
|
only under load, which is exactly when a writer gets descheduled: no refusals at all on an idle machine, and one run in
|
||||||
five when the cores were oversubscribed. The retry is now eight bare looks and then the same quarter-millisecond step,
|
five when the cores were oversubscribed. The retry is now eight bare looks and then the same quarter-millisecond step,
|
||||||
forty tries in all, so a refusal means the table really would not hold still. That moves the bound on a refusal from
|
forty tries in all, so a refusal means the table really would not hold still.
|
||||||
two milliseconds to about sixty-six -- eight milliseconds of patience per contended slot, once per walk because there
|
|
||||||
is one writer, across eight walks -- which is still far under what a person waiting for a keypress to be answered
|
**What that costs, said as a typical and a worst case, because they are far apart.** A refusal used to be bounded at
|
||||||
notices, and is only ever paid by a listing that is about to refuse anyway. See FIX.org, 2026-09-21.
|
two milliseconds. It is not bounded any more in the arithmetic sense: eight milliseconds of patience per contended
|
||||||
|
slot, and although only one slot is odd at any instant -- there is one writer -- a compaction writes *every* slot
|
||||||
|
under its own counter, so a writer the scheduler keeps preempting can charge that against several slots of the same
|
||||||
|
walk. The worst case is 4096 of them per walk, which is not a number to design around; the typical case, and every
|
||||||
|
case measured, is one contended slot per walk and eight walks, so about sixty-six milliseconds. That is still far
|
||||||
|
under what a person waiting for a keypress to be answered notices, and it is only ever paid by a listing that is about
|
||||||
|
to refuse anyway. One consequence to know rather than to fix: `flan_agent.c` holds `request_lock` across
|
||||||
|
`handle_line`, so a refusing listing now delays other agent requests -- `abort` among them -- by that much. Nothing
|
||||||
|
depends on the old figure (`over_socket` reads to close with no deadline, `await` defaults to 5000ms, and `serve`'s
|
||||||
|
two-second timeout is on the read rather than on the handling), but a reader of that code should not have to work it
|
||||||
|
out again. See FIX.org, 2026-09-21.
|
||||||
|
|
||||||
**"At exit" is not a hook, and the honest reason is that a game is killed.** A program stopped by a signal runs no
|
**"At exit" is not a hook, and the honest reason is that a game is killed.** A program stopped by a signal runs no
|
||||||
`atexit` handler, no destructor, nothing — so no code written inside the program could report anything about the run
|
`atexit` handler, no destructor, nothing — so no code written inside the program could report anything about the run
|
||||||
|
|||||||
@ -1247,6 +1247,10 @@ static void flan_reg_wait(void) {
|
|||||||
* table that was perfectly readable and never moved. It showed up only under
|
* table that was perfectly readable and never moved. It showed up only under
|
||||||
* load, which is exactly when a writer gets descheduled — no refusals at all
|
* load, which is exactly when a writer gets descheduled — no refusals at all
|
||||||
* on an idle machine, and one run in five when the cores were oversubscribed.
|
* on an idle machine, and one run in five when the cores were oversubscribed.
|
||||||
|
* The sixty-four were not a short budget, which is the part worth being exact
|
||||||
|
* about: sixty-four bare re-reads of one word finish in about two microseconds,
|
||||||
|
* so against a writer that will not run again for a millisecond the budget was
|
||||||
|
* not small, it was zero wall-clock. There was no timeout to widen.
|
||||||
* See FIX.org, 2026-09-21. */
|
* See FIX.org, 2026-09-21. */
|
||||||
#define FLAN_REG_SPINS 8
|
#define FLAN_REG_SPINS 8
|
||||||
#define FLAN_REG_TRIES 40 /* 8 spins, then 32 waits: ~8ms of patience */
|
#define FLAN_REG_TRIES 40 /* 8 spins, then 32 waits: ~8ms of patience */
|
||||||
@ -1269,9 +1273,14 @@ static int flan_reg_snap(flan_reg_entry *e, flan_reg_entry *out) {
|
|||||||
for (attempt = 0; attempt < FLAN_REG_TRIES; attempt++) {
|
for (attempt = 0; attempt < FLAN_REG_TRIES; attempt++) {
|
||||||
uint64_t g1;
|
uint64_t g1;
|
||||||
/* The wait is only on the path that found a write in flight, so a table
|
/* The wait is only on the path that found a write in flight, so a table
|
||||||
nobody is writing pays nothing for it. One writer means at most one slot
|
nobody is writing pays nothing for it. There is one writer, so only one
|
||||||
of a walk is odd at a time, and that slot is almost always readable on
|
slot is odd at any instant and it is almost always readable on the first
|
||||||
the first look after the first sleep. */
|
look after the first sleep — which is what a walk typically costs, and
|
||||||
|
is not a bound. A compaction writes every slot under its own counter, so
|
||||||
|
a writer the scheduler keeps taking the core from can charge the full
|
||||||
|
8ms against several slots of one walk; the arithmetic worst case is
|
||||||
|
4096 of them. Nothing observed comes near that, and a walk that is
|
||||||
|
paying it is a walk that is about to refuse anyway. */
|
||||||
if (attempt >= FLAN_REG_SPINS) flan_reg_wait();
|
if (attempt >= FLAN_REG_SPINS) flan_reg_wait();
|
||||||
g1 = __atomic_load_n(&e->gen, __ATOMIC_ACQUIRE);
|
g1 = __atomic_load_n(&e->gen, __ATOMIC_ACQUIRE);
|
||||||
if (g1 & 1) continue; /* a write is in progress */
|
if (g1 & 1) continue; /* a write is in progress */
|
||||||
@ -1669,11 +1678,19 @@ int32_t flan_dev_reg_at(const void *p, const char **type, int64_t *typelen,
|
|||||||
flan_reg_entry best, cur;
|
flan_reg_entry best, cur;
|
||||||
int have = 0, attempt;
|
int have = 0, attempt;
|
||||||
if (!flan_reg_on || a == 0) return 0;
|
if (!flan_reg_on || a == 0) return 0;
|
||||||
|
/* The walk-level retries wait between themselves, for [flan_reg_wait]'s
|
||||||
|
reason and not for this verb's own risk: the agent gates this one behind a
|
||||||
|
stopped program, so the writer is parked and there is usually nothing to
|
||||||
|
lose to. Going straight round again would still be the same mistake the
|
||||||
|
slot read was making — eight walks that all fit inside the one
|
||||||
|
rearrangement they are all losing to — and leaving one bare retry in the
|
||||||
|
file next to the note explaining why they are wrong is how the next
|
||||||
|
person learns the rule has exceptions it does not have. */
|
||||||
for (attempt = 0; attempt < 8; attempt++) {
|
for (attempt = 0; attempt < 8; attempt++) {
|
||||||
uint64_t at;
|
uint64_t at;
|
||||||
int64_t i;
|
int64_t i;
|
||||||
have = 0;
|
have = 0;
|
||||||
if (!flan_reg_scan_open(&at)) continue;
|
if (!flan_reg_scan_open(&at)) { flan_reg_wait(); continue; }
|
||||||
for (i = 0; i < FLAN_REG_CAP; i++) {
|
for (i = 0; i < FLAN_REG_CAP; i++) {
|
||||||
if (!flan_reg_snap(&flan_reg[i], &cur)) continue;
|
if (!flan_reg_snap(&flan_reg[i], &cur)) continue;
|
||||||
if (cur.base == 0) continue;
|
if (cur.base == 0) continue;
|
||||||
@ -1684,6 +1701,7 @@ int32_t flan_dev_reg_at(const void *p, const char **type, int64_t *typelen,
|
|||||||
}
|
}
|
||||||
if (flan_reg_scan_ok(at)) break;
|
if (flan_reg_scan_ok(at)) break;
|
||||||
have = 0;
|
have = 0;
|
||||||
|
flan_reg_wait();
|
||||||
}
|
}
|
||||||
if (!have) return 0;
|
if (!have) return 0;
|
||||||
if (type) *type = best.type;
|
if (type) *type = best.type;
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user