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.

Sign up for GitHub

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

Jump to bottom

fix: Improve Stack's style recalculation performance by selectively applying children selectors #25381

Merged
merged 3 commits into from
Oct 26, 2022
Merged

fix: Improve Stack's style recalculation performance by selectively applying children selectors #25381

merged 3 commits into from
Oct 26, 2022

Conversation

khmakoto
Copy link
Member

Previous Behavior

The Stack component applied children selectors independently of if they really needed to be applied or not. These selectors are expensive for style recalculation and are major contributors for slowdown in that step.

Here are the results of a trace running on our v8 docsite's Stack page, where most of the most expensive selectors are Stack-related:

  • rulesFastRejected: 77823
  • rulesRejected: 6660
  • elapsed time: 7329 microseconds

New Behavior

The Stack component now applies most children selectors only when the props indicate they are needed to be applied, which should create better style recalculation performance depending on the props passed in.

Here are the results of a trace running on our v8 docsite's Stack page after the change:

  • rulesFastRejected: 46619
  • rulesRejected: 5935
  • elapsed time: 5199 microseconds

This PR also removes the usage of the selectors keyword in Stack.styles.ts since that is not needed anymore and removing them should help with bundle size.

Related Issue(s)

Fixes part of #24259

Humberto Makoto Morimoto Burgos added 2 commits October 25, 2022 16:21
@khmakoto khmakoto self-assigned this Oct 25, 2022
@khmakoto khmakoto requested review from a team as code owners October 25, 2022 23:48
spmonahan
spmonahan approved these changes Oct 26, 2022
View reviewed changes
Copy link
Contributor

@spmonahan spmonahan left a comment

Choose a reason for hiding this comment

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

Nice work!

@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 26, 2022 •
edited
Loading

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 14382e2:

Sandbox Source
@fluentui/react 8 starter Configuration
@fluentui/react-components 9 starter Configuration

@size-auditor
Copy link

size-auditor bot commented Oct 26, 2022 •
edited
Loading

Asset size changes

Project Bundle Baseline Size New Size Difference
office-ui-fabric-react fluentui-react-Stack 39.301 kB 39.335 kB ExceedsBaseline     34 bytes
office-ui-fabric-react fluentui-react-TeachingBubble 193.109 kB 193.143 kB ExceedsBaseline     34 bytes

ExceedsTolerance Over Tolerance (1024 B) ExceedsBaseline Over Baseline BelowBaseline Below Baseline New New Deleted  Removed 1 kB = 1000 B

Baseline commit: e80f15987f9ab7c3b1e569e60d8182bde09b1c99 (build)

@fabricteam
Copy link
Collaborator

fabricteam commented Oct 26, 2022 •
edited
Loading

📊 Bundle size report

🤖 This report was generated against e80f15987f9ab7c3b1e569e60d8182bde09b1c99

@fabricteam
Copy link
Collaborator

fabricteam commented Oct 26, 2022 •
edited
Loading

Perf Analysis (@fluentui/react)

Scenario Render type Master Ticks PR Ticks Iterations Status
ThemeProvider virtual-rerender-with-unmount 2133 2160 5000 Possible regression
All results

