Skip to content

Commit 5f34537

Browse files
ntuckerntucker
committed
fix bench measurements
1 parent 6b15581 commit 5f34537

5 files changed

Lines changed: 137 additions & 50 deletions

File tree

‎examples/benchmark-react/bench/validate.ts‎

Lines changed: 82 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,88 @@ test(
461461
{ onlyLibs: ['data-client'] },
462462
);
463463

464+
// ── TIMING VALIDATION ────────────────────────────────────────────────
465+
// Verify that when data-bench-complete fires (measurement ends), the DOM
466+
// already reflects the update. A 100ms network delay makes timing bugs
467+
// observable: if the measureUpdate callback doesn't return its promise
468+
// chain, finish() fires via the sync-path double-rAF (~32ms) before the
469+
// async fetch resolves. data-client passes because controller.fetch()
470+
// dispatches optimistic updates to the store synchronously.
471+
472+
test('updateEntity timing: DOM reflects change at measurement end', async (page, lib) => {
473+
await initAndWaitForItems(page);
474+
await page.evaluate(() => window.__BENCH__!.setNetworkDelay(100));
475+
476+
await clearComplete(page);
477+
await page.evaluate(() => window.__BENCH__!.updateEntity('item-0'));
478+
await waitForComplete(page);
479+
480+
const labels = await getItemLabels(page);
481+
assert(
482+
labels['item-0']?.includes('(updated)') ?? false,
483+
lib,
484+
'updateEntity timing',
485+
`DOM not updated when data-bench-complete fired. ` +
486+
`Ensure measureUpdate callback returns its promise chain.`,
487+
);
488+
489+
await page.evaluate(() => window.__BENCH__!.setNetworkDelay(0));
490+
});
491+
492+
test('createEntity timing: DOM reflects change at measurement end', async (page, lib) => {
493+
if (
494+
!(await page.evaluate(
495+
() => typeof window.__BENCH__?.createEntity === 'function',
496+
))
497+
)
498+
return;
499+
500+
await initAndWaitForItems(page, 10);
501+
await page.evaluate(() => window.__BENCH__!.setNetworkDelay(100));
502+
503+
await clearComplete(page);
504+
await page.evaluate(() => window.__BENCH__!.createEntity!());
505+
await waitForComplete(page);
506+
507+
const labels = await getItemLabels(page);
508+
assert(
509+
Object.values(labels).some(l => l === 'New Item'),
510+
lib,
511+
'createEntity timing',
512+
`"New Item" not in DOM when data-bench-complete fired. ` +
513+
`Ensure measureUpdate callback returns its promise chain.`,
514+
);
515+
516+
await page.evaluate(() => window.__BENCH__!.setNetworkDelay(0));
517+
});
518+
519+
test('deleteEntity timing: DOM reflects change at measurement end', async (page, lib) => {
520+
if (
521+
!(await page.evaluate(
522+
() => typeof window.__BENCH__?.deleteEntity === 'function',
523+
))
524+
)
525+
return;
526+
527+
await initAndWaitForItems(page, 10);
528+
await page.evaluate(() => window.__BENCH__!.setNetworkDelay(100));
529+
530+
await clearComplete(page);
531+
await page.evaluate(() => window.__BENCH__!.deleteEntity!('item-0'));
532+
await waitForComplete(page);
533+
534+
const labels = await getItemLabels(page);
535+
assert(
536+
!('item-0' in labels),
537+
lib,
538+
'deleteEntity timing',
539+
`item-0 still in DOM when data-bench-complete fired. ` +
540+
`Ensure measureUpdate callback returns its promise chain.`,
541+
);
542+
543+
await page.evaluate(() => window.__BENCH__!.setNetworkDelay(0));
544+
});
545+
464546
// ═══════════════════════════════════════════════════════════════════════════
465547
// Runner
466548
// ═══════════════════════════════════════════════════════════════════════════

