From 392277f0abc2251ea93b7debad4ae86cd0f1bdff Mon Sep 17 00:00:00 2001 From: Dominic Gannaway Date: Wed, 15 Jul 2020 12:36:40 +0100 Subject: [PATCH] Revert "Scheduling profiler updates (#19334)" (#19366) This reverts commit 6d7555b014513125b0c229b9c6e45c903d974ff7. --- .../src/ReactFiberClassComponent.new.js | 48 +----- .../src/ReactFiberHooks.new.js | 23 +-- .../src/ReactFiberReconciler.new.js | 6 - .../src/ReactFiberThrow.new.js | 21 +-- .../src/ReactFiberWorkLoop.new.js | 147 ------------------ .../src/ReactFiberWorkLoop.old.js | 2 +- .../SchedulingProfiler-test.internal.js | 30 +--- packages/shared/ReactFeatureFlags.js | 2 +- .../forks/ReactFeatureFlags.www-dynamic.js | 3 +- .../shared/forks/ReactFeatureFlags.www.js | 2 +- 10 files changed, 15 insertions(+), 269 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.new.js b/packages/react-reconciler/src/ReactFiberClassComponent.new.js index 5886d3f4cb..0024e89291 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.new.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.new.js @@ -16,8 +16,6 @@ import {Update, Snapshot} from './ReactSideEffectTags'; import { debugRenderPhaseSideEffectsForStrictMode, disableLegacyContext, - enableDebugTracing, - enableSchedulingProfiler, warnAboutDeprecatedLifecycles, } from 'shared/ReactFeatureFlags'; import ReactStrictModeWarnings from './ReactStrictModeWarnings.new'; @@ -29,7 +27,7 @@ import invariant from 'shared/invariant'; import {REACT_CONTEXT_TYPE, REACT_PROVIDER_TYPE} from 'shared/ReactSymbols'; import {resolveDefaultProps} from './ReactFiberLazyComponent.new'; -import {DebugTracingMode, StrictMode} from './ReactTypeOfMode'; +import {StrictMode} from './ReactTypeOfMode'; import { enqueueUpdate, @@ -57,13 +55,8 @@ import { scheduleUpdateOnFiber, } from './ReactFiberWorkLoop.new'; import {requestCurrentSuspenseConfig} from './ReactFiberSuspenseConfig'; -import {logForceUpdateScheduled, logStateUpdateScheduled} from './DebugTracing'; import {disableLogs, reenableLogs} from 'shared/ConsolePatchingDev'; -import { - markForceUpdateScheduled, - markStateUpdateScheduled, -} from './SchedulingProfiler'; const fakeInternalInstance = {}; const isArray = Array.isArray; @@ -210,19 +203,6 @@ const classComponentUpdater = { enqueueUpdate(fiber, update); scheduleUpdateOnFiber(fiber, lane, eventTime); - - if (__DEV__) { - if (enableDebugTracing) { - if (fiber.mode & DebugTracingMode) { - const name = getComponentName(fiber.type) || 'Unknown'; - logStateUpdateScheduled(name, lane, payload); - } - } - } - - if (enableSchedulingProfiler) { - markStateUpdateScheduled(fiber, lane); - } }, enqueueReplaceState(inst, payload, callback) { const fiber = getInstance(inst); @@ -243,19 +223,6 @@ const classComponentUpdater = { enqueueUpdate(fiber, update); scheduleUpdateOnFiber(fiber, lane, eventTime); - - if (__DEV__) { - if (enableDebugTracing) { - if (fiber.mode & DebugTracingMode) { - const name = getComponentName(fiber.type) || 'Unknown'; - logStateUpdateScheduled(name, lane, payload); - } - } - } - - if (enableSchedulingProfiler) { - markStateUpdateScheduled(fiber, lane); - } }, enqueueForceUpdate(inst, callback) { const fiber = getInstance(inst); @@ -275,19 +242,6 @@ const classComponentUpdater = { enqueueUpdate(fiber, update); scheduleUpdateOnFiber(fiber, lane, eventTime); - - if (__DEV__) { - if (enableDebugTracing) { - if (fiber.mode & DebugTracingMode) { - const name = getComponentName(fiber.type) || 'Unknown'; - logForceUpdateScheduled(name, lane); - } - } - } - - if (enableSchedulingProfiler) { - markForceUpdateScheduled(fiber, lane); - } }, }; diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index c5f766759e..90afafc7a7 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -24,13 +24,9 @@ import type {FiberRoot} from './ReactInternalTypes'; import type {OpaqueIDType} from './ReactFiberHostConfig'; import ReactSharedInternals from 'shared/ReactSharedInternals'; -import { - enableDebugTracing, - enableSchedulingProfiler, - enableNewReconciler, -} from 'shared/ReactFeatureFlags'; +import {enableNewReconciler} from 'shared/ReactFeatureFlags'; -import {NoMode, BlockingMode, DebugTracingMode} from './ReactTypeOfMode'; +import {NoMode, BlockingMode} from './ReactTypeOfMode'; import { NoLane, NoLanes, @@ -92,8 +88,6 @@ import { warnAboutMultipleRenderersDEV, } from './ReactMutableSource.new'; import {getIsRendering} from './ReactCurrentFiber'; -import {logStateUpdateScheduled} from './DebugTracing'; -import {markStateUpdateScheduled} from './SchedulingProfiler'; const {ReactCurrentDispatcher, ReactCurrentBatchConfig} = ReactSharedInternals; @@ -1757,19 +1751,6 @@ function dispatchAction( } scheduleUpdateOnFiber(fiber, lane, eventTime); } - - if (__DEV__) { - if (enableDebugTracing) { - if (fiber.mode & DebugTracingMode) { - const name = getComponentName(fiber.type) || 'Unknown'; - logStateUpdateScheduled(name, lane, action); - } - } - } - - if (enableSchedulingProfiler) { - markStateUpdateScheduled(fiber, lane); - } } export const ContextOnlyDispatcher: Dispatcher = { diff --git a/packages/react-reconciler/src/ReactFiberReconciler.new.js b/packages/react-reconciler/src/ReactFiberReconciler.new.js index 6383603026..6b1b1c8676 100644 --- a/packages/react-reconciler/src/ReactFiberReconciler.new.js +++ b/packages/react-reconciler/src/ReactFiberReconciler.new.js @@ -39,7 +39,6 @@ import { } from './ReactWorkTags'; import getComponentName from 'shared/getComponentName'; import invariant from 'shared/invariant'; -import {enableSchedulingProfiler} from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import {getPublicInstance} from './ReactFiberHostConfig'; import { @@ -96,7 +95,6 @@ import { setRefreshHandler, findHostInstancesForRefresh, } from './ReactFiberHotReloading.new'; -import {markRenderScheduled} from './SchedulingProfiler'; export {registerMutableSourceForHydration} from './ReactMutableSource.new'; export {createPortal} from './ReactPortal'; @@ -275,10 +273,6 @@ export function updateContainer( const suspenseConfig = requestCurrentSuspenseConfig(); const lane = requestUpdateLane(current, suspenseConfig); - if (enableSchedulingProfiler) { - markRenderScheduled(lane); - } - const context = getContextForSubtree(parentComponent); if (container.context === null) { container.context = context; diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js index d8cacf6569..44a0165e2a 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.new.js +++ b/packages/react-reconciler/src/ReactFiberThrow.new.js @@ -31,11 +31,7 @@ import { ForceUpdateForLegacySuspense, } from './ReactSideEffectTags'; import {shouldCaptureSuspense} from './ReactFiberSuspenseComponent.new'; -import {NoMode, BlockingMode, DebugTracingMode} from './ReactTypeOfMode'; -import { - enableDebugTracing, - enableSchedulingProfiler, -} from 'shared/ReactFeatureFlags'; +import {NoMode, BlockingMode} from './ReactTypeOfMode'; import {createCapturedValue} from './ReactCapturedValue'; import { enqueueCapturedUpdate, @@ -58,8 +54,6 @@ import { pingSuspendedRoot, } from './ReactFiberWorkLoop.new'; import {logCapturedError} from './ReactFiberErrorLogger'; -import {logComponentSuspended} from './DebugTracing'; -import {markComponentSuspended} from './SchedulingProfiler'; import { SyncLane, @@ -196,19 +190,6 @@ function throwException( // This is a wakeable. const wakeable: Wakeable = (value: any); - if (__DEV__) { - if (enableDebugTracing) { - if (sourceFiber.mode & DebugTracingMode) { - const name = getComponentName(sourceFiber.type) || 'Unknown'; - logComponentSuspended(name, wakeable); - } - } - } - - if (enableSchedulingProfiler) { - markComponentSuspended(sourceFiber, wakeable); - } - if ((sourceFiber.mode & BlockingMode) === NoMode) { // Reset the memoizedState to what it was before we attempted // to render it. diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index e3218ebe91..c6b02fdf00 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -27,8 +27,6 @@ import { warnAboutUnmockedScheduler, deferRenderPhaseUpdateToNextBatch, decoupleUpdatePriorityFromScheduler, - enableDebugTracing, - enableSchedulingProfiler, enableScopeAPI, } from 'shared/ReactFeatureFlags'; import ReactSharedInternals from 'shared/ReactSharedInternals'; @@ -49,27 +47,6 @@ import { flushSyncCallbackQueue, scheduleSyncCallback, } from './SchedulerWithReactIntegration.new'; -import { - logCommitStarted, - logCommitStopped, - logLayoutEffectsStarted, - logLayoutEffectsStopped, - logPassiveEffectsStarted, - logPassiveEffectsStopped, - logRenderStarted, - logRenderStopped, -} from './DebugTracing'; -import { - markCommitStarted, - markCommitStopped, - markLayoutEffectsStarted, - markLayoutEffectsStopped, - markPassiveEffectsStarted, - markPassiveEffectsStopped, - markRenderStarted, - markRenderYielded, - markRenderStopped, -} from './SchedulingProfiler'; // The scheduler is imported here *only* to detect whether it's been mocked import * as Scheduler from 'scheduler'; @@ -1532,16 +1509,6 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) { const prevInteractions = pushInteractions(root); - if (__DEV__) { - if (enableDebugTracing) { - logRenderStarted(lanes); - } - } - - if (enableSchedulingProfiler) { - markRenderStarted(lanes); - } - do { try { workLoopSync(); @@ -1567,16 +1534,6 @@ function renderRootSync(root: FiberRoot, lanes: Lanes) { ); } - if (__DEV__) { - if (enableDebugTracing) { - logRenderStopped(); - } - } - - if (enableSchedulingProfiler) { - markRenderStopped(); - } - // Set this to null to indicate there's no in-progress render. workInProgressRoot = null; workInProgressRootRenderLanes = NoLanes; @@ -1607,16 +1564,6 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { const prevInteractions = pushInteractions(root); - if (__DEV__) { - if (enableDebugTracing) { - logRenderStarted(lanes); - } - } - - if (enableSchedulingProfiler) { - markRenderStarted(lanes); - } - do { try { workLoopConcurrent(); @@ -1633,25 +1580,12 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { popDispatcher(prevDispatcher); executionContext = prevExecutionContext; - if (__DEV__) { - if (enableDebugTracing) { - logRenderStopped(); - } - } - // Check if the tree has completed. if (workInProgress !== null) { // Still work remaining. - if (enableSchedulingProfiler) { - markRenderYielded(); - } return RootIncomplete; } else { // Completed the tree. - if (enableSchedulingProfiler) { - markRenderStopped(); - } - // Set this to null to indicate there's no in-progress render. workInProgressRoot = null; workInProgressRootRenderLanes = NoLanes; @@ -1934,28 +1868,7 @@ function commitRootImpl(root, renderPriorityLevel) { const finishedWork = root.finishedWork; const lanes = root.finishedLanes; - - if (__DEV__) { - if (enableDebugTracing) { - logCommitStarted(lanes); - } - } - - if (enableSchedulingProfiler) { - markCommitStarted(lanes); - } - if (finishedWork === null) { - if (__DEV__) { - if (enableDebugTracing) { - logCommitStopped(); - } - } - - if (enableSchedulingProfiler) { - markCommitStopped(); - } - return null; } root.finishedWork = null; @@ -2246,16 +2159,6 @@ function commitRootImpl(root, renderPriorityLevel) { } if ((executionContext & LegacyUnbatchedContext) !== NoContext) { - if (__DEV__) { - if (enableDebugTracing) { - logCommitStopped(); - } - } - - if (enableSchedulingProfiler) { - markCommitStopped(); - } - // This is a legacy edge case. We just committed the initial mount of // a ReactDOM.render-ed root inside of batchedUpdates. The commit fired // synchronously, but layout updates should be deferred until the end @@ -2266,16 +2169,6 @@ function commitRootImpl(root, renderPriorityLevel) { // If layout work was scheduled, flush it now. flushSyncCallbackQueue(); - if (__DEV__) { - if (enableDebugTracing) { - logCommitStopped(); - } - } - - if (enableSchedulingProfiler) { - markCommitStopped(); - } - return null; } @@ -2407,16 +2300,6 @@ function commitMutationEffects(root: FiberRoot, renderPriorityLevel) { } function commitLayoutEffects(root: FiberRoot, committedLanes: Lanes) { - if (__DEV__) { - if (enableDebugTracing) { - logLayoutEffectsStarted(committedLanes); - } - } - - if (enableSchedulingProfiler) { - markLayoutEffectsStarted(committedLanes); - } - // TODO: Should probably move the bulk of this function to commitWork. while (nextEffect !== null) { setCurrentDebugFiberInDEV(nextEffect); @@ -2443,16 +2326,6 @@ function commitLayoutEffects(root: FiberRoot, committedLanes: Lanes) { resetCurrentDebugFiberInDEV(); nextEffect = nextEffect.nextEffect; } - - if (__DEV__) { - if (enableDebugTracing) { - logLayoutEffectsStopped(); - } - } - - if (enableSchedulingProfiler) { - markLayoutEffectsStopped(); - } } export function flushPassiveEffects() { @@ -2542,16 +2415,6 @@ function flushPassiveEffectsImpl() { 'Cannot flush passive effects while already rendering.', ); - if (__DEV__) { - if (enableDebugTracing) { - logPassiveEffectsStarted(lanes); - } - } - - if (enableSchedulingProfiler) { - markPassiveEffectsStarted(lanes); - } - if (__DEV__) { isFlushingPassiveEffects = true; } @@ -2708,16 +2571,6 @@ function flushPassiveEffectsImpl() { isFlushingPassiveEffects = false; } - if (__DEV__) { - if (enableDebugTracing) { - logPassiveEffectsStopped(); - } - } - - if (enableSchedulingProfiler) { - markPassiveEffectsStopped(); - } - executionContext = prevExecutionContext; flushSyncCallbackQueue(); diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 42811a73b1..303d1cc0fb 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -1361,7 +1361,7 @@ function handleError(root, thrownValue): void { // sibling, or the parent if there are no siblings. But since the root // has no siblings nor a parent, we set it to null. Usually this is // handled by `completeUnitOfWork` or `unwindWork`, but since we're - // intentionally not calling those, we need set it here. + // interntionally not calling those, we need set it here. // TODO: Consider calling `unwindWork` to pop the contexts. workInProgress = null; return; diff --git a/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js b/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js index 7b030093af..b6d19a18ba 100644 --- a/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js +++ b/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js @@ -351,14 +351,8 @@ describe('SchedulingProfiler', () => { expect(Scheduler).toFlushUntilNextPaint([]); }).toErrorDev('Cannot update during an existing state transition'); - gate(({old}) => - old - ? expect(marks.map(normalizeCodeLocInfo)).toContain( - '--schedule-state-update-1024-Example-\n in Example (at **)', - ) - : expect(marks.map(normalizeCodeLocInfo)).toContain( - '--schedule-state-update-512-Example-\n in Example (at **)', - ), + expect(marks.map(normalizeCodeLocInfo)).toContain( + '--schedule-state-update-1024-Example-\n in Example (at **)', ); }); @@ -384,14 +378,8 @@ describe('SchedulingProfiler', () => { expect(Scheduler).toFlushUntilNextPaint([]); }).toErrorDev('Cannot update during an existing state transition'); - gate(({old}) => - old - ? expect(marks.map(normalizeCodeLocInfo)).toContain( - '--schedule-forced-update-1024-Example-\n in Example (at **)', - ) - : expect(marks.map(normalizeCodeLocInfo)).toContain( - '--schedule-forced-update-512-Example-\n in Example (at **)', - ), + expect(marks.map(normalizeCodeLocInfo)).toContain( + '--schedule-forced-update-1024-Example-\n in Example (at **)', ); }); @@ -473,14 +461,8 @@ describe('SchedulingProfiler', () => { ReactTestRenderer.create(, {unstable_isConcurrent: true}); }); - gate(({old}) => - old - ? expect(marks.map(normalizeCodeLocInfo)).toContain( - '--schedule-state-update-1024-Example-\n in Example (at **)', - ) - : expect(marks.map(normalizeCodeLocInfo)).toContain( - '--schedule-state-update-512-Example-\n in Example (at **)', - ), + expect(marks.map(normalizeCodeLocInfo)).toContain( + '--schedule-state-update-1024-Example-\n in Example (at **)', ); }); }); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 8d65df1a8c..4c76b7f09d 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -17,7 +17,7 @@ export const enableDebugTracing = false; // Adds user timing marks for e.g. state updates, suspense, and work loop stuff, // for an experimental scheduling profiler tool. -export const enableSchedulingProfiler = __PROFILE__ && __EXPERIMENTAL__; +export const enableSchedulingProfiler = false; // Helps identify side effects in render-phase lifecycle hooks and setState // reducers by double invoking them in Strict Mode. diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index e3721bdb4c..d244b0fda6 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -19,8 +19,9 @@ export const enableFilterEmptyStringAttributesDOM = __VARIANT__; export const enableLegacyFBSupport = __VARIANT__; export const decoupleUpdatePriorityFromScheduler = __VARIANT__; -// TODO: This feature does not currently exist in the new reconciler fork. +// TODO: These features do not currently exist in the new reconciler fork. export const enableDebugTracing = !__VARIANT__; +export const enableSchedulingProfiler = !__VARIANT__ && __PROFILE__; // This only has an effect in the new reconciler. But also, the new reconciler // is only enabled when __VARIANT__ is true. So this is set to the opposite of diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index a70720357c..4b6e29fb21 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -26,6 +26,7 @@ export const { deferRenderPhaseUpdateToNextBatch, decoupleUpdatePriorityFromScheduler, enableDebugTracing, + enableSchedulingProfiler, enableFormEventDelegation, } = dynamicFeatureFlags; @@ -34,7 +35,6 @@ export const { export const enableProfilerTimer = __PROFILE__; export const enableProfilerCommitHooks = __PROFILE__; -export const enableSchedulingProfiler = __PROFILE__; // Note: we'll want to remove this when we to userland implementation. // For now, we'll turn it on for everyone because it's *already* on for everyone in practice.