From 4129ea8c922b950be3964f98d2bb74ff4a1c5431 Mon Sep 17 00:00:00 2001 From: Noah Lemen Date: Tue, 29 Aug 2023 09:06:25 -0400 Subject: [PATCH] set priority on TaskController instead of on postTask/yield (#27295) ## Summary passing both a signal and a priority to `postTask`/`yield` in chrome causes memory to spike and potentially causes OOMs. a fix for this has landed in chrome 118, but we can avoid the issue in earlier versions by setting priority on just the TaskController instead. https://bugs.chromium.org/p/chromium/issues/detail?id=1469367 ## How did you test this change? ``` yarn test SchedulerPostTask ``` --- .../scheduler/src/__tests__/SchedulerPostTask-test.js | 10 ++++++---- packages/scheduler/src/forks/SchedulerPostTask.js | 10 +++++----- 2 files changed, 11 insertions(+), 9 deletions(-) diff --git a/packages/scheduler/src/__tests__/SchedulerPostTask-test.js b/packages/scheduler/src/__tests__/SchedulerPostTask-test.js index d76ca570c2f31..c572ec595bb10 100644 --- a/packages/scheduler/src/__tests__/SchedulerPostTask-test.js +++ b/packages/scheduler/src/__tests__/SchedulerPostTask-test.js @@ -83,7 +83,8 @@ describe('SchedulerPostTask', () => { const scheduler = {}; global.scheduler = scheduler; - scheduler.postTask = function (callback, {priority, signal}) { + scheduler.postTask = function (callback, {signal}) { + const {priority} = signal; const id = idCounter++; log( `Post Task ${id} [${priority === undefined ? '' : priority}]`, @@ -94,7 +95,8 @@ describe('SchedulerPostTask', () => { }); }; - scheduler.yield = function ({priority, signal}) { + scheduler.yield = function ({signal}) { + const {priority} = signal; const id = idCounter++; log(`Yield ${id} [${priority === undefined ? '' : priority}]`); const controller = signal._controller; @@ -111,8 +113,8 @@ describe('SchedulerPostTask', () => { }; global.TaskController = class TaskController { - constructor() { - this.signal = {_controller: this}; + constructor({priority}) { + this.signal = {_controller: this, priority}; } abort() { const task = taskQueue.get(this); diff --git a/packages/scheduler/src/forks/SchedulerPostTask.js b/packages/scheduler/src/forks/SchedulerPostTask.js index 5af147117ef56..e5f45c06ce8c9 100644 --- a/packages/scheduler/src/forks/SchedulerPostTask.js +++ b/packages/scheduler/src/forks/SchedulerPostTask.js @@ -10,7 +10,7 @@ import type {PriorityLevel} from '../SchedulerPriorities'; declare class TaskController { - constructor(priority?: string): TaskController; + constructor(options?: {priority?: string}): TaskController; signal: mixed; abort(): void; } @@ -95,9 +95,8 @@ export function unstable_scheduleCallback( break; } - const controller = new TaskController(); + const controller = new TaskController({priority: postTaskPriority}); const postTaskOptions = { - priority: postTaskPriority, delay: typeof options === 'object' && options !== null ? options.delay : 0, signal: controller.signal, }; @@ -130,9 +129,10 @@ function runTask( if (typeof result === 'function') { // Assume this is a continuation const continuation: SchedulerCallback = (result: any); - const continuationController = new TaskController(); - const continuationOptions = { + const continuationController = new TaskController({ priority: postTaskPriority, + }); + const continuationOptions = { signal: continuationController.signal, }; // Update the original callback node's controller, since even though we're