Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Perf Test: Flamegraphs #9550

Merged
merged 17 commits into from
Jun 24, 2019
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions apps/perf-test/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
logfiles
8 changes: 1 addition & 7 deletions apps/perf-test/config/pre-copy.json
Original file line number Diff line number Diff line change
@@ -1,11 +1,5 @@
{
"copyTo": {
"dist": [
"./index.html",
"./node_modules/react/umd/react.development.js",
"./node_modules/react/umd/react.production.min.js",
"./node_modules/react-dom/umd/react-dom.development.js",
"./node_modules/react-dom/umd/react-dom.production.min.js"
]
"dist": ["./index.html", "./node_modules/react/umd/react.production.min.js", "./node_modules/react-dom/umd/react-dom.production.min.js"]
}
}
2 changes: 2 additions & 0 deletions apps/perf-test/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
</head>

<body>
<script src="./react.production.min.js"></script>
<script src="./react-dom.production.min.js"></script>
<script type="text/javascript" src="perf-test.js"></script>
</body>
</html>
6 changes: 5 additions & 1 deletion apps/perf-test/just.config.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,8 @@
const perfTest = require('./tasks/perf-test');
const { preset, just } = require('@uifabric/build');
const { task } = just;
const { task, series } = just;

preset();

task('run-perf-test', perfTest);
task('perf-test', series('build', 'run-perf-test'));
8 changes: 7 additions & 1 deletion apps/perf-test/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,17 +15,23 @@
"@types/react": "16.8.11",
"@types/react-dom": "16.8.4",
"@types/webpack-env": "1.13.9",
"@types/webpack-node-externals": "1.6.3",
"@uifabric/prettier-rules": "^7.0.2",
"@uifabric/tslint-rules": "^7.0.2",
"@uifabric/build": "^7.0.0"
"@uifabric/build": "^7.0.0",
"puppeteer": "^1.13.0",
"flamebearer": "^1.1.3",
"concat-stream": "^2.0.0"
},
"dependencies": {
"@uifabric/set-version": "^7.0.0",
"@uifabric/example-app-base": "^7.1.1",
"@uifabric/experiments": "^7.2.1",
"@microsoft/load-themed-styles": "^1.7.13",
"es6-promise": "^4.1.0",
"immutability-helper": "~2.8.1",
"office-ui-fabric-react": "^7.5.0",
"querystring": "^0.2.0",
"react": "16.8.6",
"react-dom": "16.8.6",
"tslib": "^1.7.1"
Expand Down
32 changes: 32 additions & 0 deletions apps/perf-test/perfLearnings.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@

JasonGore marked this conversation as resolved.
Show resolved Hide resolved
1. Existing Perf Issues
* See attached HTMLs for analysis
* Baseline differences between reference (website) and local
* Website was using React debug vs. local using React production, causing a 2x difference in results.
* High level of variance in results
* Tried tightening variance by using V8 flags to control optimization and sandboxing.
* Variance was tightened but there are still routinely wild variances that make reported results unreliable.

* Learnings
* Perf test package was highly unreliable and should not have been used or reported on PRs.
* Performance test packages should always be characterized for:
* Consistency between reference and target
* Variance known and accounted for (or clearly presented)


2. Conversion to V8 profiling

* Flame Graph
* initial samples showed obvious areas of low hanging fruit that could be improved
* string replace with regex in 2 spots was consuming 30% of the time in mergeStyles!

3. Results
* use mergeStyles PRs as references and before/after comparison
* https://github.com/OfficeDev/office-ui-fabric-react/pull/9419
* https://github.com/OfficeDev/office-ui-fabric-react/pull/9460
* https://github.com/OfficeDev/office-ui-fabric-react/pull/9437

4. Links
* https://benchling.engineering/a-deep-dive-into-react-perf-debugging-fd2063f5a667
* https://benchling.engineering/performance-engineering-with-react-e03013e53285
* https://engineering.musefind.com/how-to-benchmark-react-components-the-quick-and-dirty-guide-f595baf1014c
70 changes: 62 additions & 8 deletions apps/perf-test/src/App.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as React from 'react';
import { Dropdown, PrimaryButton, Stack, TextField, Text } from 'office-ui-fabric-react';
import { Measurer, MeasurerTimings } from './Measurer';
import { ITimings, Measurer } from './Measurer';
import { useTimer } from './useTimer';
import { Scenarios } from './Scenarios';

Expand All @@ -12,9 +12,38 @@ export const App = () => {
const [scenario, setScenario] = React.useState(Scenarios[0]);
const [count, setCount] = React.useState(100);
const [iterations, setIterations] = React.useState(1);
const [iterationsCount, setIterationsCount] = React.useState(0);
const [showIteration, setShowIteration] = React.useState(false);
const [results, setResults] = React.useState<ITimings[]>([]);

// TODO: undo changes to this file
// TODO: move these app files into a directory that makes it obvious it's different from perf-test scripting

React.useEffect(() => {
setTimingsVisible(itemsVisible);
if (itemsVisible) {
if (iterationsCount < iterations) {
if (showIteration) {
setShowIteration(false);
setResults(results.concat(Measurer.getTimings()));
} else {
setIterationsCount(iterationsCount + 1);
setShowIteration(true);
}
} else if (!timingsVisible) {
setResults(results.concat(Measurer.getTimings()));
setTimingsVisible(true);
}
}
}, [itemsVisible, iterations, iterationsCount, showIteration, results, timingsVisible]);

const runTest = React.useCallback(() => {
setItemsVisible(!itemsVisible);
if (itemsVisible) {
setShowIteration(false);
setTimingsVisible(false);
setIterationsCount(0);
setResults([]);
}
}, [itemsVisible]);

return (
Expand All @@ -29,9 +58,7 @@ export const App = () => {
data-automationid="scenario"
onChange={(ev, option) => {
setItemsVisible(false);
if (option) {
setScenario(option);
}
setScenario(option!);
}}
/>
<TextField
Expand All @@ -41,6 +68,8 @@ export const App = () => {
type="number"
onChange={(ev, value) => {
setItemsVisible(false);
setIterationsCount(0);
setResults([]);
setCount(Number(value));
}}
/>
Expand All @@ -51,14 +80,39 @@ export const App = () => {
type="number"
onChange={(ev, value) => {
setItemsVisible(false);
setIterationsCount(0);
setResults([]);
setIterations(Number(value));
}}
/>
<PrimaryButton text={itemsVisible ? 'Reset' : 'Run test'} onClick={() => setItemsVisible(!itemsVisible)} className="runTest" />
<PrimaryButton text={itemsVisible ? 'Reset' : 'Run test'} onClick={runTest} className="runTest" />
<Text className="iterations">
<b>Iteration:</b> {iterationsCount} of {iterations}
</Text>
</Stack>
{timingsVisible && <MeasurerTimings />}
{timingsVisible && (
<Stack>
<Text className="total">
<b>Total time:</b> {results.map(result => result.totalTime + ', ')} ms
</Text>
<Text className="peritem">
<b>Per item:</b> {results.map(result => result.individualTime + ', ')} ms
</Text>
</Stack>
)}
{/* {timingsVisible &&
results.map((result) => (
<Stack>
<Text className="total">
<b>Total time:</b> {result.totalTime} ms
</Text>
<Text className="peritem">
<b>Per Item:</b> {result.individualTime} ms
</Text>
</Stack>)
)} */}
</Stack>
{itemsVisible && (
{showIteration && (
<div>
{Array.from({ length: count }, () => (
<Measurer>{scenario.data.content}</Measurer>
Expand Down
34 changes: 12 additions & 22 deletions apps/perf-test/src/Measurer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,13 +9,16 @@ export interface ITimings {
individualTime: number;
}

const _createTimings = (): ITimings => ({
count: 0,
initialTime: 0,
latestTime: 0,
totalTime: 0,
individualTime: 0
});
const _createTimings = (): ITimings => {
console.log('clearing timings');
return {
count: 0,
initialTime: 0,
latestTime: 0,
totalTime: 0,
individualTime: 0
};
};

const _round = (num: number, decimals: number): number => Number.parseFloat(num.toFixed(decimals));

Expand All @@ -27,6 +30,8 @@ export class Measurer extends React.Component<{}, {}> {

Measurer._timings = _createTimings();

console.log(JSON.stringify(timings));
JasonGore marked this conversation as resolved.
Show resolved Hide resolved

timings.totalTime = _round(timings.latestTime - timings.initialTime, 3);
timings.individualTime = _round(timings.totalTime / timings.count, 3);

Expand All @@ -50,18 +55,3 @@ export class Measurer extends React.Component<{}, {}> {
Measurer._timings.latestTime = performance.now();
}
}

export const MeasurerTimings = () => {
const timings = Measurer.getTimings();

return (
<Stack gap={20}>
<Text className="total">
<b>Total time:</b> {timings.totalTime}ms
</Text>
<Text className="peritem">
<b>Per Item:</b> {timings.individualTime}ms
</Text>
</Stack>
);
};
24 changes: 24 additions & 0 deletions apps/perf-test/src/index.scenarios.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import * as React from 'react';
import * as ReactDOM from 'react-dom';
import { initializeIcons } from 'office-ui-fabric-react/lib/Icons';
import * as qs from 'querystring';

const scenarios = require('./scenarios/scenarioList');

initializeIcons();

const div = document.createElement('div');
document.body.appendChild(div);

// TODO: could default to displaying list of scenarios if param is not provided.
const defaultScenarioName = Object.keys(scenarios)[0];
const defaultIterations = 10;

const queryParams = qs.parse(window.location.search.substring(1));
const iterations = queryParams.iterations ? parseInt(queryParams.iterations as string, 10) : defaultIterations;
const scenario = queryParams.scenario ? (queryParams.scenario as string) : defaultScenarioName;

// TODO: Using React Fragments increases React (unstable_runWithPriority) render consumption from 4% to 26%.
// It'd be interesting to root cause why at some point.
// ReactDOM.render(<>{Array.from({ length: iterations }, () => (scenarios[scenario]))}</>, div);
ReactDOM.render(<div>{Array.from({ length: iterations }, () => scenarios[scenario])}</div>, div);
6 changes: 6 additions & 0 deletions apps/perf-test/src/scenarios/BaseButton.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import * as React from 'react';
import { BaseButton } from 'office-ui-fabric-react';

const scenario = <BaseButton text="I am a button" />;

export default scenario;
6 changes: 6 additions & 0 deletions apps/perf-test/src/scenarios/BaseButtonNew.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import * as React from 'react';
import { BaseButton } from '@uifabric/experiments';

const scenario = <BaseButton text="I am a button" />;

export default scenario;
6 changes: 6 additions & 0 deletions apps/perf-test/src/scenarios/DefaultButton.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import * as React from 'react';
import { DefaultButton } from 'office-ui-fabric-react';

const scenario = <DefaultButton text="I am a button" />;

export default scenario;
6 changes: 6 additions & 0 deletions apps/perf-test/src/scenarios/DefaultButtonNew.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import * as React from 'react';
import { Button } from '@uifabric/experiments';

const scenario = <Button>I am a button</Button>;

export default scenario;
25 changes: 25 additions & 0 deletions apps/perf-test/src/scenarios/DetailsRow.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
import * as React from 'react';
import { DetailsRow, IColumn, Selection, SelectionMode } from 'office-ui-fabric-react';

// tslint:disable-next-line:typedef
const Items = Array.from({ length: 10 }, (n, i) => ({
key: `Item ${i}`,
name: `Item ${i}`,
modified: new Date().toString(),
shared: 'Private',
size: `${Math.round(Math.random() * 1000) / 10}KB`
}));

const selection = new Selection();
selection.setItems(Items);

const Columns: IColumn[] = [
{ key: 'a', name: 'Name', fieldName: 'name', minWidth: 200, maxWidth: 400 },
{ key: 'b', name: 'Last modified', fieldName: 'modified', minWidth: 200, maxWidth: 400 },
{ key: 'c', name: 'Shared', fieldName: 'shared', minWidth: 300, maxWidth: 300 },
{ key: 'c', name: 'Size', fieldName: 'size', minWidth: 300, maxWidth: 300 }
];

const scenario = <DetailsRow itemIndex={0} item={Items[0]} columns={Columns} selection={selection} selectionMode={SelectionMode.single} />;

export default scenario;
36 changes: 36 additions & 0 deletions apps/perf-test/src/scenarios/DetailsRowNoStyles.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import * as React from 'react';
import { createTheme, DetailsRowBase, IColumn, Selection, SelectionMode } from 'office-ui-fabric-react';

// tslint:disable-next-line:typedef
const Items = Array.from({ length: 10 }, (n, i) => ({
key: `Item ${i}`,
name: `Item ${i}`,
modified: new Date().toString(),
shared: 'Private',
size: `${Math.round(Math.random() * 1000) / 10}KB`
}));

const selection = new Selection();
selection.setItems(Items);

const Columns: IColumn[] = [
{ key: 'a', name: 'Name', fieldName: 'name', minWidth: 200, maxWidth: 400 },
{ key: 'b', name: 'Last modified', fieldName: 'modified', minWidth: 200, maxWidth: 400 },
{ key: 'c', name: 'Shared', fieldName: 'shared', minWidth: 300, maxWidth: 300 },
{ key: 'c', name: 'Size', fieldName: 'size', minWidth: 300, maxWidth: 300 }
];

const defaultTheme = createTheme({});

const scenario = (
<DetailsRowBase
theme={defaultTheme}
itemIndex={0}
item={Items[0]}
columns={Columns}
selection={selection}
selectionMode={SelectionMode.single}
/>
);

export default scenario;
6 changes: 6 additions & 0 deletions apps/perf-test/src/scenarios/DocumentCardTitle.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import * as React from 'react';
import { DocumentCardTitle } from 'office-ui-fabric-react';

const scenario = <DocumentCardTitle title="This is the Title of a Very Interesting Document That Everyone Wnats to Read" shouldTruncate />;

export default scenario;
21 changes: 21 additions & 0 deletions apps/perf-test/src/scenarios/MenuButton.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import * as React from 'react';
import { DefaultButton } from 'office-ui-fabric-react';

const menuProps = {
items: [
{
key: 'emailMessage',
text: 'Email message',
iconProps: { iconName: 'Mail' }
},
{
key: 'calendarEvent',
text: 'Calendar event',
iconProps: { iconName: 'Calendar' }
}
]
};

const scenario = <DefaultButton text="I am a button" menuProps={menuProps} />;

export default scenario;
Loading