{"parents":["bc241bfcfef0fb54f6b8436bcf12c402102f565e"],"commit":{"shortRevision":"7b241d1d7f","authoredAt":"2016-04-29T23:09:54+01:00","revision":"7b241d1d7fa285218fcdb3522b24de1bce9c8b25","authorName":"Dan Abramov","authorEmail":"dan.abramov@gmail.com","subject":"Add tests and fix wasted render calculation"},"body":"","patch":"diff --git a\/src\/isomorphic\/ReactPerf.js b\/src\/isomorphic\/ReactPerf.js\nindex a9cf2b5070..14ea56be51 100644\n--- a\/src\/isomorphic\/ReactPerf.js\n+++ b\/src\/isomorphic\/ReactPerf.js\n@@ -27,12 +27,10 @@ function getExclusive(flushHistory = getFlushHistory()) {\n var aggregatedStats = {};\n var affectedIDs = {};\n \n- function updateAggregatedStats(treeSnapshot, instanceID, applyUpdate) {\n+ function updateAggregatedStats(treeSnapshot, instanceID, timerType, applyUpdate) {\n var {displayName} = treeSnapshot[instanceID];\n-\n var key = displayName;\n var stats = aggregatedStats[key];\n-\n if (!stats) {\n affectedIDs[key] = {};\n stats = aggregatedStats[key] = {\n@@ -43,7 +41,12 @@ function getExclusive(flushHistory = getFlushHistory()) {\n totalDuration: 0,\n };\n }\n-\n+ if (!stats.durations[timerType]) {\n+ stats.durations[timerType] = 0;\n+ }\n+ if (!stats.counts[timerType]) {\n+ stats.counts[timerType] = 0;\n+ }\n affectedIDs[key][instanceID] = true;\n applyUpdate(stats);\n }\n@@ -52,17 +55,9 @@ function getExclusive(flushHistory = getFlushHistory()) {\n var {measurements, treeSnapshot} = flush;\n measurements.forEach(measurement => {\n var {duration, instanceID, timerType} = measurement;\n- updateAggregatedStats(treeSnapshot, instanceID, stats => {\n+ updateAggregatedStats(treeSnapshot, instanceID, timerType, stats => {\n stats.totalDuration += duration;\n-\n- if (!stats.durations[timerType]) {\n- stats.durations[timerType] = 0;\n- }\n stats.durations[timerType] += duration;\n-\n- if (!stats.counts[timerType]) {\n- stats.counts[timerType] = 0;\n- }\n stats.counts[timerType]++;\n });\n });\n@@ -73,20 +68,20 @@ function getExclusive(flushHistory = getFlushHistory()) {\n ...aggregatedStats[key],\n instanceCount: Object.keys(affectedIDs[key]).length,\n }))\n- .sort((a, b) => b.totalDuration - a.totalDuration);\n+ .sort((a, b) =>\n+ b.totalDuration - a.totalDuration\n+ );\n }\n \n-function getInclusive(flushHistory = getFlushHistory(), wastedOnly) {\n+function getInclusive(flushHistory = getFlushHistory()) {\n var aggregatedStats = {};\n var affectedIDs = {};\n \n function updateAggregatedStats(treeSnapshot, instanceID, applyUpdate) {\n var {displayName, ownerID} = treeSnapshot[instanceID];\n-\n var owner = treeSnapshot[ownerID];\n var key = `${owner ? owner.displayName + ' >' : ''} ${displayName}`;\n var stats = aggregatedStats[key];\n-\n if (!stats) {\n affectedIDs[key] = {};\n stats = aggregatedStats[key] = {\n@@ -96,12 +91,11 @@ function getInclusive(flushHistory = getFlushHistory(), wastedOnly) {\n renderCount: 0,\n };\n }\n-\n affectedIDs[key][instanceID] = true;\n applyUpdate(stats);\n }\n \n- var hasRenderedByID = {};\n+ var isCompositeByID = {};\n flushHistory.forEach(flush => {\n var {measurements} = flush;\n measurements.forEach(measurement => {\n@@ -109,7 +103,7 @@ function getInclusive(flushHistory = getFlushHistory(), wastedOnly) {\n if (timerType !== 'render') {\n return;\n }\n- hasRenderedByID[instanceID] = true;\n+ isCompositeByID[instanceID] = true;\n });\n });\n \n@@ -125,7 +119,9 @@ function getInclusive(flushHistory = getFlushHistory(), wastedOnly) {\n });\n var nextParentID = instanceID;\n while (nextParentID) {\n- if (hasRenderedByID[nextParentID]) {\n+ \/\/ As we traverse parents, only count inclusive time towards composites.\n+ \/\/ We know something is a composite if its render() was called.\n+ if (isCompositeByID[nextParentID]) {\n updateAggregatedStats(treeSnapshot, nextParentID, stats => {\n stats.inclusiveRenderDuration += duration;\n });\n@@ -140,7 +136,9 @@ function getInclusive(flushHistory = getFlushHistory(), wastedOnly) {\n ...aggregatedStats[key],\n instanceCount: Object.keys(affectedIDs[key]).length,\n }))\n- .sort((a, b) => b.inclusiveRenderDuration - a.inclusiveRenderDuration);\n+ .sort((a, b) =>\n+ b.inclusiveRenderDuration - a.inclusiveRenderDuration\n+ );\n }\n \n function getWasted(flushHistory = getFlushHistory()) {\n@@ -149,11 +147,9 @@ function getWasted(flushHistory = getFlushHistory()) {\n \n function updateAggregatedStats(treeSnapshot, instanceID, applyUpdate) {\n var {displayName, ownerID} = treeSnapshot[instanceID];\n-\n var owner = treeSnapshot[ownerID];\n var key = (owner ? owner.displayName + ' > ' : '') + displayName;\n var stats = aggregatedStats[key];\n-\n if (!stats) {\n affectedIDs[key] = {};\n stats = aggregatedStats[key] = {\n@@ -163,25 +159,27 @@ function getWasted(flushHistory = getFlushHistory()) {\n renderCount: 0,\n };\n }\n-\n affectedIDs[key][instanceID] = true;\n applyUpdate(stats);\n }\n \n flushHistory.forEach(flush => {\n var {measurements, treeSnapshot, operations} = flush;\n- var dirtyInstanceIDs = {};\n+ var isDefinitelyNotWastedByID = {};\n \n+ \/\/ Find native components associated with an operation in this batch.\n+ \/\/ Mark all components in their parent tree as definitely not wasted.\n operations.forEach(operation => {\n var {instanceID} = operation;\n-\n var nextParentID = instanceID;\n while (nextParentID) {\n- dirtyInstanceIDs[nextParentID] = true;\n+ isDefinitelyNotWastedByID[nextParentID] = true;\n nextParentID = treeSnapshot[nextParentID].parentID;\n }\n });\n \n+ \/\/ Find composite components that rendered in this batch.\n+ \/\/ These are potential candidates for being wasted renders.\n var renderedCompositeIDs = {};\n measurements.forEach(measurement => {\n var {instanceID, timerType} = measurement;\n@@ -196,21 +194,31 @@ function getWasted(flushHistory = getFlushHistory()) {\n if (timerType !== 'render') {\n return;\n }\n+\n+ \/\/ If there was a DOM update below this component, or it has just been\n+ \/\/ mounted, its render() is not considered wasted.\n var { updateCount } = treeSnapshot[instanceID];\n- if (dirtyInstanceIDs[instanceID] || updateCount === 0) {\n+ if (isDefinitelyNotWastedByID[instanceID] || updateCount === 0) {\n return;\n }\n+\n+ \/\/ We consider this render() wasted.\n updateAggregatedStats(treeSnapshot, instanceID, stats => {\n stats.renderCount++;\n });\n+\n var nextParentID = instanceID;\n while (nextParentID) {\n- if (!renderedCompositeIDs[nextParentID]) {\n- break;\n+ \/\/ Any parents rendered during this batch are considered wasted\n+ \/\/ unless we previously marked them as dirty.\n+ var isWasted =\n+ renderedCompositeIDs[nextParentID] &&\n+ !isDefinitelyNotWastedByID[nextParentID];\n+ if (isWasted) {\n+ updateAggregatedStats(treeSnapshot, nextParentID, stats => {\n+ stats.inclusiveRenderDuration += duration;\n+ });\n }\n- updateAggregatedStats(treeSnapshot, nextParentID, stats => {\n- stats.inclusiveRenderDuration += duration;\n- });\n nextParentID = treeSnapshot[nextParentID].parentID;\n }\n });\n@@ -221,7 +229,9 @@ function getWasted(flushHistory = getFlushHistory()) {\n ...aggregatedStats[key],\n instanceCount: Object.keys(affectedIDs[key]).length,\n }))\n- .sort((a, b) => b.inclusiveRenderDuration - a.inclusiveRenderDuration);\n+ .sort((a, b) =>\n+ b.inclusiveRenderDuration - a.inclusiveRenderDuration\n+ );\n }\n \n function getOperations(flushHistory = getFlushHistory()) {\ndiff --git a\/src\/isomorphic\/__tests__\/ReactPerf-test.js b\/src\/isomorphic\/__tests__\/ReactPerf-test.js\nnew file mode 100644\nindex 0000000000..98df00938b\n--- \/dev\/null\n+++ b\/src\/isomorphic\/__tests__\/ReactPerf-test.js\n@@ -0,0 +1,238 @@\n+\/**\n+ * Copyright 2013-present, Facebook, Inc.\n+ * All rights reserved.\n+ *\n+ * This source code is licensed under the BSD-style license found in the\n+ * LICENSE file in the root directory of this source tree. An additional grant\n+ * of patent rights can be found in the PATENTS file in the same directory.\n+ *\n+ * @emails react-core\n+ *\/\n+\n+'use strict';\n+\n+describe('ReactPerf', function() {\n+ var React;\n+ var ReactDOM;\n+ var ReactPerf;\n+ var ReactTestUtils;\n+\n+ var App;\n+ var Box;\n+ var Div;\n+\n+ beforeEach(function() {\n+ var now = 0;\n+ jest.setMock('fbjs\/lib\/performanceNow', function() {\n+ return now++;\n+ });\n+\n+ if (typeof console.table !== 'function') {\n+ console.table = () => {};\n+ console.table.isFake = true;\n+ }\n+\n+ React = require('React');\n+ ReactDOM = require('ReactDOM');\n+ ReactPerf = require('ReactPerf');\n+ ReactTestUtils = require('ReactTestUtils');\n+\n+ App = React.createClass({\n+ render: function() {\n+ return

<\/div>;\n+ },\n+ });\n+\n+ Box = React.createClass({\n+ render: function() {\n+ return

<\/div>;\n+ },\n+ });\n+\n+ \/\/ ReactPerf only measures composites, so we put everything in one.\n+ Div = React.createClass({\n+ render: function() {\n+ return

;\n+ },\n+ });\n+ });\n+\n+ afterEach(function() {\n+ if (console.table.isFake) {\n+ delete console.table;\n+ }\n+ });\n+\n+ function measure(fn) {\n+ ReactPerf.start();\n+ fn();\n+ ReactPerf.stop();\n+ return ReactPerf.getLastMeasurements();\n+ }\n+\n+ it('should count no-op update as waste', function() {\n+ var container = document.createElement('div');\n+ ReactDOM.render(, container);\n+ var measurements = measure(() => {\n+ ReactDOM.render(, container);\n+ });\n+\n+ var summary = ReactPerf.getWasted(measurements);\n+ expect(summary).toEqual([{\n+ key: 'App',\n+ instanceCount: 1,\n+ inclusiveRenderDuration: 3,\n+ renderCount: 1,\n+ }, {\n+ key: 'App > Box',\n+ instanceCount: 2,\n+ inclusiveRenderDuration: 2,\n+ renderCount: 2,\n+ }]);\n+ });\n+\n+ it('should count no-op update in child as waste', function() {\n+ var container = document.createElement('div');\n+ ReactDOM.render(, container);\n+\n+ \/\/ Here, we add a Box -- two of the updates are wasted time (but the\n+ \/\/ addition of the third is not)\n+ var measurements = measure(() => {\n+ ReactDOM.render(, container);\n+ });\n+\n+ var summary = ReactPerf.getWasted(measurements);\n+ expect(summary).toEqual([{\n+ key: 'App > Box',\n+ instanceCount: 1,\n+ inclusiveRenderDuration: 1,\n+ renderCount: 1,\n+ }]);\n+ });\n+\n+ function expectNoWaste(fn) {\n+ var measurements = measure(fn);\n+ var summary = ReactPerf.getWasted(measurements);\n+ expect(summary).toEqual([]);\n+ }\n+\n+ it('should not count initial render as waste', function() {\n+ expectNoWaste(() => {\n+ ReactTestUtils.renderIntoDocument();\n+ });\n+ });\n+\n+ it('should not count unmount as waste', function() {\n+ var container = document.createElement('div');\n+ ReactDOM.render(

hello<\/Div>, container);\n+ expectNoWaste(() => {\n+ ReactDOM.unmountComponentAtNode(container);\n+ });\n+ });\n+\n+ it('should not count content update as waste', function() {\n+ var container = document.createElement('div');\n+ ReactDOM.render(

hello<\/Div>, container);\n+ expectNoWaste(() => {\n+ ReactDOM.render(

hello world<\/Div>, container);\n+ });\n+ });\n+\n+ it('should not count child addition as waste', function() {\n+ var container = document.createElement('div');\n+ ReactDOM.render(

<\/Div>, container);\n+ expectNoWaste(() => {\n+ ReactDOM.render(

<\/Div>, container);\n+ });\n+ });\n+\n+ it('should not count child removal as waste', function() {\n+ var container = document.createElement('div');\n+ ReactDOM.render(

<\/Div>, container);\n+ expectNoWaste(() => {\n+ ReactDOM.render(

<\/Div>, container);\n+ });\n+ });\n+\n+ it('should not count property update as waste', function() {\n+ var container = document.createElement('div');\n+ ReactDOM.render(

hey<\/Div>, container);\n+ expectNoWaste(() => {\n+ ReactDOM.render(

hey<\/Div>, container);\n+ });\n+ });\n+\n+ it('should not count style update as waste', function() {\n+ var container = document.createElement('div');\n+ ReactDOM.render(

hey<\/Div>, container);\n+ expectNoWaste(() => {\n+ ReactDOM.render(

hey<\/Div>, container);\n+ });\n+ });\n+\n+ it('should not count property removal as waste', function() {\n+ var container = document.createElement('div');\n+ ReactDOM.render(

hey<\/Div>, container);\n+ expectNoWaste(() => {\n+ ReactDOM.render(

hey<\/Div>, container);\n+ });\n+ });\n+\n+ it('should not count raw HTML update as waste', function() {\n+ var container = document.createElement('div');\n+ ReactDOM.render(\n+

,\n+ container\n+ );\n+ expectNoWaste(() => {\n+ ReactDOM.render(\n+

,\n+ container\n+ );\n+ });\n+ });\n+\n+ it('should not count child reordering as waste', function() {\n+ var container = document.createElement('div');\n+ ReactDOM.render(

<\/Div>, container);\n+ expectNoWaste(() => {\n+ ReactDOM.render(

<\/Div>, container);\n+ });\n+ });\n+\n+ it('should not count text update as waste', function() {\n+ var container = document.createElement('div');\n+ ReactDOM.render(

{'hello'}{'world'}<\/Div>, container);\n+ expectNoWaste(() => {\n+ ReactDOM.render(

{'hello'}{'friend'}<\/Div>, container);\n+ });\n+ });\n+\n+ it('warns once when using getMeasurementsSummaryMap', function() {\n+ var measurements = measure(() => {});\n+ spyOn(console, 'error');\n+ ReactPerf.getMeasurementsSummaryMap(measurements);\n+ expect(console.error.calls.length).toBe(1);\n+ expect(console.error.argsForCall[0][0]).toContain(\n+ '`ReactPerf.getMeasurementsSummaryMap(...)` is deprecated. Use ' +\n+ '`ReactPerf.getWasted(...)` instead.'\n+ );\n+\n+ ReactPerf.getMeasurementsSummaryMap(measurements);\n+ expect(console.error.calls.length).toBe(1);\n+ });\n+\n+ it('warns once when using printDOM', function() {\n+ var measurements = measure(() => {});\n+ spyOn(console, 'error');\n+ ReactPerf.printDOM(measurements);\n+ expect(console.error.calls.length).toBe(1);\n+ expect(console.error.argsForCall[0][0]).toContain(\n+ '`ReactPerf.printDOM(...)` is deprecated. Use ' +\n+ '`ReactPerf.printOperations(...)` instead.'\n+ );\n+\n+ ReactPerf.printDOM(measurements);\n+ expect(console.error.calls.length).toBe(1);\n+ });\n+});\n","files":[{"path":"src\/isomorphic\/ReactPerf.js","status":"M"},{"path":"src\/isomorphic\/__tests__\/ReactPerf-test.js","status":"A"}]}

dillon1000/react

Files changed