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.

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

[Scheduler] Check for continuous input events #22107

Merged
merged 3 commits into from
Aug 16, 2021

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented Aug 16, 2021

isInputPending supports a includeContinuous option. When set to true, the method will check for pending continuous inputs, like mousemove, in addition to discrete ones, like click.

We will only check for pending continuous inputs if we've blocked the main thread for a non-negligible amount of time. If we've only blocked the main thread for, say, a few frames, then we'll only check for discrete inputs.

I wrote a test for this but didn't include it because we haven't yet set up the gate flag infra to work with Scheduler feature flags. For now, I ran the test locally.

Currently in `shouldYield`, we compare the current time to a deadline
that is pre-computed at the beginning of the current chunk.

However, since we use different deadlines depending on whether an input
event is pending, it makes more sense to track the start of the current
chunk and check how much time has elapsed since then.

Doesn't change any behavior, just refactors the logic.
`isInputPending` supports a `includeContinuous` option. When set to
`true`, the method will check for pending continuous inputs, like
`mousemove`, in addition to discrete ones, like `click`.

We will only check for pending continuous inputs if we've blocked the
main thread for a non-neglible amount of time. If we've only blocked
the main thread for, say, a few frames, then we'll only check for
discrete inputs.

I wrote a test for this but didn't include it because we haven't yet set
up the `gate` flag infra to work with Scheduler feature flags. For now,
I ran the test locally.
@facebook-github-bot facebook-github-bot added CLA Signed React Core Team Opened by a member of the React Core Team labels Aug 16, 2021
@acdlite
Copy link
Collaborator Author

acdlite commented Aug 16, 2021

cc: @acomminos GitHub wouldn't let me request a review from you for some reason

@acdlite acdlite force-pushed the scheduler-check-for-continuous branch from b158495 to 5b81d30 Compare August 16, 2021 22:27
@sizebot
Copy link

sizebot commented Aug 16, 2021

Comparing: 152ecce...5cce438

Critical size changes

Includes critical production bundles, as well as any change greater than 2%:

Name +/- Base Current +/- gzip Base gzip Current gzip
oss-stable/react-dom/cjs/react-dom.production.min.js = 127.58 kB 127.58 kB = 40.72 kB 40.72 kB
oss-experimental/react-dom/cjs/react-dom.production.min.js = 130.40 kB 130.40 kB = 41.65 kB 41.65 kB
facebook-www/ReactDOM-prod.classic.js = 405.16 kB 405.16 kB = 75.04 kB 75.04 kB
facebook-www/ReactDOM-prod.modern.js = 393.72 kB 393.72 kB = 73.32 kB 73.32 kB
facebook-www/ReactDOMForked-prod.classic.js = 405.16 kB 405.16 kB = 75.05 kB 75.05 kB
oss-experimental/scheduler/cjs/scheduler.production.min.js +4.38% 4.07 kB 4.24 kB +3.30% 1.73 kB 1.79 kB
oss-stable-semver/scheduler/cjs/scheduler.production.min.js +4.38% 4.07 kB 4.24 kB +3.30% 1.73 kB 1.79 kB
oss-stable/scheduler/cjs/scheduler.production.min.js +4.38% 4.07 kB 4.24 kB +3.30% 1.73 kB 1.79 kB
facebook-www/Scheduler-dev.classic.js +2.20% 23.18 kB 23.68 kB +2.65% 6.26 kB 6.43 kB
facebook-www/Scheduler-dev.modern.js +2.20% 23.18 kB 23.68 kB +2.65% 6.26 kB 6.43 kB
facebook-react-native/scheduler/cjs/Scheduler-prod.js +2.03% 10.07 kB 10.27 kB +2.20% 2.54 kB 2.60 kB
facebook-react-native/scheduler/cjs/Scheduler-profiling.js +2.03% 10.07 kB 10.27 kB +2.20% 2.54 kB 2.60 kB

Significant size changes

Includes any change greater than 0.2%:

