Expiration times #10426

acdlite · 2017-08-09T23:06:14Z

An expiration time represents a time in the future by which an update should flush. The priority of the update is related to the difference between the current clock time and the expiration time. This has the effect of increasing the priority of updates as time progresses, to prevent starvation.

acdlite · 2017-08-11T14:28:49Z

Confirms that this makes the Triangle demo work as expected. Still need to write some unit tests, but this is ready for review.

acdlite · 2017-08-11T21:52:53Z

Added unit tests. I confirmed that all the existing tests, including the sync DOM ones, are passing when expiration is enabled.

acdlite · 2017-08-11T21:56:50Z

src/renderers/testing/ReactTestRendererFiberEntry.js

@@ -249,6 +249,11 @@ var TestRenderer = ReactFiberReconciler({
  useSyncScheduling: true,

  getPublicInstance,
+
+  now(): number {
+    // Test renderer does not use expiration


@sebmarkbage I went back and forth on whether this should be an optional method. Landed on making it required because all renderers will need to account for features like expiration boundaries.

Since scheduleDeferredCallback is required it seems reasonable that this would be too since they kind of pair together.

acdlite · 2017-08-11T21:58:02Z

src/renderers/dom/fiber/ReactDOMFiberEntry.js

@@ -163,6 +163,22 @@ function shouldAutoFocusHostComponent(type: string, props: Props): boolean {
  return false;
 }

+// TODO: Better polyfill


Is this polyfill sufficient? Or am I overlooking something?

It's fine. Maybe we should colocate it with ReactDOMFrameScheduling to have all these in one place?

It is also performance.now aware: https://github.com/facebook/react/blob/master/src/renderers/shared/ReactDOMFrameScheduling.js#L73

sebmarkbage · 2017-08-11T23:00:38Z

src/renderers/dom/fiber/ReactDOMFiberEntry.js

+  window.performance &&
+  typeof window.performance.now === 'function'
+) {
+  now = function() {


Why not now = performance.now and now = Date.now?

performance.now needs to be bound, but looks like Date.now doesn't. I'll update.

flarnie · 2017-09-28T23:06:43Z

src/renderers/shared/fiber/ReactFiberCompleteWork.js

@@ -42,7 +42,7 @@ var {
  Fragment,
 } = ReactTypeOfWork;
 var {Placement, Ref, Update} = ReactTypeOfSideEffect;
-var {OffscreenPriority} = ReactPriorityLevel;
+var {Never} = ReactFiberExpirationTime;


I like that this name change decouples this expiration time from the UI context in which it's used. What we really mean is 'this might never finish updating, and that's ok' and that may or may not be literally "offscreen" in the UI.

flarnie · 2017-09-28T23:14:29Z

src/renderers/shared/fiber/ReactFiberExpirationTime.js

+  TaskPriority,
+  HighPriority,
+  LowPriority,
+  OffscreenPriority,


haha, I guess it's not a rename after all. ("OffscreenPriority" to "Never" expirationTime)

reactjs-bot · 2017-09-28T23:35:54Z

Deploy preview ready!

Built with commit 564c7e9

https://deploy-preview-10426--reactjs.netlify.com

acdlite · 2017-09-29T00:54:44Z

@gaearon I’ve rebased this locally, just haven’t pushed yet. I’ll work on getting all my PRs updated tomorrow.

sebmarkbage

I have more to review but left some nits in the meantime...

sebmarkbage · 2017-10-07T04:11:00Z

src/renderers/shared/ReactDOMFrameScheduling.js

@@ -37,6 +37,13 @@ if (__DEV__) {
  }
 }

+let now: () => number;
+if (typeof performance === 'object' && typeof performance.now === 'function') {
+  now = performance.now.bind(performance);


I'm not sure this trick is always safe cross-browser. Built-ins have all kinds of quirky behavior.

sebmarkbage · 2017-10-07T04:11:00Z

src/renderers/shared/ReactDOMFrameScheduling.js

+if (typeof performance === 'object' && typeof performance.now === 'function') {
+  now = performance.now.bind(performance);
+} else {
+  now = Date.now;


Are we sure this one works without a bind? Might be better to error on the safe side and just call it with the context instead of being clever.

sebmarkbage · 2017-10-07T04:11:00Z

src/renderers/shared/ReactDOMFrameScheduling.js

+    timeRemaining() {
+      // We assume that if we have a performance timer that the rAF callback
+      // gets a performance timer value. Not sure if this is always true.
+      return frameDeadline - now();


This call is very time sensitive since we call it in a hot loop. We should do everything we can to make it inlinable. Calling a bound function might just be the thing that breaks that. I'd prefer code duplication and simple code here.

sebmarkbage · 2017-10-07T04:11:00Z

src/renderers/testing/ReactTestRendererFiberEntry.js

@@ -249,6 +249,11 @@ var TestRenderer = ReactFiberReconciler({
  useSyncScheduling: true,

  getPublicInstance,
+
+  now(): number {
+    // Test renderer does not use expiration


Since scheduleDeferredCallback is required it seems reasonable that this would be too since they kind of pair together.

sebmarkbage · 2017-10-07T04:11:00Z

src/renderers/shared/fiber/ReactFiberExpirationTime.js

+const UNIT_SIZE = 10;
+const MAGIC_NUMBER_OFFSET = 10;
+
+exports.Done = Done;


How about keeping the NoWork name here? I also wonder if the value should be named something that has something that indicates that it is really a dirty flag and that this essentially means false. Done stood out to me as not indicating that well unless you already know how it works.

Yeah I'm not happy with any of the names here, was just trying to avoid a naming clash. NoWork seems fine, can import the other one as NoWorkPriority if I need to.

sebmarkbage · 2017-10-07T04:11:00Z

src/renderers/shared/fiber/ReactFiberExpirationTime.js

+const Done = 0;
+const Sync = 1;
+const Task = 2;
+const Never = Number.MAX_SAFE_INTEGER;


This number is way too high. Since VMs (really mostly just V8) tries to optimize for integer types when they can, sometimes they want them to fit into a single 32bit word on 32-bit systems. This number might cause them to deopt that assumption and have to expand the slot.

It should be 2^30 or whatever is the highest we can fit in 31-bit.

sebmarkbage · 2017-10-07T04:11:00Z

src/renderers/shared/fiber/ReactFiberExpirationTime.js

+// 1 unit of expiration time represents 10ms.
+function msToExpirationTime(ms: number): ExpirationTime {
+  // Always add an offset so that we don't clash with the magic number for Done.
+  return Math.round(ms / UNIT_SIZE) + MAGIC_NUMBER_OFFSET;


Why round? (Faster to do (ms / UNIT_SIZE) | 0.)

@sebmarkbage I am a little confused, ms / UNIT_SIZE) | 0 equals Math.floor rather than Math.round, right?

sebmarkbage · 2017-10-07T04:11:00Z

src/renderers/shared/fiber/ReactFiberExpirationTime.js

+exports.msToExpirationTime = msToExpirationTime;
+
+function ceiling(num: number, precision: number): number {
+  return Math.ceil(Math.ceil(num * precision) / precision);


Maybe ... | 0 + 1 would be faster and good enough?

sebmarkbage · 2017-10-07T07:28:38Z

src/renderers/shared/fiber/ReactFiberExpirationTime.js

+// Given the current clock time and an expiration time, returns the
+// corresponding priority level. The more time has advanced, the higher the
+// priority level.
+function expirationTimeToPriorityLevel(


Is this unused? I was kind of expecting that we'd completely remove the concept of PriorityLevel and delete the associated files.

What do you mean "remove"?We are based on "ReactPriorityLevel" to scheduling, right?

This function used after checking the deadline so we know whether to resume or yield, and after a setState to schedule a callback. https://github.com/acdlite/react/blob/867aeac5819c997bfee0a0ddf7d973624ef5e3d2/src/renderers/shared/fiber/ReactFiberScheduler.js#L1475

PriorityLevel is also used for going the other direction: priority -> expiration time. When scheduling an update.

sebmarkbage · 2017-10-07T07:28:38Z

src/renderers/shared/fiber/ReactFiberScheduler.js

@@ -801,14 +820,12 @@ module.exports = function<T, P, I, TI, PI, C, CX, PL>(
              'a bug in React. Please file an issue.',
          );
          // We just completed a root. Commit it now.
-          priorityContext = TaskPriority;


Why is this not needed anymore?

Rather than change the priority context every time we start on a new bucket, and then change it to task in the commit phase, and then reset it to the previous value when an error is thrown, I found it was simpler to move this complexity to getPriorityContext. https://github.com/acdlite/react/blob/867aeac5819c997bfee0a0ddf7d973624ef5e3d2/src/renderers/shared/fiber/ReactFiberScheduler.js#L1520-L1552

sebmarkbage · 2017-10-07T07:29:12Z

After the nits I think this is good to go.

acdlite · 2017-10-08T03:35:07Z

Addressed nits

sebmarkbage · 2017-10-10T00:52:04Z

src/renderers/shared/fiber/ReactFiberExpirationTime.js

+const Never = Math.pow(2, 31) - 1; // Max int32
+
+const UNIT_SIZE = 10;
+const MAGIC_NUMBER_OFFSET = 10;


Why is this 10 and not 3?

@acdlite Any explanation here?

sebmarkbage · 2017-10-10T00:52:04Z

src/renderers/shared/fiber/ReactFiberExpirationTime.js

+const NoWork = 0;
+const Sync = 1;
+const Task = 2;
+const Never = Math.pow(2, 31) - 1; // Max int32


Should we just hardcode the value so we don't have to hope compilers do it for us?

sebmarkbage · 2017-10-10T01:07:27Z

src/renderers/shared/fiber/ReactFiberUpdateQueue.js

@@ -104,6 +83,7 @@ function createUpdateQueue(): UpdateQueue {
 function cloneUpdate(update: Update): Update {
  return {
    priorityLevel: update.priorityLevel,


If we get rid of coalescing we can get rid of this.

An expiration time represents a time in the future by which an update should flush. The priority of the update is related to the difference between the current clock time and the expiration time. This has the effect of increasing the priority of updates as time progresses, to prevent starvation. This lays the initial groundwork for expiration times without changing any behavior. Future commits will replace work priority with expiration times.

Instead of a priority, a fiber has an expiration time that represents a point in the future by which it should render. Pending updates still have priorities so that they can be coalesced. We use a host config method to read the current time. This commit implements everything except that method, which currently returns a constant value. So this just proves that expiration times work the same as priorities when time is frozen. Subsequent commits will show the effect of advancing time.

shouldComponentUpdate was removed from functional components. Running the demo shows, now that expiration is enabled, the demo does not starve. (Still won't run smoothly until we add back the ability to resume interrupted work.)

There are a few cases related to sync mode where we need to distinguish between work that is scheduled as task and work that is treated like task because it expires. For example, batchedUpdates. We don't want to perform any work until the end of the batch, regardless of how much time has elapsed.

- Rename Done -> NoWork - Use max int32 instead of max safe int - Use bitwise operations instead of Math functions

acdlite · 2017-10-10T01:40:33Z

I'll remove coalescing and PriorityLevel in a follow-up.

NE-SmallTown · 2018-04-12T03:36:01Z

src/renderers/shared/ReactDOMFrameScheduling.js

-        },
-  };
+  var frameDeadlineObject;
+  if (hasNativePerformanceNow) {


Why we do the judgement again instead calling now() directly?

Related prs: + facebook/react#10715 + facebook/react#10426

facebook-github-bot added the CLA Signed label Aug 9, 2017

acdlite changed the title ~~[Work-in-progress] Assign expiration times to updates~~ [Work-in-progress] Expiration times Aug 9, 2017

acdlite force-pushed the expiration branch from 4af438f to 7ef34ab Compare August 10, 2017 00:03

acdlite mentioned this pull request Aug 10, 2017

[Umbrella] Async rendering #8830

Closed

17 tasks

acdlite force-pushed the expiration branch 3 times, most recently from ee127ed to 796db96 Compare August 10, 2017 22:46

acdlite requested a review from sebmarkbage August 11, 2017 14:28

acdlite changed the title ~~[Work-in-progress] Expiration times~~ Expiration times Aug 11, 2017

acdlite force-pushed the expiration branch from dd7229a to 8972ae8 Compare August 11, 2017 20:54

acdlite commented Aug 11, 2017

View changes

acdlite force-pushed the expiration branch 2 times, most recently from 67388a1 to 14e68e3 Compare August 11, 2017 22:08

sebmarkbage reviewed Aug 11, 2017

View changes

acdlite force-pushed the expiration branch 3 times, most recently from da21094 to 43360d9 Compare August 18, 2017 23:08

This was referenced Sep 6, 2017

New API for top-level updates #10624

Closed

Pass pendingProps as argument in createWorkInProgress #10643

Closed

acdlite force-pushed the expiration branch 2 times, most recently from 0d4f363 to 36244ac Compare September 14, 2017 18:35

acdlite mentioned this pull request Sep 15, 2017

Deterministic updates #10715

Merged

acdlite mentioned this pull request Sep 28, 2017

Feature Request: Global didUpdate() #10900

Closed

flarnie reviewed Sep 28, 2017

View changes

sebmarkbage requested changes Oct 7, 2017

View changes

sebmarkbage reviewed Oct 7, 2017

View changes

acdlite force-pushed the expiration branch 2 times, most recently from 29498c1 to 0d6a636 Compare October 8, 2017 03:34

acdlite force-pushed the expiration branch 2 times, most recently from 2bbd1c5 to ca14382 Compare October 9, 2017 22:05

sebmarkbage approved these changes Oct 10, 2017

View changes

sebmarkbage reviewed Oct 10, 2017

View changes

acdlite force-pushed the expiration branch from ca14382 to 36244ac Compare October 10, 2017 01:32

acdlite added 10 commits October 9, 2017 18:32

Triangle Demo should use a class

ec3435a

shouldComponentUpdate was removed from functional components. Running the demo shows, now that expiration is enabled, the demo does not starve. (Still won't run smoothly until we add back the ability to resume interrupted work.)

Use current time to calculate expiration time

3913a05

Add unit tests for expiration and coalescing

608e5b4

Delete unnecessary abstraction

91bd584

Move performance.now polyfill to ReactDOMFrameScheduling

a4c602c

Add expiration to fuzz tester

d89d6a3

Expiration nits

721fdfc

- Rename Done -> NoWork - Use max int32 instead of max safe int - Use bitwise operations instead of Math functions

acdlite force-pushed the expiration branch from 36244ac to 721fdfc Compare October 10, 2017 01:39

acdlite merged commit 3019210 into facebook:master Oct 10, 2017

gaearon mentioned this pull request Oct 12, 2017

Investigate the nested update overflow caused by coalescing #11207

Closed

acdlite mentioned this pull request Oct 12, 2017

[Not for commit] Demonstrate async bug #11210

Closed

NE-SmallTown reviewed Apr 12, 2018

View changes

MQuy added a commit to MQuy/qreact that referenced this pull request Jan 26, 2019

Support priority scheduling

5314b6a

Related prs: + facebook/react#10715 + facebook/react#10426

hushicai mentioned this pull request Apr 20, 2019

React Fiber hushicai/hushicai.github.io#44

Open

gaearon mentioned this pull request Mar 29, 2022

Add React 18 changelog #24195

Merged

Feb	MAR	Apr
	16
2022	2023	2024

Expiration times #10426

Expiration times #10426

acdlite commented Aug 9, 2017 •

edited

acdlite commented Aug 11, 2017

acdlite commented Aug 11, 2017 •

edited

acdlite Aug 11, 2017

sebmarkbage Oct 7, 2017

acdlite Aug 11, 2017

sebmarkbage Aug 11, 2017

sebmarkbage Aug 11, 2017

acdlite Aug 11, 2017

flarnie Sep 28, 2017

flarnie Sep 28, 2017

reactjs-bot commented Sep 28, 2017 •

edited

acdlite commented Sep 29, 2017

sebmarkbage left a comment

sebmarkbage Oct 7, 2017

sebmarkbage Oct 7, 2017

sebmarkbage Oct 7, 2017

sebmarkbage Oct 7, 2017

sebmarkbage Oct 7, 2017

acdlite Oct 8, 2017

sebmarkbage Oct 7, 2017

sebmarkbage Oct 7, 2017

NE-SmallTown Apr 12, 2018 •

edited

sebmarkbage Oct 7, 2017

sebmarkbage Oct 7, 2017

NE-SmallTown Oct 7, 2017 •

edited

acdlite Oct 8, 2017

sebmarkbage Oct 7, 2017

acdlite Oct 8, 2017

sebmarkbage commented Oct 7, 2017

acdlite commented Oct 8, 2017

sebmarkbage Oct 10, 2017

NE-SmallTown Apr 12, 2018

sebmarkbage Oct 10, 2017

sebmarkbage Oct 10, 2017

acdlite commented Oct 10, 2017

NE-SmallTown Apr 12, 2018

Expiration times #10426

Expiration times #10426

Conversation

acdlite commented Aug 9, 2017 • edited

acdlite commented Aug 11, 2017

acdlite commented Aug 11, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

reactjs-bot commented Sep 28, 2017 • edited

acdlite commented Sep 29, 2017

sebmarkbage left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NE-SmallTown Apr 12, 2018 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

NE-SmallTown Oct 7, 2017 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebmarkbage commented Oct 7, 2017

acdlite commented Oct 8, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acdlite commented Oct 10, 2017

Choose a reason for hiding this comment

acdlite commented Aug 9, 2017 •

edited

acdlite commented Aug 11, 2017 •

edited

reactjs-bot commented Sep 28, 2017 •

edited

NE-SmallTown Apr 12, 2018 •

edited

NE-SmallTown Oct 7, 2017 •

edited