perf(Theme): performance measures for Context usage by layershifter · Pull Request #1106 · microsoft/fluent-ui-react

layershifter · 2019-03-26T12:35:15Z

This PR has two targets:

test different approach to get theme
improve our profiling tooling

1. Use `minimatch` for `--filter`

This allows to get flexible filtering and run only needed examples:
yarn gulp perf --filter=*Context*Nesting*
yarn gulp perf "--filter=*(ContextUse|ContextProp|ContextConsumer|ContextFelaTheme).*"

2. Add progress bar for perf tests

3. Add markdown table to output

You can now just copy-paste to Github 👍

4. Run perf tests with production React

As we concentrated on production performance, with should run production React and pass process.env.NODE_ENV = production to get optimizations for third party libraries.

5. Add `perf:debug`

This will start development build of React to get measurements in flamegraph and will allow to run perf tests:

gulp perf --filter=*Context*Nesting* --times=100

Example	Avg	Avg diff	Median	Median diff
ContextPropNesting.perf.tsx	25	+1.5%	22.3	22.3
ContextUseNesting.perf.tsx	24.63	24.63	23.32	+4.57%
ContextConsumerNesting.perf.tsx	32	+29.92%	31.29	+40.31%
ContextFelaThemeNesting.perf.tsx	36.97	+50.1%	34.99	+56.91%

Example	Avg	Avg diff	Median	Median diff
ContextPropNesting.perf.tsx	25.49	+5.94%	25.71	+14.42%
ContextUseNesting.perf.tsx	24.06	24.06	22.47	22.47
ContextConsumerNesting.perf.tsx	31.8	+32.17%	29.77	+32.49%
ContextFelaThemeNesting.perf.tsx	34.3	+42.56%	33.18	+47.66%

Example	Avg	Avg diff	Median	Median diff
ContextPropNesting.perf.tsx	24.06	24.06	22.72	22.72
ContextUseNesting.perf.tsx	25.43	+5.69%	25.6	+12.68%
ContextConsumerNesting.perf.tsx	30.3	+25.94%	28.45	+25.22%
ContextFelaThemeNesting.perf.tsx	32.93	+36.87%	31.6	+39.08%

Example	Avg	Avg diff	Median	Median diff
ContextProp.perf.tsx	45.05	+4.43%	40.7	+4.01%
ContextUse.perf.tsx	43.14	43.14	39.13	39.13
ContextConsumer.perf.tsx	55.27	+28.12%	49.15	+25.61%
ContextFelaTheme.perf.tsx	61.83	+43.32%	55.94	+42.96%
ContextClassConsumer.perf.tsx	67.68	+56.88%	61.87	+58.11%
ContextClassField.perf.tsx	55.18	+27.91%	53.19	+35.93%

Example	Avg	Avg diff	Median	Median diff
ContextProp.perf.tsx	43.79	43.79	39.32	+0.87%
ContextUse.perf.tsx	44.76	+2.22%	38.98	38.98
ContextConsumer.perf.tsx	52.94	+20.9%	48.01	+23.17%
ContextFelaTheme.perf.tsx	63.08	+44.05%	55.4	+42.12%
ContextClassConsumer.perf.tsx	63.73	+45.54%	58.06	+48.95%
ContextClassField.perf.tsx	55.79	+27.4%	51.22	+31.4%

layershifter · 2019-03-26T12:35:36Z

build/gulp/tasks/perf.ts

Breaks test runs on Windows...

codecov · 2019-03-26T12:53:37Z

Codecov Report

Merging #1106 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #1106   +/-   ##
=======================================
  Coverage   82.38%   82.38%           
=======================================
  Files         733      733           
  Lines        8720     8720           
  Branches     1165     1165           
=======================================
  Hits         7184     7184           
  Misses       1521     1521           
  Partials       15       15

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0be142a...68aba16. Read the comment docs.

DustyTheBot · 2019-03-26T13:13:13Z

	Warnings
⚠️	There are no updates provided to CHANGELOG. Ensure there are no publicly visible changes introduced by this PR.

Generated by 🚫 dangerJS

layershifter · 2019-03-27T10:55:45Z

perf/src/index.tsx

+    matchBase: true,
+  })
+
+const sortObjectKeys = o =>


