Disable Just Noticeable Difference heuristic #15423

acdlite · 2019-04-15T21:45:20Z

c25c59c resulted in bugs during internal testing. I don't know why yet, but removing the Just Noticeable Difference heuristic was sufficient to fix/hide the bugs again. In the interest of fixing this downstream quickly, let's revert it and land it again once we've figured out the root cause.

Specially, what I've done is this PR is revert computeMsUntilTimeout to what is what in the last known non-buggy commit:

react/packages/react-reconciler/src/ReactFiberScheduler.new.js

Lines 1818 to 1833 in 3e2e930

    
           function computeMsUntilTimeout(mostRecentEventTime: ExpirationTime) { 
        
             if (disableYielding) { 
        
               // Timeout immediately when yielding is disabled. 
        
               return 0; 
        
             } 
        
             const eventTimeMs: number = inferTimeFromExpirationTime(mostRecentEventTime); 
        
             const currentTimeMs: number = now(); 
        
             const timeElapsed = currentTimeMs - eventTimeMs; 
        
             // TODO: Account for the Just Noticeable Difference 
        
             const timeoutMs = 150; 
        
             const msUntilTimeout = timeoutMs - timeElapsed; 
        
             // This is the value that is passed to `setTimeout`. 
        
             return msUntilTimeout < 0 ? 0 : msUntilTimeout; 
        
           }

c25c59c resulted in bugs during internal testing. I don't know why yet, but removing the Just Noticable Difference heuristic was sufficient to fix/hide the bugs again. In the interest of fixing this downstream quickly, let's revert it and land it again once we've figured out the root cause.

acdlite · 2019-04-15T21:49:26Z

~~Oh wait this isn't right. One sec.~~ Ok fixed

sizebot · 2019-04-15T21:55:55Z

ReactDOM: size: -0.1%, gzip: -0.2%

Details of bundled changes.

Comparing: 9ebe176...d77d230

