From cb8d56281fd72f258cf8c2e2bc1ea0b835e0c6fd Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Tue, 4 Aug 2020 14:04:02 +0800 Subject: [PATCH 1/2] Add ReactVersion to SchedulingProfiler render scheduled marks --- .../src/SchedulingProfiler.js | 5 +++- .../SchedulingProfiler-test.internal.js | 30 ++++++++++--------- 2 files changed, 20 insertions(+), 15 deletions(-) diff --git a/packages/react-reconciler/src/SchedulingProfiler.js b/packages/react-reconciler/src/SchedulingProfiler.js index c6032014a198f..bb78ca655461f 100644 --- a/packages/react-reconciler/src/SchedulingProfiler.js +++ b/packages/react-reconciler/src/SchedulingProfiler.js @@ -15,6 +15,7 @@ import { enableSchedulingProfiler, enableSchedulingProfilerComponentStacks, } from 'shared/ReactFeatureFlags'; +import ReactVersion from 'shared/ReactVersion'; import getComponentName from 'shared/getComponentName'; import {getStackByFiberInDevAndProd} from './ReactFiberComponentStack'; @@ -166,7 +167,9 @@ export function markRenderStopped(): void { export function markRenderScheduled(lane: Lane): void { if (enableSchedulingProfiler) { if (supportsUserTiming) { - performance.mark(`--schedule-render-${formatLanes(lane)}`); + performance.mark( + `--schedule-render-${formatLanes(lane)}-${ReactVersion}`, + ); } } } diff --git a/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js b/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js index 3afc4bfb5443c..1a4ad9cf08b68 100644 --- a/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js +++ b/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js @@ -10,6 +10,8 @@ 'use strict'; +import ReactVersion from 'shared/ReactVersion'; + function normalizeCodeLocInfo(str) { return ( str && @@ -81,7 +83,7 @@ describe('SchedulingProfiler', () => { ReactTestRenderer.create(
); expect(marks).toEqual([ - '--schedule-render-1', + `--schedule-render-1-${ReactVersion}`, '--render-start-1', '--render-stop', '--commit-start-1', @@ -95,7 +97,7 @@ describe('SchedulingProfiler', () => { it('should mark concurrent render without suspends or state updates', () => { ReactTestRenderer.create(
, {unstable_isConcurrent: true}); - expect(marks).toEqual(['--schedule-render-512']); + expect(marks).toEqual([`--schedule-render-512-${ReactVersion}`]); marks.splice(0); @@ -128,7 +130,7 @@ describe('SchedulingProfiler', () => { expect(ReactNoop.flushNextYield()).toEqual(['Foo']); expect(marks).toEqual([ - '--schedule-render-512', + `--schedule-render-512-${ReactVersion}`, '--render-start-512', '--render-yield', ]); @@ -148,7 +150,7 @@ describe('SchedulingProfiler', () => { ); expect(marks).toEqual([ - '--schedule-render-1', + `--schedule-render-1-${ReactVersion}`, '--render-start-1', toggleComponentStacks( '--suspense-suspend-0-Example-\n at Example\n at Suspense', @@ -184,7 +186,7 @@ describe('SchedulingProfiler', () => { ); expect(marks).toEqual([ - '--schedule-render-1', + `--schedule-render-1-${ReactVersion}`, '--render-start-1', toggleComponentStacks( '--suspense-suspend-0-Example-\n at Example\n at Suspense', @@ -220,7 +222,7 @@ describe('SchedulingProfiler', () => { {unstable_isConcurrent: true}, ); - expect(marks).toEqual(['--schedule-render-512']); + expect(marks).toEqual([`--schedule-render-512-${ReactVersion}`]); marks.splice(0); @@ -262,7 +264,7 @@ describe('SchedulingProfiler', () => { {unstable_isConcurrent: true}, ); - expect(marks).toEqual(['--schedule-render-512']); + expect(marks).toEqual([`--schedule-render-512-${ReactVersion}`]); marks.splice(0); @@ -304,7 +306,7 @@ describe('SchedulingProfiler', () => { ReactTestRenderer.create(, {unstable_isConcurrent: true}); - expect(marks).toEqual(['--schedule-render-512']); + expect(marks).toEqual([`--schedule-render-512-${ReactVersion}`]); marks.splice(0); @@ -340,7 +342,7 @@ describe('SchedulingProfiler', () => { ReactTestRenderer.create(, {unstable_isConcurrent: true}); - expect(marks).toEqual(['--schedule-render-512']); + expect(marks).toEqual([`--schedule-render-512-${ReactVersion}`]); marks.splice(0); @@ -377,7 +379,7 @@ describe('SchedulingProfiler', () => { ReactTestRenderer.create(, {unstable_isConcurrent: true}); - expect(marks).toEqual(['--schedule-render-512']); + expect(marks).toEqual([`--schedule-render-512-${ReactVersion}`]); marks.splice(0); @@ -414,7 +416,7 @@ describe('SchedulingProfiler', () => { ReactTestRenderer.create(, {unstable_isConcurrent: true}); - expect(marks).toEqual(['--schedule-render-512']); + expect(marks).toEqual([`--schedule-render-512-${ReactVersion}`]); marks.splice(0); @@ -449,7 +451,7 @@ describe('SchedulingProfiler', () => { ReactTestRenderer.create(, {unstable_isConcurrent: true}); - expect(marks).toEqual(['--schedule-render-512']); + expect(marks).toEqual([`--schedule-render-512-${ReactVersion}`]); marks.splice(0); @@ -489,7 +491,7 @@ describe('SchedulingProfiler', () => { gate(({old}) => { if (old) { expect(marks.map(normalizeCodeLocInfo)).toEqual([ - '--schedule-render-512', + `--schedule-render-512-${ReactVersion}`, '--render-start-512', '--render-stop', '--commit-start-512', @@ -508,7 +510,7 @@ describe('SchedulingProfiler', () => { ]); } else { expect(marks.map(normalizeCodeLocInfo)).toEqual([ - '--schedule-render-512', + `--schedule-render-512-${ReactVersion}`, '--render-start-512', '--render-stop', '--commit-start-512', From f0c6d1074c3a864009d704a138b1d27d9cd644cc Mon Sep 17 00:00:00 2001 From: E-Liang Tan Date: Mon, 10 Aug 2020 20:46:51 +0800 Subject: [PATCH 2/2] Move ReactVersion to a new --react-init-* mark --- .../src/SchedulingProfiler.js | 11 +++- .../SchedulingProfiler-test.internal.js | 66 ++++++++++++++----- 2 files changed, 58 insertions(+), 19 deletions(-) diff --git a/packages/react-reconciler/src/SchedulingProfiler.js b/packages/react-reconciler/src/SchedulingProfiler.js index bb78ca655461f..7e06b9821600a 100644 --- a/packages/react-reconciler/src/SchedulingProfiler.js +++ b/packages/react-reconciler/src/SchedulingProfiler.js @@ -30,6 +30,13 @@ function formatLanes(laneOrLanes: Lane | Lanes): string { return ((laneOrLanes: any): number).toString(); } +// Create a mark on React initialization +if (enableSchedulingProfiler) { + if (supportsUserTiming) { + performance.mark(`--react-init-${ReactVersion}`); + } +} + export function markCommitStarted(lanes: Lanes): void { if (enableSchedulingProfiler) { if (supportsUserTiming) { @@ -167,9 +174,7 @@ export function markRenderStopped(): void { export function markRenderScheduled(lane: Lane): void { if (enableSchedulingProfiler) { if (supportsUserTiming) { - performance.mark( - `--schedule-render-${formatLanes(lane)}-${ReactVersion}`, - ); + performance.mark(`--schedule-render-${formatLanes(lane)}`); } } } diff --git a/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js b/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js index 1a4ad9cf08b68..ce6bc2ac0904c 100644 --- a/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js +++ b/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js @@ -56,6 +56,7 @@ describe('SchedulingProfiler', () => { beforeEach(() => { jest.resetModules(); global.performance = createUserTimingPolyfill(); + marks = []; React = require('react'); @@ -64,8 +65,6 @@ describe('SchedulingProfiler', () => { ReactNoop = require('react-noop-renderer'); Scheduler = require('scheduler'); - - marks = []; }); afterEach(() => { @@ -78,12 +77,18 @@ describe('SchedulingProfiler', () => { expect(marks).toEqual([]); }); + // @gate enableSchedulingProfiler + it('should log React version on initialization', () => { + expect(marks).toEqual([`--react-init-${ReactVersion}`]); + }); + // @gate enableSchedulingProfiler it('should mark sync render without suspends or state updates', () => { ReactTestRenderer.create(
); expect(marks).toEqual([ - `--schedule-render-1-${ReactVersion}`, + `--react-init-${ReactVersion}`, + '--schedule-render-1', '--render-start-1', '--render-stop', '--commit-start-1', @@ -97,7 +102,10 @@ describe('SchedulingProfiler', () => { it('should mark concurrent render without suspends or state updates', () => { ReactTestRenderer.create(
, {unstable_isConcurrent: true}); - expect(marks).toEqual([`--schedule-render-512-${ReactVersion}`]); + expect(marks).toEqual([ + `--react-init-${ReactVersion}`, + '--schedule-render-512', + ]); marks.splice(0); @@ -130,7 +138,8 @@ describe('SchedulingProfiler', () => { expect(ReactNoop.flushNextYield()).toEqual(['Foo']); expect(marks).toEqual([ - `--schedule-render-512-${ReactVersion}`, + `--react-init-${ReactVersion}`, + '--schedule-render-512', '--render-start-512', '--render-yield', ]); @@ -150,7 +159,8 @@ describe('SchedulingProfiler', () => { ); expect(marks).toEqual([ - `--schedule-render-1-${ReactVersion}`, + `--react-init-${ReactVersion}`, + '--schedule-render-1', '--render-start-1', toggleComponentStacks( '--suspense-suspend-0-Example-\n at Example\n at Suspense', @@ -186,7 +196,8 @@ describe('SchedulingProfiler', () => { ); expect(marks).toEqual([ - `--schedule-render-1-${ReactVersion}`, + `--react-init-${ReactVersion}`, + '--schedule-render-1', '--render-start-1', toggleComponentStacks( '--suspense-suspend-0-Example-\n at Example\n at Suspense', @@ -222,7 +233,10 @@ describe('SchedulingProfiler', () => { {unstable_isConcurrent: true}, ); - expect(marks).toEqual([`--schedule-render-512-${ReactVersion}`]); + expect(marks).toEqual([ + `--react-init-${ReactVersion}`, + '--schedule-render-512', + ]); marks.splice(0); @@ -264,7 +278,10 @@ describe('SchedulingProfiler', () => { {unstable_isConcurrent: true}, ); - expect(marks).toEqual([`--schedule-render-512-${ReactVersion}`]); + expect(marks).toEqual([ + `--react-init-${ReactVersion}`, + '--schedule-render-512', + ]); marks.splice(0); @@ -306,7 +323,10 @@ describe('SchedulingProfiler', () => { ReactTestRenderer.create(, {unstable_isConcurrent: true}); - expect(marks).toEqual([`--schedule-render-512-${ReactVersion}`]); + expect(marks).toEqual([ + `--react-init-${ReactVersion}`, + '--schedule-render-512', + ]); marks.splice(0); @@ -342,7 +362,10 @@ describe('SchedulingProfiler', () => { ReactTestRenderer.create(, {unstable_isConcurrent: true}); - expect(marks).toEqual([`--schedule-render-512-${ReactVersion}`]); + expect(marks).toEqual([ + `--react-init-${ReactVersion}`, + '--schedule-render-512', + ]); marks.splice(0); @@ -379,7 +402,10 @@ describe('SchedulingProfiler', () => { ReactTestRenderer.create(, {unstable_isConcurrent: true}); - expect(marks).toEqual([`--schedule-render-512-${ReactVersion}`]); + expect(marks).toEqual([ + `--react-init-${ReactVersion}`, + '--schedule-render-512', + ]); marks.splice(0); @@ -416,7 +442,10 @@ describe('SchedulingProfiler', () => { ReactTestRenderer.create(, {unstable_isConcurrent: true}); - expect(marks).toEqual([`--schedule-render-512-${ReactVersion}`]); + expect(marks).toEqual([ + `--react-init-${ReactVersion}`, + '--schedule-render-512', + ]); marks.splice(0); @@ -451,7 +480,10 @@ describe('SchedulingProfiler', () => { ReactTestRenderer.create(, {unstable_isConcurrent: true}); - expect(marks).toEqual([`--schedule-render-512-${ReactVersion}`]); + expect(marks).toEqual([ + `--react-init-${ReactVersion}`, + '--schedule-render-512', + ]); marks.splice(0); @@ -491,7 +523,8 @@ describe('SchedulingProfiler', () => { gate(({old}) => { if (old) { expect(marks.map(normalizeCodeLocInfo)).toEqual([ - `--schedule-render-512-${ReactVersion}`, + `--react-init-${ReactVersion}`, + '--schedule-render-512', '--render-start-512', '--render-stop', '--commit-start-512', @@ -510,7 +543,8 @@ describe('SchedulingProfiler', () => { ]); } else { expect(marks.map(normalizeCodeLocInfo)).toEqual([ - `--schedule-render-512-${ReactVersion}`, + `--react-init-${ReactVersion}`, + '--schedule-render-512', '--render-start-512', '--render-stop', '--commit-start-512',