I will be grad to replace this with Lodash equivalent, if you know it, please propose 😺

I've always thought order of object keys is not guaranteed 🤔

Current Language Spec (since ES2015) insertion order is preserved, except in the case of keys that parse as integers (eg "7" or "99"), where behavior varies between browsers. For example, Chrome/V8 does not respect insertion order when the keys are parse as numeric.

https://stackoverflow.com/a/23202095/6488546 ¯_(ツ)_/¯

build/gulp/tasks/perf.ts

docs/src/examples/components/Box/Performance/ContextPropNesting.perf.tsx

layershifter · 2019-04-01T06:42:38Z

Example	Avg	Avg diff	Median	Median diff
ContextClassConsumer.perf.tsx	153.23	+35.88%	141.05	+32.84%
ContextClassField.perf.tsx	112.77	112.77	106.18	106.18

Example	Avg	Avg diff	Median	Median diff
ContextClassConsumer.perf.tsx	161.03	+25.27%	147.59	+26.25%
ContextClassField.perf.tsx	128.55	128.55	116.9	116.9

Example	Avg	Avg diff	Median	Median diff
ContextClassConsumer.perf.tsx	146.68	+30.66%	133.66	+22.92%
ContextClassField.perf.tsx	112.26	112.26	108.74	108.74

Actually, there is one more API that we can use Class.contextType. It was introduced in 16.6.

…dust-ui/react into perf/context

…dust-ui/react into perf/context # Conflicts: # package.json # yarn.lock

…dust-ui/react into perf/context

kuzhelov · 2019-04-03T18:04:28Z

build/gulp/tasks/perf.ts