Expand to show
Name +/- Base Current +/- gzip Base gzip Current gzip
oss-experimental/scheduler/cjs/scheduler.production.min.js +4.38% 4.07 kB 4.24 kB +3.30% 1.73 kB 1.79 kB
oss-stable-semver/scheduler/cjs/scheduler.production.min.js +4.38% 4.07 kB 4.24 kB +3.30% 1.73 kB 1.79 kB
oss-stable/scheduler/cjs/scheduler.production.min.js +4.38% 4.07 kB 4.24 kB +3.30% 1.73 kB 1.79 kB
facebook-www/Scheduler-dev.classic.js +2.20% 23.18 kB 23.68 kB +2.65% 6.26 kB 6.43 kB
facebook-www/Scheduler-dev.modern.js +2.20% 23.18 kB 23.68 kB +2.65% 6.26 kB 6.43 kB
facebook-react-native/scheduler/cjs/Scheduler-prod.js +2.03% 10.07 kB 10.27 kB +2.20% 2.54 kB 2.60 kB
facebook-react-native/scheduler/cjs/Scheduler-profiling.js +2.03% 10.07 kB 10.27 kB +2.20% 2.54 kB 2.60 kB
facebook-react-native/scheduler/cjs/Scheduler-dev.js +1.72% 16.62 kB 16.91 kB +0.96% 4.77 kB 4.81 kB
facebook-www/Scheduler-prod.classic.js +1.67% 11.04 kB 11.22 kB +2.43% 2.76 kB 2.83 kB
facebook-www/Scheduler-prod.modern.js +1.67% 11.04 kB 11.22 kB +2.43% 2.76 kB 2.83 kB
oss-stable-semver/react/umd/react.profiling.min.js +1.65% 11.00 kB 11.18 kB +1.18% 4.41 kB 4.47 kB
oss-stable/react/umd/react.profiling.min.js +1.65% 11.00 kB 11.18 kB +1.18% 4.41 kB 4.47 kB
oss-stable-semver/react/umd/react.production.min.js +1.65% 11.00 kB 11.18 kB +1.18% 4.41 kB 4.47 kB
oss-stable/react/umd/react.production.min.js +1.65% 11.00 kB 11.18 kB +1.18% 4.41 kB 4.47 kB
oss-experimental/scheduler/cjs/scheduler.development.js +1.64% 16.63 kB 16.90 kB +0.86% 4.77 kB 4.81 kB
oss-stable-semver/scheduler/cjs/scheduler.development.js +1.64% 16.63 kB 16.90 kB +0.86% 4.77 kB 4.81 kB
oss-stable/scheduler/cjs/scheduler.development.js +1.64% 16.63 kB 16.90 kB +0.86% 4.77 kB 4.81 kB
oss-experimental/react/umd/react.profiling.min.js +1.60% 11.46 kB 11.64 kB +1.10% 4.55 kB 4.60 kB
oss-experimental/react/umd/react.production.min.js +1.60% 11.46 kB 11.64 kB +1.14% 4.55 kB 4.60 kB
facebook-www/Scheduler-profiling.classic.js +1.25% 14.74 kB 14.93 kB +1.85% 3.46 kB 3.53 kB
facebook-www/Scheduler-profiling.modern.js +1.25% 14.74 kB 14.93 kB +1.85% 3.46 kB 3.53 kB
oss-stable-semver/react/umd/react.development.js +0.26% 106.87 kB 107.14 kB +0.17% 27.33 kB 27.38 kB
oss-stable/react/umd/react.development.js +0.26% 106.87 kB 107.14 kB +0.17% 27.33 kB 27.38 kB
oss-experimental/react/umd/react.development.js +0.26% 107.35 kB 107.63 kB +0.16% 27.37 kB 27.41 kB

Generated by 🚫 dangerJS against 5cce438

@acdlite acdlite force-pushed the scheduler-check-for-continuous branch from 5b81d30 to e28bcce Compare August 16, 2021 22:39
Copy link

@acomminos acomminos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

} else if (timeElapsed < maxInterval) {
// Yield if there's either a pending discrete or continuous input.
if (isInputPending !== null) {
return isInputPending({includeContinuous: true});

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we declare the arg above as continuousOptions (or something similar) in module scope to avoid an object allocation each call?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea

@@ -499,7 +515,7 @@ const performWorkUntilDeadline = () => {
// Yield after `yieldInterval` ms, regardless of where we are in the vsync

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this comment needs to be updated :)

@acdlite acdlite merged commit aebf3b4 into facebook:main Aug 16, 2021
facebook-github-bot pushed a commit to facebook/react-native that referenced this pull request Aug 17, 2021
Summary:
This sync includes the following changes:
- **[424fe5870](facebook/react@424fe5870 )**: Revert "Show a soft error when a text string or number is supplied as a child to non text wrappers ([#21953](facebook/react#21953))" ([#22108](facebook/react#22108)) //<Sota>//
- **[aebf3b456](facebook/react@aebf3b456 )**: [Scheduler] Check for continuous input events ([#22107](facebook/react#22107)) //<Andrew Clark>//
- **[e9b2028b3](facebook/react@e9b2028b3 )**: Show a soft error when a text string or number is supplied as a child to non text wrappers ([#21953](facebook/react#21953)) //<Sota>//
- **[ecd73e17b](facebook/react@ecd73e17b )**: Enable enableSuspenseLayoutEffectSemantics flag statically for Facebook ([#22050](facebook/react#22050)) //<Brian Vaughn>//
- **[a8725a3e6](facebook/react@a8725a3e6 )**: Scheduling profiler: Added lane labels and durations to React measures ([#22029](facebook/react#22029)) //<Brian Vaughn>//

Changelog:
[General][Changed] - React Native sync for revisions 19092ac...5634ed1

jest_e2e[run_all_tests]

Reviewed By: kacieb

Differential Revision: D30225923

fbshipit-source-id: 562895d3e0d264f40770dadb89d4a16241967c4c
zhengjitf pushed a commit to zhengjitf/react that referenced this pull request Apr 15, 2022
* [Scheduler] Track start of current chunk

Currently in `shouldYield`, we compare the current time to a deadline
that is pre-computed at the beginning of the current chunk.

However, since we use different deadlines depending on whether an input
event is pending, it makes more sense to track the start of the current
chunk and check how much time has elapsed since then.

Doesn't change any behavior, just refactors the logic.

* [Scheduler] Check for continuous input events

`isInputPending` supports a `includeContinuous` option. When set to
`true`, the method will check for pending continuous inputs, like
`mousemove`, in addition to discrete ones, like `click`.

We will only check for pending continuous inputs if we've blocked the
main thread for a non-neglible amount of time. If we've only blocked
the main thread for, say, a few frames, then we'll only check for
discrete inputs.

I wrote a test for this but didn't include it because we haven't yet set
up the `gate` flag infra to work with Scheduler feature flags. For now,
I ran the test locally.

* Review nits
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed React Core Team Opened by a member of the React Core Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants