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

ResizeGroup: Improve perf by making sure resize data state change is batched. #15701

Merged
merged 4 commits into from
Oct 27, 2020
Merged

ResizeGroup: Improve perf by making sure resize data state change is batched. #15701

merged 4 commits into from
Oct 27, 2020

Conversation

xugao
Copy link
Contributor

@xugao xugao commented Oct 26, 2020 •
edited
Loading

Pull request checklist

  • Addresses an existing issue: Fixes #0000
  • Include a change request file using $ yarn change

Description of changes

React is not batching the state changes when states are set in callbacks like setTimeout or requestAnimationFrame. See issue: facebook/react#14259

In ResizeGroup, we call updateResizeState from requestAnimationFrame callback. updateResizeState call different useState setters which causes un-batched re-renders. The fix here is to group resize data into a single state in useResizeData hook.

Also noticed couple places where we should memoize the callbacks returned from updateResizeState.

Focus areas to test

perf-test results for components which uses ResizeGroup: Breadcrumb, CommandBar

@xugao xugao force-pushed the perf-resize-group branch from a5c5ad6 to 9f2e9ac Compare October 26, 2020 21:49
@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 26, 2020 •
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 749b825:

Sandbox Source
Fluent UI Button Configuration
codesandbox-react-template Configuration
codesandbox-react-northstar-template Configuration

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Oct 26, 2020 •
edited
Loading

Perf Analysis

No significant results to display.

All results

Scenario Render type Master Ticks PR Ticks Iterations Status
Avatar mount 852 841 5000
BaseButtonCompat mount 937 903 5000
Breadcrumb mount 160618 43626 5000
Checkbox mount 1555 1540 5000
CheckboxBase mount 1251 1242 5000
ChoiceGroup mount 4934 4759 5000
ComboBox mount 961 995 1000
CommandBar mount 22217 10246 1000
ContextualMenu mount 6136 6146 1000
DefaultButtonCompat mount 1125 1155 5000
DetailsRow mount 3726 3587 5000
DetailsRowFast mount 3630 3661 5000
DetailsRowNoStyles mount 3535 3483 5000
Dialog mount 1514 1539 1000
DocumentCardTitle mount 1836 1785 1000
Dropdown mount 3458 3377 5000
FocusTrapZone mount 1855 1810 5000
FocusZone mount 1853 1820 5000
IconButtonCompat mount 1725 1788 5000
Label mount 337 341 5000
Layer mount 1778 1839 5000
Link mount 468 508 5000
MenuButtonCompat mount 1465 1486 5000
MessageBar mount 2011 2063 5000
Nav mount 3309 3277 1000
OverflowSet mount 1105 1040 5000
Panel mount 1458 1452 1000
Persona mount 885 866 1000
Pivot mount 1406 1426 1000
PrimaryButtonCompat mount 1274 1312 5000
Rating mount 7612 7449 5000
SearchBox mount 1298 1315 5000
Shimmer mount 2594 2574 5000
Slider mount 1880 1949 5000
SpinButton mount 4991 5012 5000
Spinner mount 435 423 5000
SplitButtonCompat mount 3141 3175 5000
Stack mount 487 502 5000
StackWithIntrinsicChildren mount 1582 1570 5000
StackWithTextChildren mount 4666 4674 5000
SwatchColorPicker mount 10269 10299 5000
TagPicker mount 2753 2825 5000
TeachingBubble mount 11602 11678 5000
Text mount 437 437 5000
TextField mount 1463 1420 5000
ThemeProvider mount 2043 2014 5000
ThemeProvider virtual-rerender 669 663 5000
Toggle mount 827 817 5000
button mount 557 532 5000
buttonNative mount 125 115 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.44 0.51 0.86:1 2000 886
🦄 Button.Fluent 0.11 0.24 0.46:1 5000 561
🔧 Checkbox.Fluent 0.67 0.34 1.97:1 1000 669
🎯 Dialog.Fluent 0.16 0.22 0.73:1 5000 806
🔧 Dropdown.Fluent 3.04 0.4 7.6:1 1000 3044
🔧 Icon.Fluent 0.15 0.06 2.5:1 5000 770
🦄 Image.Fluent 0.08 0.13 0.62:1 5000 399
🔧 Slider.Fluent 1.63 0.45 3.62:1 1000 1630
🔧 Text.Fluent 0.08 0.04 2:1 5000 385
🦄 Tooltip.Fluent 0.12 0.92 0.13:1 5000 617

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
SkeletonMinimalPerf.default 476 431 1.1:1
TreeWith60ListItems.default 225 208 1.08:1
AttachmentMinimalPerf.default 178 166 1.07:1
VideoMinimalPerf.default 691 647 1.07:1
Tooltip.Fluent 617 576 1.07:1
Checkbox.Fluent 669 630 1.06:1
AnimationMinimalPerf.default 441 423 1.04:1
BoxMinimalPerf.default 405 388 1.04:1
ButtonMinimalPerf.default 189 181 1.04:1
ButtonUseCssPerf.default 864 831 1.04:1
ChatDuplicateMessagesPerf.default 447 429 1.04:1
ChatWithPopoverPerf.default 487 467 1.04:1
DividerMinimalPerf.default 395 381 1.04:1
DropdownManyItemsPerf.default 787 759 1.04:1
Image.Fluent 399 383 1.04:1
ButtonSlotsPerf.default 630 609 1.03:1
CardMinimalPerf.default 583 564 1.03:1
LabelMinimalPerf.default 453 439 1.03:1
ListNestedPerf.default 599 583 1.03:1
MenuMinimalPerf.default 897 870 1.03:1
Dropdown.Fluent 3044 2955 1.03:1
ButtonOverridesMissPerf.default 1756 1730 1.02:1
DropdownMinimalPerf.default 3049 2983 1.02:1
GridMinimalPerf.default 373 367 1.02:1
PortalMinimalPerf.default 171 167 1.02:1
ProviderMergeThemesPerf.default 2148 2112 1.02:1
AttachmentSlotsPerf.default 1145 1137 1.01:1
CheckboxMinimalPerf.default 2952 2919 1.01:1
EmbedMinimalPerf.default 1960 1942 1.01:1
ImageMinimalPerf.default 394 392 1.01:1
ListWith60ListItems.default 964 954 1.01:1
LoaderMinimalPerf.default 763 755 1.01:1
MenuButtonMinimalPerf.default 1639 1629 1.01:1
PopupMinimalPerf.default 741 735 1.01:1
SegmentMinimalPerf.default 373 370 1.01:1
SplitButtonMinimalPerf.default 3847 3796 1.01:1
TextAreaMinimalPerf.default 505 500 1.01:1
Icon.Fluent 770 759 1.01:1
AvatarMinimalPerf.default 489 488 1:1
CarouselMinimalPerf.default 455 454 1:1
ChatMinimalPerf.default 627 625 1:1
InputMinimalPerf.default 1355 1350 1:1
IconMinimalPerf.default 687 685 1:1
TextMinimalPerf.default 377 376 1:1
CustomToolbarPrototype.default 3956 3942 1:1
TooltipMinimalPerf.default 840 844 1:1
Dialog.Fluent 806 809 1:1
Slider.Fluent 1630 1633 1:1
Text.Fluent 385 386 1:1
AlertMinimalPerf.default 298 300 0.99:1
ButtonUseCssNestingPerf.default 1107 1114 0.99:1
FlexMinimalPerf.default 321 325 0.99:1
RefMinimalPerf.default 250 252 0.99:1
StatusMinimalPerf.default 733 739 0.99:1
TableManyItemsPerf.default 2178 2210 0.99:1
ToolbarMinimalPerf.default 950 962 0.99:1
DialogMinimalPerf.default 818 834 0.98:1
HeaderMinimalPerf.default 378 387 0.98:1
ItemLayoutMinimalPerf.default 1264 1290 0.98:1
RadioGroupMinimalPerf.default 456 467 0.98:1
SliderMinimalPerf.default 1580 1611 0.98:1
HeaderSlotsPerf.default 783 808 0.97:1
LayoutMinimalPerf.default 430 443 0.97:1
ProviderMinimalPerf.default 1036 1068 0.97:1
TableMinimalPerf.default 425 438 0.97:1
TreeMinimalPerf.default 897 924 0.97:1
Avatar.Fluent 886 914 0.97:1
Button.Fluent 561 576 0.97:1
FormMinimalPerf.default 419 440 0.95:1
ListMinimalPerf.default 517 542 0.95:1
ReactionMinimalPerf.default 415 444 0.93:1
ListCommonPerf.default 641 694 0.92:1
AccordionMinimalPerf.default 160 176 0.91:1

@size-auditor
Copy link

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

Asset size changes

⚠️ Insufficient baseline data to detect size changes

Unable to find bundle size details for Baseline commit: 0fe2bfa

Possible causes

  • The baseline build 0fe2bfa is broken
  • The Size Auditor run for the baseline build 0fe2bfa was not triggered

Recommendations

  • Please merge your branch for this Pull request with the latest master build and commit your changes once again

ecraig12345
ecraig12345 approved these changes Oct 27, 2020
View reviewed changes
Copy link
Member

@ecraig12345 ecraig12345 left a comment

Choose a reason for hiding this comment

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

Nice fix!

@xugao xugao merged commit f67490d into microsoft:master Oct 27, 2020
@msft-github-bot
Copy link
Contributor

🎉@fluentui/[email protected] has been released which incorporates this pull request.:tada:

Handy links:

SethDonohue pushed a commit to SethDonohue/fluentui that referenced this pull request Nov 2, 2020
@xugao
ResizeGroup: Improve perf by making sure resize data state change is …
7961d32 
…batched. (microsoft#15701)

* fix resize group perf

* Change files

* change to useReducer

* add comments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ecraig12345 ecraig12345 ecraig12345 approved these changes

@micahgodbolt micahgodbolt micahgodbolt approved these changes

@dzearing dzearing Awaiting requested review from dzearing

Assignees

@tomi-msft tomi-msft

Labels
Component: ResizeGroup
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@xugao @msft-github-bot @micahgodbolt @ecraig12345 @tomi-msft