‎examples/benchmark-react/src/baseline/index.tsx‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -86,13 +86,11 @@ function BenchmarkHarness() {
8686
(id: string) => {
8787
const item = FIXTURE_ITEMS_BY_ID.get(id);
8888
if (!item) return;
89-
measureUpdate(() => {
89+
measureUpdate(() =>
9090
ItemResource.update({ id }, { label: `${item.label} (updated)` }).then(
91-
() => {
92-
ItemResource.getList({ count: listViewCount! }).then(setItems);
93-
},
94-
);
95-
});
91+
() => ItemResource.getList({ count: listViewCount! }).then(setItems),
92+
),
93+
);
9694
},
9795
[measureUpdate, listViewCount],
9896
);
@@ -115,20 +113,20 @@ function BenchmarkHarness() {
115113

116114
const createEntity = useCallback(() => {
117115
const author = FIXTURE_AUTHORS[0];
118-
measureUpdate(() => {
119-
ItemResource.create({ label: 'New Item', author }).then(() => {
120-
ItemResource.getList({ count: listViewCount! }).then(setItems);
121-
});
122-
});
116+
measureUpdate(() =>
117+
ItemResource.create({ label: 'New Item', author }).then(() =>
118+
ItemResource.getList({ count: listViewCount! }).then(setItems),
119+
),
120+
);
123121
}, [measureUpdate, listViewCount]);
124122

125123
const deleteEntity = useCallback(
126124
(id: string) => {
127-
measureUpdate(() => {
128-
ItemResource.delete({ id }).then(() => {
129-
ItemResource.getList({ count: listViewCount! }).then(setItems);
130-
});
131-
});
125+
measureUpdate(() =>
126+
ItemResource.delete({ id }).then(() =>
127+
ItemResource.getList({ count: listViewCount! }).then(setItems),
128+
),
129+
);
132130
},
133131
[measureUpdate, listViewCount],
134132
);

‎examples/benchmark-react/src/shared/benchHarness.tsx‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,16 @@ export function useBenchState() {
6868
[setComplete],
6969
);
7070

71+
/**
72+
* Measure an update action. If the callback performs async work (fetch →
73+
* setState), it MUST return the promise chain so finish() runs after the
74+
* state update, not before. Sync dispatch (data-client's controller.fetch)
75+
* legitimately returns void — the store update is synchronous.
76+
*
77+
* The timing-validation tests in validate.ts enforce this contract by
78+
* injecting network delay and checking that the DOM is already updated
79+
* when data-bench-complete fires.
80+
*/
7181
const measureUpdate = useCallback(
7282
(fn: () => void | Promise<void>) => {
7383
performance.mark('update-start');

‎examples/benchmark-react/src/swr/index.tsx‎

Lines changed: 14 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -74,13 +74,11 @@ function BenchmarkHarness() {
7474
(id: string) => {
7575
const item = FIXTURE_ITEMS_BY_ID.get(id);
7676
if (!item) return;
77-
measureUpdate(() => {
77+
measureUpdate(() =>
7878
ItemResource.update({ id }, { label: `${item.label} (updated)` }).then(
79-
() => {
80-
void mutate(`items:${listViewCount}`);
81-
},
82-
);
83-
});
79+
() => mutate(`items:${listViewCount}`),
80+
),
81+
);
8482
},
8583
[measureUpdate, mutate, listViewCount],
8684
);
@@ -102,20 +100,20 @@ function BenchmarkHarness() {
102100

103101
const createEntity = useCallback(() => {
104102
const author = FIXTURE_AUTHORS[0];
105-
measureUpdate(() => {
106-
ItemResource.create({ label: 'New Item', author }).then(() => {
107-
void mutate(`items:${listViewCount}`);
108-
});
109-
});
103+
measureUpdate(() =>
104+
ItemResource.create({ label: 'New Item', author }).then(() =>
105+
mutate(`items:${listViewCount}`),
106+
),
107+
);
110108
}, [measureUpdate, mutate, listViewCount]);
111109

112110
const deleteEntity = useCallback(
113111
(id: string) => {
114-
measureUpdate(() => {
115-
ItemResource.delete({ id }).then(() => {
116-
void mutate(`items:${listViewCount}`);
117-
});
118-
});
112+
measureUpdate(() =>
113+
ItemResource.delete({ id }).then(() =>
114+
mutate(`items:${listViewCount}`),
115+
),
116+
);
119117
},
120118
[measureUpdate, mutate, listViewCount],
121119
);

‎examples/benchmark-react/src/tanstack-query/index.tsx‎

Lines changed: 17 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -97,15 +97,14 @@ function BenchmarkHarness() {
9797
(id: string) => {
9898
const item = FIXTURE_ITEMS_BY_ID.get(id);
9999
if (!item) return;
100-
measureUpdate(() => {
100+
measureUpdate(() =>
101101
ItemResource.update({ id }, { label: `${item.label} (updated)` }).then(
102-
() => {
103-
void client.invalidateQueries({
102+
() =>
103+
client.invalidateQueries({
104104
queryKey: ['items', listViewCount],
105-
});
106-
},
107-
);
108-
});
105+
}),
106+
),
107+
);
109108
},
110109
[measureUpdate, client, listViewCount],
111110
);
@@ -130,22 +129,22 @@ function BenchmarkHarness() {
130129

131130
const createEntity = useCallback(() => {
132131
const author = FIXTURE_AUTHORS[0];
133-
measureUpdate(() => {
134-
ItemResource.create({ label: 'New Item', author }).then(() => {
135-
void client.invalidateQueries({ queryKey: ['items', listViewCount] });
136-
});
137-
});
132+
measureUpdate(() =>
133+
ItemResource.create({ label: 'New Item', author }).then(() =>
134+
client.invalidateQueries({ queryKey: ['items', listViewCount] }),
135+
),
136+
);
138137
}, [measureUpdate, client, listViewCount]);
139138

140139
const deleteEntity = useCallback(
141140
(id: string) => {
142-
measureUpdate(() => {
143-
ItemResource.delete({ id }).then(() => {
144-
void client.invalidateQueries({
141+
measureUpdate(() =>
142+
ItemResource.delete({ id }).then(() =>
143+
client.invalidateQueries({
145144
queryKey: ['items', listViewCount],
146-
});
147-
});
148-
});
145+
}),
146+
),
147+
);
149148
},
150149
[measureUpdate, client, listViewCount],
151150
);

0 commit comments

Comments
 (0)