react-dom

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
react-dom.development.js	-0.0%	0.0%	810.69 KB	810.64 KB	185.02 KB	185.03 KB	UMD_DEV
react-dom.production.min.js	-0.1%	-0.2%	103.58 KB	103.46 KB	33.65 KB	33.58 KB	UMD_PROD
react-dom.profiling.min.js	-0.1%	-0.2%	106.73 KB	106.61 KB	34.6 KB	34.53 KB	UMD_PROFILING
react-dom.development.js	-0.0%	0.0%	805.17 KB	805.13 KB	183.49 KB	183.49 KB	NODE_DEV
react-dom.production.min.js	-0.1%	-0.2%	103.57 KB	103.45 KB	33.1 KB	33.02 KB	NODE_PROD
react-dom.profiling.min.js	-0.1%	-0.2%	106.9 KB	106.77 KB	33.95 KB	33.87 KB	NODE_PROFILING
ReactDOM-dev.js	-0.0%	-0.0%	829.52 KB	829.36 KB	184.87 KB	184.86 KB	FB_WWW_DEV
ReactDOM-prod.js	-0.2%	-0.2%	337.35 KB	336.66 KB	62.68 KB	62.55 KB	FB_WWW_PROD
ReactDOM-profiling.js	-0.2%	-0.2%	342.74 KB	342.04 KB	63.61 KB	63.48 KB	FB_WWW_PROFILING
react-dom-unstable-fire.development.js	-0.0%	0.0%	811.02 KB	810.97 KB	185.16 KB	185.17 KB	UMD_DEV
react-dom-unstable-fire.production.min.js	-0.1%	-0.2%	103.6 KB	103.47 KB	33.66 KB	33.59 KB	UMD_PROD
react-dom-unstable-fire.profiling.min.js	-0.1%	-0.2%	106.75 KB	106.62 KB	34.61 KB	34.54 KB	UMD_PROFILING
react-dom-unstable-fire.development.js	-0.0%	0.0%	805.5 KB	805.45 KB	183.62 KB	183.63 KB	NODE_DEV
react-dom-unstable-fire.production.min.js	-0.1%	-0.2%	103.59 KB	103.46 KB	33.11 KB	33.03 KB	NODE_PROD
react-dom-unstable-fire.profiling.min.js	-0.1%	-0.2%	106.91 KB	106.79 KB	33.96 KB	33.88 KB	NODE_PROFILING
ReactFire-dev.js	-0.0%	-0.0%	828.71 KB	828.55 KB	184.89 KB	184.87 KB	FB_WWW_DEV
ReactFire-prod.js	-0.2%	-0.2%	325.33 KB	324.55 KB	60.23 KB	60.09 KB	FB_WWW_PROD
ReactFire-profiling.js	-0.2%	-0.2%	330.68 KB	329.9 KB	61.19 KB	61.05 KB	FB_WWW_PROFILING
react-dom-test-utils.production.min.js	0.0%	🔺+0.1%	10.52 KB	10.52 KB	3.89 KB	3.9 KB	UMD_PROD
react-dom-test-utils.development.js	0.0%	0.0%	53.91 KB	53.91 KB	14.91 KB	14.91 KB	NODE_DEV
react-dom-test-utils.production.min.js	0.0%	0.0%	10.31 KB	10.31 KB	3.82 KB	3.82 KB	NODE_PROD
react-dom-unstable-native-dependencies.development.js	0.0%	0.0%	60.76 KB	60.76 KB	15.85 KB	15.85 KB	UMD_DEV
react-dom-unstable-native-dependencies.development.js	0.0%	0.0%	60.43 KB	60.43 KB	15.72 KB	15.72 KB	NODE_DEV
react-dom-server.browser.development.js	0.0%	0.0%	136.73 KB	136.73 KB	35.92 KB	35.92 KB	UMD_DEV
react-dom-server.browser.production.min.js	0.0%	0.0%	19.18 KB	19.18 KB	7.21 KB	7.21 KB	UMD_PROD
react-dom-server.browser.development.js	0.0%	0.0%	132.86 KB	132.86 KB	34.99 KB	34.99 KB	NODE_DEV
react-dom-server.browser.production.min.js	0.0%	0.0%	19.1 KB	19.1 KB	7.2 KB	7.2 KB	NODE_PROD
ReactDOMServer-dev.js	0.0%	0.0%	134.95 KB	134.95 KB	34.61 KB	34.61 KB	FB_WWW_DEV
react-dom-server.node.development.js	0.0%	0.0%	134.8 KB	134.8 KB	35.53 KB	35.54 KB	NODE_DEV
react-dom-server.node.production.min.js	0.0%	0.0%	19.96 KB	19.96 KB	7.51 KB	7.51 KB	NODE_PROD
react-dom-unstable-fizz.browser.development.js	0.0%	+0.1%	3.66 KB	3.66 KB	1.45 KB	1.45 KB	UMD_DEV
react-dom-unstable-fizz.browser.production.min.js	0.0%	-0.1%	1.21 KB	1.21 KB	706 B	705 B	UMD_PROD
react-dom-unstable-fizz.browser.development.js	0.0%	+0.1%	3.49 KB	3.49 KB	1.41 KB	1.41 KB	NODE_DEV
react-dom-unstable-fizz.node.development.js	0.0%	+0.1%	3.74 KB	3.74 KB	1.43 KB	1.43 KB	NODE_DEV
react-dom-unstable-fizz.node.production.min.js	0.0%	🔺+0.2%	1.1 KB	1.1 KB	666 B	667 B	NODE_PROD

react-art

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
react-art.development.js	-0.0%	0.0%	556.46 KB	556.41 KB	121.46 KB	121.46 KB	UMD_DEV
react-art.production.min.js	-0.1%	-0.2%	95.49 KB	95.36 KB	29.31 KB	29.25 KB	UMD_PROD
react-art.development.js	-0.0%	0.0%	487.49 KB	487.44 KB	104.07 KB	104.08 KB	NODE_DEV
react-art.production.min.js	-0.2%	-0.4%	60.46 KB	60.33 KB	18.61 KB	18.53 KB	NODE_PROD
ReactART-dev.js	-0.0%	-0.0%	496.65 KB	496.49 KB	103.13 KB	103.13 KB	FB_WWW_DEV
ReactART-prod.js	-0.4%	-0.4%	194.93 KB	194.16 KB	33.15 KB	33.01 KB	FB_WWW_PROD

react-native-renderer

