Skip to content

Commit 789375c

Browse files
trialclaude
andcommitted
fix(plugin): review of 0.101.0 -- the adoption budget packs hour and count into one 64-bit cell per lane, so the hour rolls in a single CAS; sameDocument compares strings only; v0.101.0
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
1 parent ea67ce5 commit 789375c

4 files changed

Lines changed: 127 additions & 83 deletions

File tree

‎packages/plugin/src/util/entity.js‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -144,8 +144,10 @@ export const readEntity = async (key) => {
144144
* spells a slug raw while the sitemap — and so the Target key — percent-encodes it must not read as a new
145145
* canonical: that would adopt a duplicate target on every pass and flap the row against every render.
146146
*/
147-
export const sameDocument = (a, b) => a === b || (safeDecodeURI(a) ?? a) === (safeDecodeURI(b) ?? b);
147+
export const sameDocument = (a, b) =>
148+
typeof a === 'string' && typeof b === 'string' && (a === b || (safeDecodeURI(a) ?? a) === (safeDecodeURI(b) ?? b));
148149
const safeDecodeURI = (value) => {
150+
if (typeof value !== 'string') return null;
149151
try {
150152
return decodeURI(value);
151153
} catch {
@@ -284,15 +286,15 @@ const canonicalFromValue = (value, probedUrl) => {
284286
// documents on any worker), so a per-thread counter would multiply the cap by the thread count. Two lanes: real
285287
// adoptions, and dry-run ones, so a dry run measures what the cap would do without spending the slots an armed
286288
// node's real adoptions need (an operator's measure-only sweep on an armed node, for one).
287-
const BUDGET_SAB_KEY = 'entity_adopt_budget_v2';
289+
const BUDGET_SAB_KEY = 'entity_adopt_budget_v3';
288290
const REAL = 0;
289291
const DRY = 1;
290292
let budgetCell = null;
291-
const budgetI32 = () =>
292-
(budgetCell ??= new Int32Array(getSab(BUDGET_SAB_KEY, hourlyBudgetLength(2) * Int32Array.BYTES_PER_ELEMENT)));
293+
const budgetCells = () =>
294+
(budgetCell ??= new BigInt64Array(getSab(BUDGET_SAB_KEY, hourlyBudgetLength(2) * BigInt64Array.BYTES_PER_ELEMENT)));
293295

294296
/** The node's adoption budget (util/hourlyBudget.js): lane 0 real adoptions, lane 1 dry-run ones. */
295-
export const adoptionBudget = createHourlyBudget(budgetI32);
297+
export const adoptionBudget = createHourlyBudget(budgetCells);
296298
/** Tests: an empty budget. */
297299
export const resetAdoptionCountsForTest = () => adoptionBudget.reset();
298300

@@ -386,7 +388,7 @@ export const createCanonicalResolver = ({
386388
...(Number.isFinite(interval) && interval > 0 ? { renderInterval: interval } : {}),
387389
});
388390
} catch (e) {
389-
budget.release(REAL);
391+
budget.release(REAL, nowMs);
390392
logger.warn?.(`[prerender] entity: adopting ${observed.canonical} failed: ${e?.message ?? String(e)}`);
391393
return decided(AdoptOutcome.ERROR);
392394
}

‎packages/plugin/src/util/hourlyBudget.js‎

Lines changed: 37 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -1,62 +1,63 @@
11
/**
2-
* AN HOURLY BUDGET SHARED BY EVERY WORKER THREAD ON A NODE, over an Int32Array the caller hands in (a shared
2+
* AN HOURLY BUDGET SHARED BY EVERY WORKER THREAD ON A NODE, over a BigInt64Array the caller hands in (a shared
33
* buffer in production, util/coordination.js `getSab`). No Harper dependency, so it can be tested across real
44
* worker threads.
55
*
6-
* Layout: [hourSinceEpoch, spent in lane 0, spent in lane 1, ...]. Hours since the epoch fit an int32 for a
7-
* quarter of a million years. Each LANE is its own count against the same limit: the entity registry spends
8-
* real adoptions in one and dry-run adoptions in the other, so a dry run measures what the cap would do without
9-
* spending the slots a real adoption needs.
6+
* ONE 64-BIT CELL PER LANE, holding the hour and the count together: `hourSinceEpoch << 32 | spent`. Every
7+
* reserve and release is a single compare-and-swap of that cell, so a thread can never see a new hour with the
8+
* old hour's count, or the old hour's count reset under it: there is no window between rolling the hour and
9+
* zeroing the count, because they are one write. (Hours since the epoch fit 32 bits for 490,000 years.) Each LANE
10+
* is its own count against the same limit: the entity registry spends real adoptions in one and dry-run adoptions
11+
* in the other, so a dry run measures what the cap would do without spending the slots a real adoption needs.
1012
*
11-
* The window rolls forward only, and whoever wins the roll zeroes every lane — the shape of
12-
* util/invalidationReenqueue.js `reserveSlot`, for the same reasons: a straggler carrying an older hour must not
13-
* zero a count the new hour has spent, and a loser of a race costs or grants at most one slot. This bounds load
14-
* from a burst; it is not a safety property.
13+
* The hour only moves forward: a straggler carrying an older hour is refused rather than allowed to roll the cell
14+
* back and hand out the spent hour's budget again. This bounds load from a burst; it is not a safety property.
1515
*/
1616

1717
const HOUR_MS = 3_600_000;
18-
const B_HOUR = 0;
18+
const COUNT_BITS = 32n;
19+
const COUNT_MASK = (1n << COUNT_BITS) - 1n;
1920

20-
/** The Int32Array length a budget with `lanes` lanes needs. */
21-
export const hourlyBudgetLength = (lanes) => 1 + lanes;
21+
/** The BigInt64Array length a budget with `lanes` lanes needs. */
22+
export const hourlyBudgetLength = (lanes) => lanes;
23+
24+
const hourOf = (cell) => Number(cell >> COUNT_BITS);
25+
const countOf = (cell) => Number(cell & COUNT_MASK);
26+
const pack = (hour, count) => (BigInt(hour) << COUNT_BITS) | BigInt(count);
2227

2328
/**
24-
* A budget over `cell()` (called on every use, so the caller can allocate lazily). `reserve(limit, nowMs, lane)`
25-
* takes a slot in `lane` this hour, false past `limit`; `release(lane)` returns one.
29+
* A budget over `cells()` (called on every use, so the caller can allocate lazily). `reserve(limit, nowMs, lane)`
30+
* takes a slot in `lane` this hour, false past `limit`; `release(lane, nowMs)` returns one taken this hour.
2631
*/
27-
export const createHourlyBudget = (cell) => {
28-
const roll = (i32, nowMs) => {
29-
const hour = Math.floor(nowMs / HOUR_MS);
30-
const observed = Atomics.load(i32, B_HOUR);
31-
if (hour > observed && Atomics.compareExchange(i32, B_HOUR, observed, hour) === observed) {
32-
for (let lane = 1; lane < i32.length; lane++) Atomics.store(i32, lane, 0);
33-
}
34-
};
35-
return Object.freeze({
32+
export const createHourlyBudget = (cells) =>
33+
Object.freeze({
3634
reserve(limit, nowMs = Date.now(), lane = 0) {
37-
const i32 = cell();
38-
roll(i32, nowMs);
39-
const at = 1 + lane;
35+
const i64 = cells();
36+
const hour = Math.floor(nowMs / HOUR_MS);
4037
for (let attempt = 0; attempt < 8; attempt++) {
41-
const used = Atomics.load(i32, at);
38+
const seen = Atomics.load(i64, lane);
39+
const seenHour = hourOf(seen);
40+
if (hour < seenHour) return false; // a clock behind the cell: refuse, never roll back
41+
const used = hour === seenHour ? countOf(seen) : 0;
4242
if (used >= limit) return false;
43-
if (Atomics.compareExchange(i32, at, used, used + 1) === used) return true;
43+
if (Atomics.compareExchange(i64, lane, seen, pack(hour, used + 1)) === seen) return true;
4444
}
4545
// Eight lost races: other workers are spending the budget right now. Refusing is the safe direction.
4646
return false;
4747
},
48-
release(lane = 0) {
49-
const i32 = cell();
50-
const at = 1 + lane;
48+
release(lane = 0, nowMs = Date.now()) {
49+
const i64 = cells();
50+
const hour = Math.floor(nowMs / HOUR_MS);
5151
for (let attempt = 0; attempt < 8; attempt++) {
52-
const used = Atomics.load(i32, at);
53-
if (used <= 0 || Atomics.compareExchange(i32, at, used, used - 1) === used) return;
52+
const seen = Atomics.load(i64, lane);
53+
// A slot from an hour that has since rolled is already gone with it.
54+
if (hourOf(seen) !== hour || countOf(seen) === 0) return;
55+
if (Atomics.compareExchange(i64, lane, seen, seen - 1n) === seen) return;
5456
}
5557
},
5658
/** Tests: an empty budget. */
5759
reset() {
58-
const i32 = cell();
59-
for (let i = 0; i < i32.length; i++) Atomics.store(i32, i, 0);
60+
const i64 = cells();
61+
for (let i = 0; i < i64.length; i++) Atomics.store(i64, i, 0n);
6062
},
6163
});
62-
};

‎packages/plugin/test/entity.test.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -503,3 +503,10 @@ test('retryAfter is per ENTITY: two spellings naming each other cannot reactivat
503503
assert.equal((await watch({ url: CANON, value: OLD })).adopt, 'recent');
504504
assert.equal(filed.length, 1);
505505
});
506+
507+
test('sameDocument compares two URLs, never a coerced non-string', () => {
508+
assert.equal(entity.sameDocument(null, 'null'), false);
509+
assert.equal(entity.sameDocument(undefined, 'undefined'), false);
510+
assert.equal(entity.sameDocument(null, null), false);
511+
assert.equal(entity.sameDocument(`${PRODUCT}levi%27s.jsp`, `${PRODUCT}levi's.jsp`), true);
512+
});

‎packages/plugin/test/hourlyBudget.test.js‎

Lines changed: 75 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -4,59 +4,93 @@ import { Worker } from 'node:worker_threads';
44
import { createHourlyBudget, hourlyBudgetLength } from '../src/util/hourlyBudget.js';
55

66
/**
7-
* The hourly budget (util/hourlyBudget.js) the entity registry's adoptions spend: one count per lane per hour,
8-
* shared by every worker thread through a shared buffer.
7+
* The hourly budget (util/hourlyBudget.js) the entity registry's adoptions spend: one 64-bit cell per lane,
8+
* holding the hour and the count, shared by every worker thread through a shared buffer.
99
*/
1010

1111
const MODULE = new URL('../src/util/hourlyBudget.js', import.meta.url).href;
12+
const HOUR = 3_600_000;
1213
const NOW = Date.UTC(2026, 9, 2, 10, 30, 0);
14+
const cells = (lanes = 2) =>
15+
new BigInt64Array(new SharedArrayBuffer(hourlyBudgetLength(lanes) * BigInt64Array.BYTES_PER_ELEMENT));
1316

14-
test('across real worker threads the limit holds exactly: no thread multiplies it', async () => {
15-
const shared = new Int32Array(new SharedArrayBuffer(hourlyBudgetLength(2) * Int32Array.BYTES_PER_ELEMENT));
16-
const workers = 4;
17-
const attempts = 200;
18-
const limit = 150;
19-
const granted = await Promise.all(
20-
Array.from(
21-
{ length: workers },
22-
() =>
23-
new Promise((resolve, reject) => {
24-
const worker = new Worker(
25-
`const { parentPort, workerData } = require('node:worker_threads');
26-
import(workerData.module).then(({ createHourlyBudget }) => {
27-
const budget = createHourlyBudget(() => workerData.cell);
28-
let granted = 0;
29-
for (let i = 0; i < workerData.attempts; i++) if (budget.reserve(workerData.limit, workerData.now, 0)) granted++;
30-
parentPort.postMessage(granted);
31-
});`,
32-
{ eval: true, workerData: { module: MODULE, cell: shared, attempts, limit, now: NOW } }
33-
);
34-
worker.once('message', resolve);
35-
worker.once('error', reject);
36-
})
37-
)
17+
// Four real threads, released together from a gate, each try `attempts` reserves against `limit`. With `rollAt`, a
18+
// thread's attempts from that index on are made an hour later.
19+
const race = async (shared, { attempts, limit, rollAt = null }) => {
20+
const gate = new Int32Array(new SharedArrayBuffer(8)); // [ready, go]
21+
const runs = Array.from(
22+
{ length: 4 },
23+
() =>
24+
new Promise((resolve, reject) => {
25+
const worker = new Worker(
26+
`const { parentPort, workerData } = require('node:worker_threads');
27+
import(workerData.module).then(({ createHourlyBudget }) => {
28+
const budget = createHourlyBudget(() => workerData.cells);
29+
Atomics.add(workerData.gate, 0, 1);
30+
Atomics.wait(workerData.gate, 1, 0);
31+
const granted = [0, 0];
32+
for (let i = 0; i < workerData.attempts; i++) {
33+
const late = workerData.rollAt !== null && i >= workerData.rollAt;
34+
if (budget.reserve(workerData.limit, workerData.now + (late ? workerData.hour : 0), 0)) granted[late ? 1 : 0]++;
35+
}
36+
parentPort.postMessage(granted);
37+
});`,
38+
{
39+
eval: true,
40+
workerData: { module: MODULE, cells: shared, gate, attempts, limit, rollAt, now: NOW, hour: HOUR },
41+
}
42+
);
43+
worker.once('message', resolve);
44+
worker.once('error', reject);
45+
})
3846
);
47+
while (Atomics.load(gate, 0) < 4) await new Promise((r) => setTimeout(r, 1));
48+
Atomics.store(gate, 1, 1);
49+
Atomics.notify(gate, 1);
50+
return Promise.all(runs);
51+
};
52+
53+
test('across real worker threads the limit holds exactly: no thread multiplies it', async () => {
54+
const shared = cells();
55+
// 200,000 attempts for a limit of 150,000: headroom for the reserves contention refuses.
56+
const granted = await race(shared, { attempts: 50_000, limit: 150_000 });
3957
assert.equal(
40-
granted.reduce((a, b) => a + b, 0),
41-
limit,
42-
`granted ${granted.join(' + ')} across ${workers} threads`
58+
granted.reduce((a, [now]) => a + now, 0),
59+
150_000,
60+
`granted ${granted.map(([n]) => n).join(' + ')} across 4 threads`
4361
);
44-
assert.equal(shared[2], 0, 'the other lane is untouched');
62+
assert.equal(shared[1], 0n, 'the other lane is untouched');
4563
});
4664

47-
test('lanes are independent, a release returns a slot, and the next hour starts afresh', () => {
48-
const cell = new Int32Array(new SharedArrayBuffer(hourlyBudgetLength(2) * Int32Array.BYTES_PER_ELEMENT));
49-
const budget = createHourlyBudget(() => cell);
65+
test('the hour rolls under contention without handing out more than one hour’s budget per hour', async () => {
66+
// Every thread crosses into the next hour mid-run, at its own pace. The first hour grants at most its limit (a
67+
// thread still in it once another has rolled the cell is a straggler, and refused); the next hour grants exactly
68+
// its limit: no stale count from the first hour refuses it (the race a separate hour word and count word had).
69+
// Each hour has well over its limit in attempts: a reserve that loses eight races in a row is refused (the safe
70+
// direction), so "in full" needs headroom.
71+
const granted = await race(cells(), { attempts: 50_000, limit: 40_000, rollAt: 20_000 });
72+
const first = granted.reduce((a, [n]) => a + n, 0);
73+
const second = granted.reduce((a, [, n]) => a + n, 0);
74+
assert.ok(first <= 40_000, `the first hour granted ${first}`);
75+
assert.equal(second, 40_000, 'the next hour, in full');
76+
});
77+
78+
test('lanes are independent, a release returns a slot this hour only, and the hour never moves back', () => {
79+
const shared = cells();
80+
const budget = createHourlyBudget(() => shared);
5081
assert.equal(budget.reserve(1, NOW, 0), true);
5182
assert.equal(budget.reserve(1, NOW, 0), false);
5283
assert.equal(budget.reserve(1, NOW, 1), true, 'lane 1 has its own count');
53-
budget.release(0);
84+
budget.release(0, NOW);
5485
assert.equal(budget.reserve(1, NOW, 0), true, 'a released slot is spent again');
55-
assert.equal(budget.reserve(1, NOW + 3_600_000, 0), true, 'a new hour');
56-
assert.equal(budget.reserve(1, NOW + 3_600_000, 1), true, 'every lane rolls');
57-
// A straggler carrying the old hour does not zero what the new hour has spent.
58-
assert.equal(budget.reserve(1, NOW, 0), false);
59-
budget.release(0);
60-
budget.release(0);
61-
assert.equal(cell[1], 0, 'a release never goes below zero');
86+
assert.equal(budget.reserve(1, NOW + HOUR, 0), true, 'a new hour');
87+
assert.equal(budget.reserve(1, NOW + HOUR, 1), true, 'every lane rolls on its own first use');
88+
assert.equal(budget.reserve(1, NOW, 0), false, 'a straggler carrying the old hour is refused, not rolled back');
89+
budget.release(0, NOW);
90+
assert.equal(budget.reserve(1, NOW + HOUR, 0), false, 'a release for an hour already gone returns nothing');
91+
budget.release(0, NOW + HOUR);
92+
budget.release(0, NOW + HOUR);
93+
assert.equal(budget.reserve(2, NOW + HOUR, 0), true, 'a release never goes below zero');
94+
assert.equal(budget.reserve(2, NOW + HOUR, 0), true);
95+
assert.equal(budget.reserve(2, NOW + HOUR, 0), false);
6296
});

0 commit comments

Comments
 (0)