Wire up configuration to use modern version of RuntimeScheduler #40945
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
facebook-github-bot
added
CLA Signed
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D50171297 |
Base commit: c9d0a00 |
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D50171297 |
rubennorte
added a commit
to rubennorte/react-native
that referenced
this pull request
Oct 16, 2023
…book#40945) Summary: Pull Request resolved: facebook#40945 This adds some temporary logic to configure the use of the modern version of RuntimeScheduler based on values coming from the app configuration. This logic is centralized in `ReactInstance` so from that point the code is completely cross-platform. This doesn't use `ReactNativeConfig`/`CoreFeatures` because they're initialized after the point where we need to access them for this use case. This way is a bit uglier but this isn't intended to live for long (only until we verify this doesn't have regressions in a complex app). Changelog: [internal] --- Differential Revision: D50171297 fbshipit-source-id: 219f6053588c14f34ec9c2f74a477c2d78239183
rubennorte
force-pushed
the
export-D50171297
branch
from
1e21952 to
302d2f8
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D50171297 |
rubennorte
added a commit
to rubennorte/react-native
that referenced
this pull request
Oct 16, 2023
…book#40945) Summary: Pull Request resolved: facebook#40945 This adds some temporary logic to configure the use of the modern version of RuntimeScheduler based on values coming from the app configuration. This logic is centralized in `ReactInstance` so from that point the code is completely cross-platform. This doesn't use `ReactNativeConfig`/`CoreFeatures` because they're initialized after the point where we need to access them for this use case. This way is a bit uglier but this isn't intended to live for long (only until we verify this doesn't have regressions in a complex app). Changelog: [internal] --- Differential Revision: D50171297 fbshipit-source-id: e92a71779bf2b588c67562da7f02349860df9e58
rubennorte
force-pushed
the
export-D50171297
branch
from
302d2f8 to
a2312ad
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D50171297 |
rubennorte
added a commit
to rubennorte/react-native
that referenced
this pull request
Oct 16, 2023
…book#40945) Summary: Pull Request resolved: facebook#40945 This adds some temporary logic to configure the use of the modern version of RuntimeScheduler based on values coming from the app configuration. This logic is centralized in `ReactInstance` so from that point the code is completely cross-platform. This doesn't use `ReactNativeConfig`/`CoreFeatures` because they're initialized after the point where we need to access them for this use case. This way is a bit uglier but this isn't intended to live for long (only until we verify this doesn't have regressions in a complex app). Changelog: [internal] --- Differential Revision: D50171297 fbshipit-source-id: c66711c9e26b970069a73e71af9e8d1bf1e7ef87
rubennorte
force-pushed
the
export-D50171297
branch
from
a2312ad to
2f929a5
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D50171297 |
rubennorte
added a commit
to rubennorte/react-native
that referenced
this pull request
Oct 17, 2023
…book#40945) Summary: Pull Request resolved: facebook#40945 This adds some temporary logic to configure the use of the modern version of RuntimeScheduler based on values coming from the app configuration. This logic is centralized in `ReactInstance` so from that point the code is completely cross-platform. This doesn't use `ReactNativeConfig`/`CoreFeatures` because they're initialized after the point where we need to access them for this use case. This way is a bit uglier but this isn't intended to live for long (only until we verify this doesn't have regressions in a complex app). Changelog: [internal] --- Reviewed By: sammy-SC Differential Revision: D50171297 fbshipit-source-id: f92e98739cc60022288f1391af2e44d6e755a682
rubennorte
force-pushed
the
export-D50171297
branch
from
2f929a5 to
377191a
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D50171297 |
rubennorte
added a commit
to rubennorte/react-native
that referenced
this pull request
Oct 17, 2023
…book#40945) Summary: Pull Request resolved: facebook#40945 This adds some temporary logic to configure the use of the modern version of RuntimeScheduler based on values coming from the app configuration. This logic is centralized in `ReactInstance` so from that point the code is completely cross-platform. This doesn't use `ReactNativeConfig`/`CoreFeatures` because they're initialized after the point where we need to access them for this use case. This way is a bit uglier but this isn't intended to live for long (only until we verify this doesn't have regressions in a complex app). Changelog: [internal] --- Reviewed By: sammy-SC Differential Revision: D50171297 fbshipit-source-id: 27af6302aae3f13bc891754fa8141e854ae25000
rubennorte
force-pushed
the
export-D50171297
branch
from
377191a to
51b6447
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D50171297 |
rubennorte
added a commit
to rubennorte/react-native
that referenced
this pull request
Oct 17, 2023
…book#40945) Summary: Pull Request resolved: facebook#40945 This adds some temporary logic to configure the use of the modern version of RuntimeScheduler based on values coming from the app configuration. This logic is centralized in `ReactInstance` so from that point the code is completely cross-platform. This doesn't use `ReactNativeConfig`/`CoreFeatures` because they're initialized after the point where we need to access them for this use case. This way is a bit uglier but this isn't intended to live for long (only until we verify this doesn't have regressions in a complex app). Changelog: [internal] --- Reviewed By: sammy-SC Differential Revision: D50171297 fbshipit-source-id: 09ff8a4eb822405babeebbc075be3bed9abbcbac
rubennorte
force-pushed
the
export-D50171297
branch
from
51b6447 to
bcaf066
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D50171297 |
rubennorte
added a commit
to rubennorte/react-native
that referenced
this pull request
Oct 17, 2023
…book#40945) Summary: Pull Request resolved: facebook#40945 This adds some temporary logic to configure the use of the modern version of RuntimeScheduler based on values coming from the app configuration. This logic is centralized in `ReactInstance` so from that point the code is completely cross-platform. This doesn't use `ReactNativeConfig`/`CoreFeatures` because they're initialized after the point where we need to access them for this use case. This way is a bit uglier but this isn't intended to live for long (only until we verify this doesn't have regressions in a complex app). Changelog: [internal] --- Reviewed By: sammy-SC Differential Revision: D50171297 fbshipit-source-id: 9b0a35aac59d83355d6beb1b9a3e19c12fe58dfd
rubennorte
force-pushed
the
export-D50171297
branch
from
bcaf066 to
0f5749a
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D50171297 |
rubennorte
added a commit
to rubennorte/react-native
that referenced
this pull request
Oct 17, 2023
…book#40945) Summary: Pull Request resolved: facebook#40945 This adds some temporary logic to configure the use of the modern version of RuntimeScheduler based on values coming from the app configuration. This logic is centralized in `ReactInstance` so from that point the code is completely cross-platform. This doesn't use `ReactNativeConfig`/`CoreFeatures` because they're initialized after the point where we need to access them for this use case. This way is a bit uglier but this isn't intended to live for long (only until we verify this doesn't have regressions in a complex app). Changelog: [internal] --- Reviewed By: sammy-SC Differential Revision: D50171297 fbshipit-source-id: dfc5526d8293c6d1169d8627fb1cdc7647a745db
rubennorte
force-pushed
the
export-D50171297
branch
from
0f5749a to
91ead50
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D50171297 |
rubennorte
added a commit
to rubennorte/react-native
that referenced
this pull request
Oct 17, 2023
…book#40945) Summary: Pull Request resolved: facebook#40945 This adds some temporary logic to configure the use of the modern version of RuntimeScheduler based on values coming from the app configuration. This logic is centralized in `ReactInstance` so from that point the code is completely cross-platform. This doesn't use `ReactNativeConfig`/`CoreFeatures` because they're initialized after the point where we need to access them for this use case. This way is a bit uglier but this isn't intended to live for long (only until we verify this doesn't have regressions in a complex app). Changelog: [internal] --- Reviewed By: sammy-SC Differential Revision: D50171297 fbshipit-source-id: 82586fa5e5c394d804b7d6e83f8102986be777d5
rubennorte
force-pushed
the
export-D50171297
branch
from
91ead50 to
78b8763
Compare
rubennorte
added a commit
to rubennorte/react-native
that referenced
this pull request
Oct 18, 2023
…book#40945) Summary: This adds some temporary logic to configure the use of the modern version of RuntimeScheduler based on values coming from the app configuration. This logic is centralized in `ReactInstance` so from that point the code is completely cross-platform. This doesn't use `ReactNativeConfig`/`CoreFeatures` because they're initialized after the point where we need to access them for this use case. This way is a bit uglier but this isn't intended to live for long (only until we verify this doesn't have regressions in a complex app). Changelog: [internal] --- Reviewed By: sammy-SC Differential Revision: D50171297
rubennorte
force-pushed
the
export-D50171297
branch
from
78b8763 to
c15017a
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D50171297 |
Summary: This removes an old experiment to implement microtasks in React Native (which is incorrect now that the runtime scheduler executes multiple tasks per runtime executor "task"). `drainMicrotasks` is a no-op at the moment in Hermes because the flag isn't set, so this code is essentially dead. We'll add the new iteration of microtasks in a following PR. Changelog: [internal] Reviewed By: christophpurrer Differential Revision: D49536251
…facebook#40875) Summary: This introduces a proxy for RuntimeScheduler so we can select between 2 different implementations at runtime (current implementation vs. new implementation, done in D49316881). Changelog: [internal] Reviewed By: javache, sammy-SC Differential Revision: D49316880
Summary: ## Summary This creates a new version of `RuntimeScheduler` that's intended to be backwards compatible but with a few notable changes: 1. `scheduleTask` is now thread-safe. 2. `scheduleWork` is now just an alias of `scheduleTask` with immediate priority (to preserve the yielding semantics it had over other tasks). 3. Yielding mechanism has changed, to make lower priority tasks to yield to higher priority tasks, instead of just yielding to `scheduleWork` and `executeNowOnTheSameThread`. We don't expect this to have any impact in performance or user perceivable behavior, so we consider it a short-lived refactor. When we validate this assumptions in a complex application we'll delete the old version and only keep the fork. ## Motivation The main motivation for this refactor is to reduce the amount of unnecessary interruptions of running tasks (via `shouldYield`) that are only used to schedule asynchronous tasks from native. The `scheduleWork` method is the only available mechanism exposed to native APIs to schedule work in the JS thread (as the existing version of `scheduleTask` is only meant to be called from JS). This mechanism **always** asks for any running tasks in the scheduler to yield, so these tasks are always considered to have the highest priority. This makes sense for discrete user events, but not for many other use cases coming from native (e.g.: notifying network responses could be UserBlocking, Normal or Low depending on the use case). We need a way to schedule tasks from native with other kinds of priorities, so we don't always have to interrupt what's currently executing if it has a higher priority than what we're scheduling. ## Changes **General APIs:** This centralizes scheduling in only 2 APIs in `RuntimeScheduler` (which already exist in the legacy version): * `scheduleTask`, which is non-blocking for the caller and can be used from any thread. This always uses the task queue in the scheduler and a new yielding mechanism. * `executeNowOnTheSameThread`, which is blocking for the caller and asks any task executing in the scheduler to yield. These tasks don't go through the task queue and instead queue through the existing synchronization mechanism in `RuntimeExecutor`. The yielding mechanism for these tasks is preserved. `scheduleWork` will be deprecated and it's just an alias for `scheduleTask` with an immediate priority (to preserve a similar behavior). **Yielding behavior:** Before, tasks would only yield to tasks scheduled via `scheduleWork` and `executeNowOnTheSameThread` (those tasks didn't go through the task queue). With this implementation, tasks would now yield to any task that has a higher position in the task queue. That means we reuse the existing mechanism to avoid lower priority tasks to never execute because higher priority tasks never stop coming. All tasks would yield to requests for synchronous access (via `executeNowOnTheSameThread`) as did the current implementation. Changelog: [internal] Reviewed By: javache, sammy-SC Differential Revision: D49316881
…book#40945) Summary: This adds some temporary logic to configure the use of the modern version of RuntimeScheduler based on values coming from the app configuration. This logic is centralized in `ReactInstance` so from that point the code is completely cross-platform. This doesn't use `ReactNativeConfig`/`CoreFeatures` because they're initialized after the point where we need to access them for this use case. This way is a bit uglier but this isn't intended to live for long (only until we verify this doesn't have regressions in a complex app). Changelog: [internal] --- Reviewed By: sammy-SC Differential Revision: D50171297
rubennorte
force-pushed
the
export-D50171297
branch
from
c15017a to
ee32b7e
Compare
Contributor
|
This pull request was exported from Phabricator. Differential Revision: D50171297 |
Contributor
|
This pull request has been merged in 87c08df. |
Othinn
pushed a commit
to Othinn/react-native
that referenced
this pull request
Oct 30, 2023
…book#40945) Summary: Pull Request resolved: facebook#40945 This adds some temporary logic to configure the use of the modern version of RuntimeScheduler based on values coming from the app configuration. This logic is centralized in `ReactInstance` so from that point the code is completely cross-platform. This doesn't use `ReactNativeConfig`/`CoreFeatures` because they're initialized after the point where we need to access them for this use case. This way is a bit uglier but this isn't intended to live for long (only until we verify this doesn't have regressions in a complex app). Changelog: [internal] --- Reviewed By: sammy-SC Differential Revision: D50171297 fbshipit-source-id: 8d96e228550cc6112ffe2abec4d531514b052f82
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary:
This adds some temporary logic to configure the use of the modern version of RuntimeScheduler based on values coming from the app configuration.
This logic is centralized in
ReactInstanceso from that point the code is completely cross-platform.This doesn't use
ReactNativeConfig/CoreFeaturesbecause they're initialized after the point where we need to access them for this use case. This way is a bit uglier but this isn't intended to live for long (only until we verify this doesn't have regressions in a complex app).Changelog: [internal]
Differential Revision: D50171297
See react-native-community/discussions-and-proposals#744