File	Filesize Diff	Gzip Diff	Prev Size	Current Size	Prev Gzip	Current Gzip	ENV
ReactNativeRenderer-dev.js	-0.0%	-0.0%	628.2 KB	628.04 KB	133.33 KB	133.31 KB	RN_FB_DEV
ReactNativeRenderer-prod.js	-0.3%	-0.3%	243.71 KB	243.02 KB	42.35 KB	42.21 KB	RN_FB_PROD
ReactNativeRenderer-profiling.js	-0.3%	-0.3%	251.89 KB	251.21 KB	43.92 KB	43.78 KB	RN_FB_PROFILING
ReactNativeRenderer-dev.js	-0.0%	-0.0%	628.12 KB	627.96 KB	133.29 KB	133.27 KB	RN_OSS_DEV
ReactNativeRenderer-prod.js	-0.3%	-0.3%	243.72 KB	243.04 KB	42.35 KB	42.21 KB	RN_OSS_PROD
ReactNativeRenderer-profiling.js	-0.3%	-0.3%	251.91 KB	251.22 KB	43.92 KB	43.78 KB	RN_OSS_PROFILING
ReactFabric-dev.js	-0.0%	-0.0%	616.89 KB	616.73 KB	130.62 KB	130.6 KB	RN_FB_DEV
ReactFabric-prod.js	-0.3%	-0.4%	237.48 KB	236.79 KB	41.09 KB	40.95 KB	RN_FB_PROD
ReactFabric-profiling.js	-0.3%	-0.3%	245.13 KB	244.45 KB	42.7 KB	42.57 KB	RN_FB_PROFILING
ReactFabric-dev.js	-0.0%	-0.0%	616.79 KB	616.63 KB	130.58 KB	130.57 KB	RN_OSS_DEV
ReactFabric-prod.js	-0.3%	-0.4%	237.49 KB	236.8 KB	41.09 KB	40.94 KB	RN_OSS_PROD
ReactFabric-profiling.js	-0.3%	-0.3%	245.14 KB	244.46 KB	42.7 KB	42.57 KB	RN_OSS_PROFILING

Generated by 🚫 dangerJS

acdlite · 2019-04-15T22:07:35Z

packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js

  });
 });
-
-// TODO:


This is unrelated; deleting because I noticed it and they are outdated.

bvaughn

I think w e normally would revert the commit that landed this change, but since the scheduler refactor landed in between, that would be more difficult.

acdlite · 2019-04-15T23:01:11Z

Closing because I found a proper fix. Will open a new PR.

acdlite · 2019-04-15T23:11:33Z

Actual fix landed in: #15424

acdlite requested review from bvaughn, gaearon, necolas and sebmarkbage April 15, 2019 21:45

facebook-github-bot added the CLA Signed label Apr 15, 2019

acdlite changed the title ~~Disable Just Noticable Difference heuristic~~ Disable Just Noticeable Difference heuristic Apr 15, 2019

acdlite force-pushed the revert-jnd branch 2 times, most recently from 102d07f to b88c058 Compare April 15, 2019 22:05

acdlite commented Apr 15, 2019

View reviewed changes

acdlite force-pushed the revert-jnd branch from b88c058 to d77d230 Compare April 15, 2019 22:09

bvaughn approved these changes Apr 15, 2019

View reviewed changes

acdlite closed this Apr 15, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Disable Just Noticeable Difference heuristic #15423

Disable Just Noticeable Difference heuristic #15423

Uh oh!

acdlite commented Apr 15, 2019 •

edited

Loading

Uh oh!

acdlite commented Apr 15, 2019 •

edited

Loading

Uh oh!

sizebot commented Apr 15, 2019 •

edited

Loading

react-dom

react-art

react-native-renderer

Uh oh!

acdlite Apr 15, 2019

Uh oh!

bvaughn left a comment

Uh oh!

acdlite commented Apr 15, 2019

Uh oh!

acdlite commented Apr 15, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

	function computeMsUntilTimeout(mostRecentEventTime: ExpirationTime) {
	if (disableYielding) {
	// Timeout immediately when yielding is disabled.
	return 0;
	}

	const eventTimeMs: number = inferTimeFromExpirationTime(mostRecentEventTime);
	const currentTimeMs: number = now();
	const timeElapsed = currentTimeMs - eventTimeMs;

	// TODO: Account for the Just Noticeable Difference
	const timeoutMs = 150;
	const msUntilTimeout = timeoutMs - timeElapsed;
	// This is the value that is passed to `setTimeout`.
	return msUntilTimeout < 0 ? 0 : msUntilTimeout;
	}

Disable Just Noticeable Difference heuristic #15423

Disable Just Noticeable Difference heuristic #15423

Uh oh!

Conversation

acdlite commented Apr 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

acdlite commented Apr 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sizebot commented Apr 15, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

react-dom

react-art

react-native-renderer

Uh oh!

acdlite Apr 15, 2019

Choose a reason for hiding this comment

Uh oh!

bvaughn left a comment

Choose a reason for hiding this comment

Uh oh!

acdlite commented Apr 15, 2019

Uh oh!

acdlite commented Apr 15, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

acdlite commented Apr 15, 2019 •

edited

Loading

acdlite commented Apr 15, 2019 •

edited

Loading

sizebot commented Apr 15, 2019 •

edited

Loading