+  _.round((actualValue / minValue) * 100 - 100, 2)
+
+const createMarkdownTable = (
+  data,


can we introduce type for data arg - to make it easier to reason about structure of contained data?

renamed to normalizedMeasures and add more types 👍

kuzhelov · 2019-04-03T18:23:58Z

yarn.lock

  resolved "https://registry.yarnpkg.com/react-is/-/react-is-16.6.3.tgz#d2d7462fcfcbe6ec0da56ad69047e47e56e7eac0"
  integrity sha512-u7FDWtthB4rWibG/+mFbVd5FvdI20yde86qKGx4lVUTWmPlSWQ4QxbBIrrs+HnXGbxOUlUzTAP/VDmvCwaP2yA==

+react-is@^16.7.0, react-is@^16.8.6:


just curious if there is a real reason to have so many versions of this package in resolutions - maybe it could be possible to reduce all into one

Deduped it 👍

kuzhelov · 2019-04-03T18:46:04Z

perf/src/index.tsx

+  const [filter, setFilter] = React.useState('')
+
+  return (
+    /* Heads up! This provider also does first run work! */


lets be a bit more explicit about first run meaning. Essentially, we need to provide the implications for the perf tests first, and then elaborate on the reason for them, something like

On first run, this Provider increases measured time due to style DOM elements being rendered to the browser. Subsequent rerenders, in contrast, are not rendering these style DOM elements again.

kuzhelov · 2019-04-03T18:47:30Z

docs/src/examples/components/Box/index.tsx

 import Types from './Types'

-const HeaderExamples = () => (
+const BoxExamples = () => (


kuzhelov · 2019-04-03T18:50:43Z

build/webpack.config.perf.ts

      docs: paths.base('docs'),
      src: paths.packageSrc('react'),
+
+      // We are React in production mode with tracing.


nit: seems that using word is missed

kuzhelov · 2019-04-03T18:56:25Z

perf/src/index.tsx

+const sortObjectKeys = o =>
+  Object.keys(o)
+    .sort()
+    .reduce((r, k) => ((r[k] = o[k]), r), {})


Removed it at all, it confused two people...

kuzhelov · 2019-04-03T18:58:02Z

yarn.lock

    react-is "^16.8.1"

-react-test-renderer@^16.0.0, react-test-renderer@^16.0.0-0:
+react-test-renderer@^16.0.0-0:


same here - is there any reason for us to have these two (I mean, non-addressable reason:)

kuzhelov · 2019-04-03T19:22:51Z

perf/src/index.tsx

-}
+const satisfiesFilter = (componentName: string, filter: string) =>
+  minimatch(componentName, filter || '*', {
+    matchBase: true,


nit: lets be a bit more strict here: we may rename componentName with either componentExampleName or most explicit componentFilePath to better indicate the fact that this is more than just name of the component. It will make it easier to get the reason for this option from code

componentFilePath is correct 👍

…dust-ui/react into perf/context

kuzhelov · 2019-04-03T21:24:45Z

perf/src/index.html

 </head>
 <body>
 <div id="root"></div>
+<div id="control"></div>


lets introduce comment for the second mount point that would explain what it is used for (and, maybe we could rename it to control-tools, tools-panel or something like that)

…rf/context

layershifter requested review from dvdzkwsk, kuzhelov, levithomason, miroslavstastny and mnajdova as code owners March 26, 2019 12:35

layershifter commented Mar 26, 2019

View reviewed changes

build/gulp/tasks/perf.ts Outdated

Copy link

Member Author

layershifter Mar 26, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Breaks test runs on Windows...

perf tooling

90fab36

layershifter force-pushed the perf/context branch from 806db58 to 90fab36 Compare March 27, 2019 10:40

layershifter added 🚀 ready for review perf labels Mar 27, 2019

layershifter commented Mar 27, 2019

View reviewed changes

miroslavstastny reviewed Mar 28, 2019

View reviewed changes

build/gulp/tasks/perf.ts Show resolved Hide resolved

docs/src/examples/components/Box/Performance/ContextPropNesting.perf.tsx Outdated Show resolved Hide resolved

layershifter added 6 commits April 1, 2019 09:42

add perf tests

5e0b500

Merge branches 'master' and 'perf/context' of https://github.com/star…

411995a

…dust-ui/react into perf/context

add dependency, silence in CI

0f544ff

Merge branches 'master' and 'perf/context' of https://github.com/star…

f4f2b76

…dust-ui/react into perf/context # Conflicts: # package.json # yarn.lock

remove examples

56228b8

update react-hot-loader

75265f8

layershifter force-pushed the perf/context branch from 3265141 to 75265f8 Compare April 2, 2019 13:09

layershifter added 2 commits April 2, 2019 15:09

Merge branches 'master' and 'perf/context' of https://github.com/star…

c1d98f3

…dust-ui/react into perf/context

update condition

daba12b

kuzhelov reviewed Apr 3, 2019

View reviewed changes

layershifter added 8 commits April 3, 2019 22:24

add more types

6e00ddc

Merge branches 'master' and 'perf/context' of https://github.com/star…

2b232f8

…dust-ui/react into perf/context

dedupe react-is

128b8aa

update note for Provider

b1c2ffb

update comment

ecc5994

remove sortObjectKeys

5255fc3

dedupe react-test-renderer

91f1b3f

=> componentFilePath

a1155cd

kuzhelov reviewed Apr 3, 2019

View reviewed changes

kuzhelov approved these changes Apr 3, 2019

View reviewed changes

miroslavstastny approved these changes Apr 4, 2019

View reviewed changes

layershifter added 2 commits April 4, 2019 10:05

rename container

fa03224

Merge branch 'master' of https://github.com/stardust-ui/react into pe…

68aba16

…rf/context

layershifter merged commit f5faa4e into master Apr 4, 2019

delete-merged-branch bot deleted the perf/context branch April 4, 2019 09:26

layershifter mentioned this pull request Apr 4, 2019

perf(renderComponent): drop FelaTheme and use React.Context directly #1163

Merged

Conversation

layershifter commented Mar 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

1. Use minimatch for --filter

2. Add progress bar for perf tests

3. Add markdown table to output

4. Run perf tests with production React

5. Add perf:debug

Uh oh!

Choose a reason for hiding this comment

Uh oh!

codecov bot commented Mar 26, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

DustyTheBot commented Mar 26, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

layershifter commented Apr 1, 2019

Uh oh!

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

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

layershifter commented Mar 26, 2019 •

edited

Loading

1. Use `minimatch` for `--filter`

5. Add `perf:debug`

codecov bot commented Mar 26, 2019 •

edited

Loading

kuzhelov Apr 3, 2019 •

edited

Loading