Scenario Render type Master Ticks PR Ticks Iterations Status
BaseButton mount 1183 1127 5000
Breadcrumb mount 2910 2964 1000
Checkbox mount 2583 2645 5000
CheckboxBase mount 2344 2378 5000
ChoiceGroup mount 4400 4357 5000
ComboBox mount 1194 1255 1000
CommandBar mount 9577 9363 1000
ContextualMenu mount 11277 11353 1000
DefaultButton mount 1372 1308 5000
DetailsRow mount 3589 3562 5000
DetailsRowFast mount 3604 3608 5000
DetailsRowNoStyles mount 3413 3473 5000
Dialog mount 3080 3079 1000
DocumentCardTitle mount 583 578 1000
Dropdown mount 3212 3229 5000
FocusTrapZone mount 2018 2041 5000
FocusZone mount 1985 1983 5000
GroupedList mount 54337 59972 2
GroupedList virtual-rerender 24806 24864 2
GroupedList virtual-rerender-with-unmount 90317 91875 2
GroupedListV2 mount 535 553 2
GroupedListV2 virtual-rerender 533 545 2
GroupedListV2 virtual-rerender-with-unmount 537 529 2
IconButton mount 1815 1845 5000
Label mount 660 717 5000
Layer mount 3987 4136 5000
Link mount 778 794 5000
MenuButton mount 1586 1656 5000
MessageBar mount 2212 2187 5000
Nav mount 3002 3058 1000
OverflowSet mount 1266 1256 5000
Panel mount 2496 2323 1000
Persona mount 1259 1225 1000
Pivot mount 1649 1616 1000
PrimaryButton mount 1384 1365 5000
Rating mount 6488 6498 5000
SearchBox mount 1476 1391 5000
Shimmer mount 2808 2632 5000
Slider mount 1986 1965 5000
SpinButton mount 4308 4266 5000
Spinner mount 781 754 5000
SplitButton mount 2962 2931 5000
Stack mount 853 773 5000
StackWithIntrinsicChildren mount 2298 2250 5000
StackWithTextChildren mount 4676 4695 5000
SwatchColorPicker mount 9955 10257 5000
TagPicker mount 2473 2415 5000
TeachingBubble mount 86826 87235 5000
Text mount 773 769 5000
TextField mount 1606 1589 5000
ThemeProvider mount 1476 1523 5000
ThemeProvider virtual-rerender 1074 1076 5000
ThemeProvider virtual-rerender-with-unmount 2133 2160 5000 Possible regression
Toggle mount 1098 1119 5000
buttonNative mount 495 544 5000

jspurlin
jspurlin approved these changes Oct 26, 2022
View reviewed changes
Copy link
Contributor

@jspurlin jspurlin left a comment

Choose a reason for hiding this comment

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

:shipit:

@khmakoto khmakoto enabled auto-merge (squash) October 26, 2022 02:10
@khmakoto khmakoto merged commit 135655c into microsoft:master Oct 26, 2022
@khmakoto khmakoto deleted the stackSelectiveSelectors branch October 26, 2022 03:29
@khmakoto khmakoto mentioned this pull request Oct 27, 2022
Merged
marcosmoura added a commit to marcosmoura/fluentui that referenced this pull request Oct 27, 2022
@marcosmoura
Merge branch 'master' into marcosmoura/feat/selectable-cards
c0f216e 
* master: (21 commits)
  fix(react-tabster): make acceptCondition optional as Tabster dont require it (microsoft#25416)
  chore: adds disableButtonEnhancement on triggers (microsoft#25211)
  applying package updates
  feat: Add support for the wbtx whiteboard file extension (microsoft#25346)
  chore(react-select): migrate to new package structure (microsoft#25359)
  chore(react-divider): migrate to new package structure (microsoft#25360)
  fix(docsite): codesandbox exports now working properly for newly migrated v9 packages (microsoft#25388)
  Website: fix focus border on UHF footer links (microsoft#25389)
  applying package updates
  feat: re-export react-table logic hooks (microsoft#25386)
  chore(react-aria): migrate to new package structure (microsoft#25199)
  chore(babel-preset-global-context): migrate to new package structure (microsoft#25340)
  applying package updates
  fix: Improve Stack's style recalculation performance by selectively applying children selectors (microsoft#25381)
  applying package updates
  fix(projects-test): explicitly install next version 12 to fix CI (microsoft#25374)
  allow details element to be toggled inside selection and focus zones (microsoft#25324)
  fix(react-persona): Changing persona's versions to pinned versions (microsoft#25367)
  update fast element and foundation package versions stable (microsoft#25364)
  chore(keyboard-keys, priority-overflow, react-context-selector, react-conformance-griffel): migrate to new package structure (microsoft#25362)
  ...
NotWoods pushed a commit to NotWoods/fluentui that referenced this pull request Nov 18, 2022
@khmakoto @NotWoods
fix: Improve Stack's style recalculation performance by selectively a…
45cb457 
…pplying children selectors (microsoft#25381)

* fix: Improve Stack's style recalculation performance by selectively applying children selectors.

* Adding change file.

Co-authored-by: Humberto Makoto Morimoto Burgos <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@spmonahan spmonahan spmonahan approved these changes

@jspurlin jspurlin jspurlin approved these changes

Assignees

@khmakoto khmakoto

Labels
Area: Performance Component: Stack Fluent UI react (v8) Issues about @fluentui/react (v8)
Projects
None yet
Milestone
October Project Cycle Q4 2022
Development

Successfully merging this pull request may close these issues.
4 participants
@khmakoto @fabricteam @jspurlin @spmonahan