Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

set priority on TaskController instead of on postTask/yield #27295

Merged
merged 1 commit into from
Aug 29, 2023
Merged

set priority on TaskController instead of on postTask/yield #27295

merged 1 commit into from
Aug 29, 2023

Conversation

noahlemen
Copy link
Member

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

@noahlemen noahlemen requested a review from acdlite August 28, 2023 17:36
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Aug 28, 2023
@noahlemen noahlemen merged commit 4129ea8 into main Aug 29, 2023
2 checks passed
@noahlemen noahlemen deleted the noahlemen/taskcontroller-priority branch August 29, 2023 13:06
github-actions bot pushed a commit that referenced this pull request Aug 29, 2023
@noahlemen
set priority on TaskController instead of on postTask/yield (#27295)
5a211bb 
## 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
```

DiffTrain build for [4129ea8](4129ea8)
@acdlite acdlite mentioned this pull request Sep 14, 2023
Merged
acdlite added a commit to acdlite/next.js that referenced this pull request Sep 14, 2023
@acdlite
Update React from dd480ef92 to d6dcad6a8.
49dcc74 
React upstream changes:

- facebook/react#27374
- facebook/react#27369
- facebook/react#27372
- facebook/react#27371
- facebook/react#27370
- facebook/react#27321
- facebook/react#27368
- facebook/react#27367
- facebook/react#27366
- facebook/react#27360
- facebook/react#27361
- facebook/react#27357
- facebook/react#27359
- facebook/react#27358
- facebook/react#27330
- facebook/react#27347
- facebook/react#27307
- facebook/react#27346
- facebook/react#27342
- facebook/react#27340
- facebook/react#27328
- facebook/react#27327
- facebook/react#27325
- facebook/react#27337
- facebook/react#27336
- facebook/react#27323
- facebook/react#27320
- facebook/react#27317
- facebook/react#27318
- facebook/react#27316
- facebook/react#27313
- facebook/react#27309
- facebook/react#27302
- facebook/react#27297
- facebook/react#27295
- facebook/react#27305
- facebook/react#27215
- facebook/react#27304
- facebook/react#27067
- facebook/react#27179
- facebook/react#27278
- facebook/react#27277
- facebook/react#27282
- facebook/react#27230
- facebook/react#27260
- facebook/react#27270
- facebook/react#27273
- facebook/react#27268
- facebook/react#27269
acdlite added a commit to acdlite/next.js that referenced this pull request Sep 14, 2023
@acdlite
Update React from dd480ef92 to d6dcad6a8.
2361578 
React upstream changes:

- facebook/react#27374
- facebook/react#27369
- facebook/react#27372
- facebook/react#27371
- facebook/react#27370
- facebook/react#27321
- facebook/react#27368
- facebook/react#27367
- facebook/react#27366
- facebook/react#27360
- facebook/react#27361
- facebook/react#27357
- facebook/react#27359
- facebook/react#27358
- facebook/react#27330
- facebook/react#27347
- facebook/react#27307
- facebook/react#27346
- facebook/react#27342
- facebook/react#27340
- facebook/react#27328
- facebook/react#27327
- facebook/react#27325
- facebook/react#27337
- facebook/react#27336
- facebook/react#27323
- facebook/react#27320
- facebook/react#27317
- facebook/react#27318
- facebook/react#27316
- facebook/react#27313
- facebook/react#27309
- facebook/react#27302
- facebook/react#27297
- facebook/react#27295
- facebook/react#27305
- facebook/react#27215
- facebook/react#27304
- facebook/react#27067
- facebook/react#27179
- facebook/react#27278
- facebook/react#27277
- facebook/react#27282
- facebook/react#27230
- facebook/react#27260
- facebook/react#27270
- facebook/react#27273
- facebook/react#27268
- facebook/react#27269
acdlite added a commit to acdlite/next.js that referenced this pull request Sep 14, 2023
@acdlite
Update React from dd480ef92 to d6dcad6a8.
7e8e320 
React upstream changes:

- facebook/react#27374
- facebook/react#27369
- facebook/react#27372
- facebook/react#27371
- facebook/react#27370
- facebook/react#27321
- facebook/react#27368
- facebook/react#27367
- facebook/react#27366
- facebook/react#27360
- facebook/react#27361
- facebook/react#27357
- facebook/react#27359
- facebook/react#27358
- facebook/react#27330
- facebook/react#27347
- facebook/react#27307
- facebook/react#27346
- facebook/react#27342
- facebook/react#27340
- facebook/react#27328
- facebook/react#27327
- facebook/react#27325
- facebook/react#27337
- facebook/react#27336
- facebook/react#27323
- facebook/react#27320
- facebook/react#27317
- facebook/react#27318
- facebook/react#27316
- facebook/react#27313
- facebook/react#27309
- facebook/react#27302
- facebook/react#27297
- facebook/react#27295
- facebook/react#27305
- facebook/react#27215
- facebook/react#27304
- facebook/react#27067
- facebook/react#27179
- facebook/react#27278
- facebook/react#27277
- facebook/react#27282
- facebook/react#27230
- facebook/react#27260
- facebook/react#27270
- facebook/react#27273
- facebook/react#27268
- facebook/react#27269
kodiakhq bot pushed a commit to vercel/next.js that referenced this pull request Sep 14, 2023
@acdlite
Update React from dd480ef92 to d6dcad6a8. (#55399)
6f2e9ae 
### React upstream changes:

- facebook/react#27374
- facebook/react#27369
- facebook/react#27372
- facebook/react#27371
- facebook/react#27370
- facebook/react#27321
- facebook/react#27368
- facebook/react#27367
- facebook/react#27366
- facebook/react#27360
- facebook/react#27361
- facebook/react#27357
- facebook/react#27359
- facebook/react#27358
- facebook/react#27330
- facebook/react#27347
- facebook/react#27307
- facebook/react#27346
- facebook/react#27342
- facebook/react#27340
- facebook/react#27328
- facebook/react#27327
- facebook/react#27325
- facebook/react#27337
- facebook/react#27336
- facebook/react#27323
- facebook/react#27320
- facebook/react#27317
- facebook/react#27318
- facebook/react#27316
- facebook/react#27313
- facebook/react#27309
- facebook/react#27302
- facebook/react#27297
- facebook/react#27295
- facebook/react#27305
- facebook/react#27215
- facebook/react#27304
- facebook/react#27067
- facebook/react#27179
- facebook/react#27278
- facebook/react#27277
- facebook/react#27282
- facebook/react#27230
- facebook/react#27260
- facebook/react#27270
- facebook/react#27273
- facebook/react#27268
- facebook/react#27269
EdisonVan pushed a commit to EdisonVan/react that referenced this pull request Apr 15, 2024
@noahlemen @AndyPengc12
set priority on TaskController instead of on postTask/yield (facebook…
d8ea260 
…#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
```
bigfootjon pushed a commit that referenced this pull request Apr 18, 2024
@noahlemen
set priority on TaskController instead of on postTask/yield (#27295)
33e4ca3 
## 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
```

DiffTrain build for commit 4129ea8.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@acdlite acdlite acdlite approved these changes

Assignees
No one assigned
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@noahlemen @acdlite @facebook-github-bot