Skip to content

Commit d0012c5

Browse files
wmertenswmertens
authored
Merge pull request #8087 from QwikDev/v2-fix-apply-journal
fix: ensure DOM is updated during long running tasks
2 parents b14fe22 + 822feb0 commit d0012c5

File tree

3 files changed

+229
-9
lines changed

3 files changed

+229
-9
lines changed

‎.changeset/nice-teams-grow.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@qwik.dev/core': patch
3+
---
4+
5+
fix: ensure DOM is updated during long running tasks

‎packages/qwik/src/core/shared/scheduler.ts‎

Lines changed: 45 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ export const createScheduler = (
193193
let flushBudgetStart = 0;
194194
let currentTime = performance.now();
195195
const nextTick = createNextTick(drainChoreQueue);
196+
let flushTimerId: number | null = null;
196197

197198
function drainInNextTick() {
198199
if (!drainScheduled) {
@@ -387,13 +388,50 @@ This is often caused by modifying a signal in an already rendered component duri
387388
// Drain queue helpers
388389
////////////////////////////////////////////////////////////////////////////////
389390

391+
function cancelFlushTimer() {
392+
if (flushTimerId != null) {
393+
clearTimeout(flushTimerId);
394+
flushTimerId = null;
395+
}
396+
}
397+
398+
function scheduleFlushTimer(): void {
399+
const isServer = isServerPlatform();
400+
// Never schedule timers on the server
401+
if (isServer) {
402+
return;
403+
}
404+
// Ignore if a timer is already scheduled
405+
if (flushTimerId != null) {
406+
return;
407+
}
408+
409+
const now = performance.now();
410+
const elapsed = now - flushBudgetStart;
411+
const delay = Math.max(0, FREQUENCY_MS - elapsed);
412+
// Deadline already reached, flush now
413+
if (delay === 0) {
414+
if (!isDraining) {
415+
applyJournalFlush();
416+
}
417+
return;
418+
}
419+
420+
flushTimerId = setTimeout(() => {
421+
flushTimerId = null;
422+
423+
applyJournalFlush();
424+
}, delay) as unknown as number;
425+
}
426+
390427
function applyJournalFlush() {
391428
if (!isJournalFlushRunning) {
392429
// prevent multiple journal flushes from running at the same time
393430
isJournalFlushRunning = true;
394431
journalFlush();
395432
isJournalFlushRunning = false;
396433
flushBudgetStart = performance.now();
434+
cancelFlushTimer();
397435
DEBUG && debugTrace('journalFlush.DONE', null, choreQueue, blockedChores);
398436
}
399437
}
@@ -421,6 +459,7 @@ This is often caused by modifying a signal in an already rendered component duri
421459
}
422460
isDraining = true;
423461
flushBudgetStart = performance.now();
462+
cancelFlushTimer();
424463

425464
const maybeFinishDrain = () => {
426465
if (choreQueue.length) {
@@ -535,15 +574,12 @@ This is often caused by modifying a signal in an already rendered component duri
535574
scheduleBlockedChoresAndDrainIfNeeded(chore);
536575
// If drainChore is not null, we are waiting for it to finish.
537576
// If there are no running chores, we can finish the drain.
538-
if (!runningChores.size) {
539-
let finished = false;
540-
if (drainChore) {
541-
finished = maybeFinishDrain();
542-
}
543-
if (!finished && !isDraining) {
544-
// if finished, then journal flush is already applied
545-
applyJournalFlush();
546-
}
577+
let finished = false;
578+
if (drainChore && !runningChores.size) {
579+
finished = maybeFinishDrain();
580+
}
581+
if (!finished && !isDraining) {
582+
scheduleFlushTimer();
547583
}
548584
});
549585
} else {

‎packages/qwik/src/core/shared/scheduler.unit.tsx‎

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -411,6 +411,185 @@ describe('scheduler', () => {
411411
});
412412
});
413413

414+
describe('deadline-based async flushing', () => {
415+
let scheduler: ReturnType<typeof createScheduler> = null!;
416+
let document: ReturnType<typeof createDocument> = null!;
417+
let vHost: VirtualVNode = null!;
418+
419+
async function waitForDrain() {
420+
await scheduler(ChoreType.WAIT_FOR_QUEUE).$returnValue$;
421+
}
422+
423+
beforeEach(() => {
424+
vi.clearAllMocks();
425+
(globalThis as any).testLog = [];
426+
vi.useFakeTimers();
427+
document = createDocument();
428+
document.body.setAttribute(QContainerAttr, 'paused');
429+
const container = getDomContainer(document.body);
430+
const choreQueue = new ChoreArray();
431+
const blockedChores = new Set<Chore>();
432+
const runningChores = new Set<Chore>();
433+
scheduler = createScheduler(
434+
container,
435+
() => testLog.push('journalFlush'),
436+
choreQueue,
437+
blockedChores,
438+
runningChores
439+
);
440+
vnode_newUnMaterializedElement(document.body);
441+
vHost = vnode_newVirtual();
442+
vHost.setProp('q:id', 'host');
443+
});
444+
445+
afterEach(() => {
446+
vi.useRealTimers();
447+
vi.unstubAllGlobals();
448+
});
449+
450+
it('fast async (<16ms) + long async (>16ms): flush at ~16ms while long runs', async () => {
451+
const FREQUENCY_MS = Math.floor(1000 / 60);
452+
// Fast async (5ms)
453+
scheduler(
454+
ChoreType.TASK,
455+
mockTask(vHost, {
456+
index: 0,
457+
qrl: $(
458+
() =>
459+
new Promise<void>((resolve) =>
460+
setTimeout(() => {
461+
testLog.push('fastAsync');
462+
resolve();
463+
}, 5)
464+
)
465+
),
466+
})
467+
);
468+
// Long async (1000ms)
469+
scheduler(
470+
ChoreType.TASK,
471+
mockTask(vHost, {
472+
index: 1,
473+
qrl: $(
474+
() =>
475+
new Promise<void>((resolve) =>
476+
setTimeout(() => {
477+
testLog.push('longAsync');
478+
resolve();
479+
}, 1000)
480+
)
481+
),
482+
})
483+
);
484+
485+
// Advance to 5ms: fast async resolves
486+
await vi.advanceTimersByTimeAsync(5);
487+
expect(testLog).toEqual([
488+
// end of queue flush
489+
'journalFlush',
490+
// task execution
491+
'fastAsync',
492+
]);
493+
494+
await vi.advanceTimersByTimeAsync(FREQUENCY_MS - 5);
495+
496+
// Flush should have occurred before longAsync finishes
497+
expect(testLog).toEqual([
498+
// end of queue flush
499+
'journalFlush',
500+
// task execution
501+
'fastAsync',
502+
// after task execution flush
503+
'journalFlush',
504+
]);
505+
506+
// Finish long async
507+
await vi.advanceTimersByTimeAsync(1000 - FREQUENCY_MS);
508+
509+
// Now long async completes and a final flush happens at end of drain
510+
const drainPromise = waitForDrain();
511+
// Need to advance timers to process the nextTick that waitForDrain schedules
512+
await vi.advanceTimersByTimeAsync(0);
513+
await drainPromise;
514+
515+
expect(testLog).toEqual([
516+
// end of queue flush
517+
'journalFlush',
518+
// task execution
519+
'fastAsync',
520+
// after task execution flush
521+
'journalFlush',
522+
'longAsync',
523+
'journalFlush',
524+
// TODO: not sure why this is here, but seems related to the vi.advanceTimersByTimeAsync(0) above
525+
'journalFlush',
526+
]);
527+
});
528+
529+
it('multiple fast async (<16ms total): do not flush between, only after', async () => {
530+
const FREQUENCY_MS = Math.floor(1000 / 60);
531+
// Two fast async chores: 5ms and 6ms (total 11ms < 16ms)
532+
scheduler(
533+
ChoreType.TASK,
534+
mockTask(vHost, {
535+
index: 0,
536+
qrl: $(
537+
() =>
538+
new Promise<void>((resolve) =>
539+
setTimeout(() => {
540+
testLog.push('fast1');
541+
resolve();
542+
}, 5)
543+
)
544+
),
545+
})
546+
);
547+
scheduler(
548+
ChoreType.TASK,
549+
mockTask(vHost, {
550+
index: 1,
551+
qrl: $(
552+
() =>
553+
new Promise<void>((resolve) =>
554+
setTimeout(() => {
555+
testLog.push('fast2');
556+
resolve();
557+
}, 6)
558+
)
559+
),
560+
})
561+
);
562+
563+
// First resolves at 5ms
564+
await vi.advanceTimersByTimeAsync(5);
565+
expect(testLog).toEqual([
566+
// end of queue flush
567+
'journalFlush',
568+
'fast1',
569+
]);
570+
571+
// Second resolves at 11ms
572+
await vi.advanceTimersByTimeAsync(6);
573+
expect(testLog).toEqual([
574+
// end of queue flush
575+
'journalFlush',
576+
'fast1',
577+
'fast2',
578+
]);
579+
580+
await vi.advanceTimersByTimeAsync(FREQUENCY_MS - 11);
581+
582+
expect(testLog).toEqual([
583+
// end of queue flush
584+
'journalFlush',
585+
'fast1',
586+
'fast2',
587+
// journal flush after fast1/fast2 chore
588+
'journalFlush',
589+
]);
590+
});
591+
});
592+
414593
describe('addChore', () => {
415594
let choreArray: ChoreArray;
416595
let vHost: VirtualVNode;

0 commit comments

Comments
 (0)