Skip to content

Commit 2e40544

Browse files
VarixoVarixo
committed
fix: skip apply journal during vnode diff
1 parent da59cf0 commit 2e40544

File tree

2 files changed

+80
-5
lines changed

2 files changed

+80
-5
lines changed

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

Lines changed: 69 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,7 @@ export interface Chore<T extends ChoreType = ChoreType> {
157157
$blockedChores$: ChoreArray | null;
158158
$startTime$: number | undefined;
159159
$endTime$: number | undefined;
160+
$blocksJournalFlush$: boolean;
160161

161162
$resolve$: ((value: any) => void) | undefined;
162163
$reject$: ((reason?: any) => void) | undefined;
@@ -194,6 +195,8 @@ export const createScheduler = (
194195
let currentTime = performance.now();
195196
const nextTick = createNextTick(drainChoreQueue);
196197
let flushTimerId: number | null = null;
198+
let blockingChoresCount = 0;
199+
let currentChore: Chore | null = null;
197200

198201
function drainInNextTick() {
199202
if (!drainScheduled) {
@@ -275,6 +278,10 @@ export const createScheduler = (
275278
if (isTask) {
276279
(hostOrTask as Task).$flags$ |= TaskFlags.DIRTY;
277280
}
281+
// Determine if this chore should block journal flush
282+
// NODE_DIFF and COMPONENT always block journal flush
283+
const shouldBlockJournalFlush = type === ChoreType.NODE_DIFF || type === ChoreType.COMPONENT;
284+
278285
const chore: Chore<T> = {
279286
$type$: type,
280287
$idx$: isTask
@@ -289,11 +296,17 @@ export const createScheduler = (
289296
$blockedChores$: null,
290297
$startTime$: undefined,
291298
$endTime$: undefined,
299+
$blocksJournalFlush$: shouldBlockJournalFlush,
292300
$resolve$: undefined,
293301
$reject$: undefined,
294302
$returnValue$: null!,
295303
};
296304

305+
// Increment blocking counter as soon as chore is created (even if it gets blocked)
306+
if (shouldBlockJournalFlush) {
307+
blockingChoresCount++;
308+
}
309+
297310
if (type === ChoreType.WAIT_FOR_QUEUE) {
298311
getChorePromise(chore);
299312
drainChore = chore as Chore<ChoreType.WAIT_FOR_QUEUE>;
@@ -345,6 +358,10 @@ This is often caused by modifying a signal in an already rendered component duri
345358
logWarn(warningMessage);
346359
DEBUG &&
347360
debugTrace('schedule.SKIPPED host is not updatable', chore, choreQueue, blockedChores);
361+
// Decrement counter if this was a blocking chore that we're skipping
362+
if (chore.$blocksJournalFlush$) {
363+
blockingChoresCount--;
364+
}
348365
return chore;
349366
}
350367
}
@@ -374,7 +391,14 @@ This is often caused by modifying a signal in an already rendered component duri
374391
}
375392

376393
addChore(chore, choreQueue);
377-
DEBUG && debugTrace('schedule', chore, choreQueue, blockedChores);
394+
395+
DEBUG &&
396+
debugTrace(
397+
chore.$blocksJournalFlush$ ? `schedule (blocking ${blockingChoresCount})` : 'schedule',
398+
chore,
399+
choreQueue,
400+
blockedChores
401+
);
378402

379403
const runImmediately = (isServer && type === ChoreType.COMPONENT) || type === ChoreType.RUN_QRL;
380404

@@ -432,6 +456,16 @@ This is often caused by modifying a signal in an already rendered component duri
432456
}
433457

434458
function applyJournalFlush() {
459+
if (blockingChoresCount > 0) {
460+
DEBUG &&
461+
debugTrace(
462+
`journalFlush.BLOCKED (${blockingChoresCount} blocking chores)`,
463+
null,
464+
choreQueue,
465+
blockedChores
466+
);
467+
return;
468+
}
435469
if (!isJournalFlushRunning) {
436470
// prevent multiple journal flushes from running at the same time
437471
isJournalFlushRunning = true;
@@ -521,13 +555,12 @@ This is often caused by modifying a signal in an already rendered component duri
521555
}
522556
};
523557

524-
let currentChore: Chore | null = null;
525-
526558
try {
527559
while (choreQueue.length) {
528560
currentTime = performance.now();
529561
const chore = (currentChore = choreQueue.shift()!);
530562
if (chore.$state$ !== ChoreState.NONE) {
563+
// Chore was already processed, counter already decremented in finishChore/handleError
531564
continue;
532565
}
533566

@@ -541,6 +574,10 @@ This is often caused by modifying a signal in an already rendered component duri
541574
if (vnode_isVNode(chore.$host$)) {
542575
chore.$host$.chores?.delete(chore);
543576
}
577+
// Decrement counter if this was a blocking chore that we're skipping
578+
if (chore.$blocksJournalFlush$) {
579+
blockingChoresCount--;
580+
}
544581
continue;
545582
}
546583

@@ -618,13 +655,40 @@ This is often caused by modifying a signal in an already rendered component duri
618655
if (vnode_isVNode(chore.$host$)) {
619656
chore.$host$.chores?.delete(chore);
620657
}
621-
DEBUG && debugTrace('execute.DONE', chore, choreQueue, blockedChores);
658+
659+
// Decrement blocking counter if this chore was blocking journal flush
660+
if (chore.$blocksJournalFlush$) {
661+
blockingChoresCount--;
662+
DEBUG &&
663+
debugTrace(
664+
`execute.DONE (blocking ${blockingChoresCount})`,
665+
chore,
666+
choreQueue,
667+
blockedChores
668+
);
669+
} else {
670+
DEBUG && debugTrace('execute.DONE', chore, choreQueue, blockedChores);
671+
}
622672
}
623673

624674
function handleError(chore: Chore, e: any) {
625675
chore.$endTime$ = performance.now();
626676
chore.$state$ = ChoreState.FAILED;
627-
DEBUG && debugTrace('execute.ERROR', chore, choreQueue, blockedChores);
677+
678+
// Decrement blocking counter if this chore was blocking journal flush
679+
if (chore.$blocksJournalFlush$) {
680+
blockingChoresCount--;
681+
DEBUG &&
682+
debugTrace(
683+
`execute.ERROR (blocking ${blockingChoresCount})`,
684+
chore,
685+
choreQueue,
686+
blockedChores
687+
);
688+
} else {
689+
DEBUG && debugTrace('execute.ERROR', chore, choreQueue, blockedChores);
690+
}
691+
628692
// If we used the result as promise, this won't exist
629693
chore.$reject$?.(e);
630694
container.handleError(e, chore.$host$);

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -612,6 +612,7 @@ describe('scheduler', () => {
612612
$blockedChores$: null,
613613
$startTime$: undefined,
614614
$endTime$: undefined,
615+
$blocksJournalFlush$: false,
615616
$resolve$: undefined,
616617
$reject$: undefined,
617618
$returnValue$: null!,
@@ -635,6 +636,7 @@ describe('scheduler', () => {
635636
$blockedChores$: null,
636637
$startTime$: undefined,
637638
$endTime$: undefined,
639+
$blocksJournalFlush$: false,
638640
$resolve$: undefined,
639641
$reject$: undefined,
640642
$returnValue$: null!,
@@ -659,6 +661,7 @@ describe('scheduler', () => {
659661
$blockedChores$: null,
660662
$startTime$: undefined,
661663
$endTime$: undefined,
664+
$blocksJournalFlush$: false,
662665
$resolve$: undefined,
663666
$reject$: undefined,
664667
$returnValue$: null!,
@@ -674,6 +677,7 @@ describe('scheduler', () => {
674677
$blockedChores$: null,
675678
$startTime$: undefined,
676679
$endTime$: undefined,
680+
$blocksJournalFlush$: false,
677681
$resolve$: undefined,
678682
$reject$: undefined,
679683
$returnValue$: null!,
@@ -701,6 +705,7 @@ describe('scheduler', () => {
701705
$blockedChores$: null,
702706
$startTime$: undefined,
703707
$endTime$: undefined,
708+
$blocksJournalFlush$: false,
704709
$resolve$: undefined,
705710
$reject$: undefined,
706711
$returnValue$: null!,
@@ -727,6 +732,7 @@ describe('scheduler', () => {
727732
$blockedChores$: null,
728733
$startTime$: undefined,
729734
$endTime$: undefined,
735+
$blocksJournalFlush$: false,
730736
$resolve$: undefined,
731737
$reject$: undefined,
732738
$returnValue$: null!,
@@ -742,6 +748,7 @@ describe('scheduler', () => {
742748
$blockedChores$: null,
743749
$startTime$: undefined,
744750
$endTime$: undefined,
751+
$blocksJournalFlush$: false,
745752
$resolve$: undefined,
746753
$reject$: undefined,
747754
$returnValue$: null!,
@@ -757,6 +764,7 @@ describe('scheduler', () => {
757764
$blockedChores$: null,
758765
$startTime$: undefined,
759766
$endTime$: undefined,
767+
$blocksJournalFlush$: false,
760768
$resolve$: undefined,
761769
$reject$: undefined,
762770
$returnValue$: null!,
@@ -792,6 +800,7 @@ describe('scheduler', () => {
792800
$blockedChores$: null,
793801
$startTime$: undefined,
794802
$endTime$: undefined,
803+
$blocksJournalFlush$: false,
795804
$resolve$: undefined,
796805
$reject$: undefined,
797806
$returnValue$: null!,
@@ -807,6 +816,7 @@ describe('scheduler', () => {
807816
$blockedChores$: null,
808817
$startTime$: undefined,
809818
$endTime$: undefined,
819+
$blocksJournalFlush$: false,
810820
$resolve$: undefined,
811821
$reject$: undefined,
812822
$returnValue$: null!,
@@ -1216,6 +1226,7 @@ describe('scheduler', () => {
12161226
expect(chore.$type$).not.toBe(ChoreType.QRL_RESOLVE);
12171227
}
12181228

1229+
await waitForDrain();
12191230
nextTickSpy.mockRestore();
12201231
});
12211232
});

0 commit comments

Comments
 (0)