Skip to content

Commit a21e92b

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

File tree

3 files changed

+95
-14
lines changed

3 files changed

+95
-14
lines changed

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ const createMockChore = (
3030
$endTime$: undefined,
3131
$resolve$: undefined,
3232
$reject$: undefined,
33+
$blocksJournalFlush$: false,
3334
});
3435

3536
let rootVNode = vnode_newVirtual();

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

Lines changed: 83 additions & 14 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,6 +296,7 @@ export const createScheduler = (
289296
$blockedChores$: null,
290297
$startTime$: undefined,
291298
$endTime$: undefined,
299+
$blocksJournalFlush$: shouldBlockJournalFlush,
292300
$resolve$: undefined,
293301
$reject$: undefined,
294302
$returnValue$: null!,
@@ -345,6 +353,10 @@ This is often caused by modifying a signal in an already rendered component duri
345353
logWarn(warningMessage);
346354
DEBUG &&
347355
debugTrace('schedule.SKIPPED host is not updatable', chore, choreQueue, blockedChores);
356+
// Decrement counter if this was a blocking chore that we're skipping
357+
if (chore.$blocksJournalFlush$) {
358+
blockingChoresCount--;
359+
}
348360
return chore;
349361
}
350362
}
@@ -373,8 +385,15 @@ This is often caused by modifying a signal in an already rendered component duri
373385
}
374386
}
375387

376-
addChore(chore, choreQueue);
377-
DEBUG && debugTrace('schedule', chore, choreQueue, blockedChores);
388+
addChoreAndIncrementBlockingCounter(chore, choreQueue);
389+
390+
DEBUG &&
391+
debugTrace(
392+
chore.$blocksJournalFlush$ ? `schedule (blocking ${blockingChoresCount})` : 'schedule',
393+
chore,
394+
choreQueue,
395+
blockedChores
396+
);
378397

379398
const runImmediately = (isServer && type === ChoreType.COMPONENT) || type === ChoreType.RUN_QRL;
380399

@@ -432,6 +451,16 @@ This is often caused by modifying a signal in an already rendered component duri
432451
}
433452

434453
function applyJournalFlush() {
454+
if (blockingChoresCount > 0) {
455+
DEBUG &&
456+
debugTrace(
457+
`journalFlush.BLOCKED (${blockingChoresCount} blocking chores)`,
458+
null,
459+
choreQueue,
460+
blockedChores
461+
);
462+
return;
463+
}
435464
if (!isJournalFlushRunning) {
436465
// prevent multiple journal flushes from running at the same time
437466
isJournalFlushRunning = true;
@@ -509,7 +538,7 @@ This is often caused by modifying a signal in an already rendered component duri
509538
if (vnode_isVNode(blockedChore.$host$)) {
510539
blockedChore.$host$.blockedChores?.delete(blockedChore);
511540
}
512-
addChore(blockedChore, choreQueue);
541+
addChoreAndIncrementBlockingCounter(blockedChore, choreQueue);
513542
DEBUG && debugTrace('schedule.UNBLOCKED', blockedChore, choreQueue, blockedChores);
514543
blockedChoresScheduled = true;
515544
}
@@ -521,13 +550,12 @@ This is often caused by modifying a signal in an already rendered component duri
521550
}
522551
};
523552

524-
let currentChore: Chore | null = null;
525-
526553
try {
527554
while (choreQueue.length) {
528555
currentTime = performance.now();
529556
const chore = (currentChore = choreQueue.shift()!);
530557
if (chore.$state$ !== ChoreState.NONE) {
558+
// Chore was already processed, counter already decremented in finishChore/handleError
531559
continue;
532560
}
533561

@@ -541,6 +569,10 @@ This is often caused by modifying a signal in an already rendered component duri
541569
if (vnode_isVNode(chore.$host$)) {
542570
chore.$host$.chores?.delete(chore);
543571
}
572+
// Decrement counter if this was a blocking chore that we're skipping
573+
if (chore.$blocksJournalFlush$) {
574+
blockingChoresCount--;
575+
}
544576
continue;
545577
}
546578

@@ -618,13 +650,40 @@ This is often caused by modifying a signal in an already rendered component duri
618650
if (vnode_isVNode(chore.$host$)) {
619651
chore.$host$.chores?.delete(chore);
620652
}
621-
DEBUG && debugTrace('execute.DONE', chore, choreQueue, blockedChores);
653+
654+
// Decrement blocking counter if this chore was blocking journal flush
655+
if (chore.$blocksJournalFlush$) {
656+
blockingChoresCount--;
657+
DEBUG &&
658+
debugTrace(
659+
`execute.DONE (blocking ${blockingChoresCount})`,
660+
chore,
661+
choreQueue,
662+
blockedChores
663+
);
664+
} else {
665+
DEBUG && debugTrace('execute.DONE', chore, choreQueue, blockedChores);
666+
}
622667
}
623668

624669
function handleError(chore: Chore, e: any) {
625670
chore.$endTime$ = performance.now();
626671
chore.$state$ = ChoreState.FAILED;
627-
DEBUG && debugTrace('execute.ERROR', chore, choreQueue, blockedChores);
672+
673+
// Decrement blocking counter if this chore was blocking journal flush
674+
if (chore.$blocksJournalFlush$) {
675+
blockingChoresCount--;
676+
DEBUG &&
677+
debugTrace(
678+
`execute.ERROR (blocking ${blockingChoresCount})`,
679+
chore,
680+
choreQueue,
681+
blockedChores
682+
);
683+
} else {
684+
DEBUG && debugTrace('execute.ERROR', chore, choreQueue, blockedChores);
685+
}
686+
628687
// If we used the result as promise, this won't exist
629688
chore.$reject$?.(e);
630689
container.handleError(e, chore.$host$);
@@ -814,8 +873,25 @@ This is often caused by modifying a signal in an already rendered component duri
814873
}
815874
return null;
816875
}
876+
877+
function addChoreAndIncrementBlockingCounter(chore: Chore, choreArray: ChoreArray) {
878+
if (addChore(chore, choreArray)) {
879+
blockingChoresCount++;
880+
}
881+
}
817882
};
818883

884+
export function addChore(chore: Chore, choreArray: ChoreArray): boolean {
885+
const idx = choreArray.add(chore);
886+
if (idx < 0) {
887+
if (vnode_isVNode(chore.$host$)) {
888+
(chore.$host$.chores ||= new ChoreArray()).add(chore);
889+
}
890+
return chore.$blocksJournalFlush$;
891+
}
892+
return false;
893+
}
894+
819895
function vNodeAlreadyDeleted(chore: Chore): boolean {
820896
return !!(chore.$host$ && vnode_isVNode(chore.$host$) && chore.$host$.flags & VNodeFlags.Deleted);
821897
}
@@ -839,13 +915,6 @@ export function addBlockedChore(
839915
}
840916
}
841917

842-
export function addChore(chore: Chore, choreArray: ChoreArray) {
843-
const idx = choreArray.add(chore);
844-
if (idx < 0 && vnode_isVNode(chore.$host$)) {
845-
(chore.$host$.chores ||= new ChoreArray()).add(chore);
846-
}
847-
}
848-
849918
function choreTypeToName(type: ChoreType): string {
850919
return (
851920
(

‎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)