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

perf: omit "_.findKey" usage in focusVisibleEnhancer() #13343

Merged
merged 4 commits into from
May 27, 2020
Merged

perf: omit "_.findKey" usage in focusVisibleEnhancer() #13343

merged 4 commits into from
May 27, 2020

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented May 27, 2020 •
edited

Pull request checklist

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

Description of changes

It's fine to change by reference and update the object itself instead of creating a new one and trying to find a key via _.findKey(). There is around 2-3% perf improvement with these changes.

Focus areas to test

(optional)

@msft-github-bot msft-github-bot added the Fluent UI react-northstar (v0) Work related to Fluent UI V0 label May 27, 2020
@msft-github-bot
Copy link
Contributor

msft-github-bot commented May 27, 2020 •
edited

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Iterations Status
BaseButton 786 766 5000
ButtonNext 433 441 5000
Checkbox 1488 1529 5000
CheckboxBase 1214 1288 5000
CheckboxNext 1510 1451 5000
ChoiceGroup 4814 4721 5000
ComboBox 891 876 1000
CommandBar 7468 7463 1000
ContextualMenu 12891 12913 1000
DefaultButton 1024 1044 5000
DetailsRow 3353 3233 5000
DetailsRow (fast icons) 3321 3258 5000
DetailsRow without styles 3160 3083 5000
Dialog 1429 1405 1000
DocumentCardTitle with truncation 1873 1867 1000
Dropdown 2314 2289 5000
FocusZone 1632 1678 5000
IconButton 1593 1643 5000
Label 298 307 5000
Link 441 447 5000
LinkNext 425 446 5000
MenuButton 1306 1334 5000
Nav 3101 3081 1000
Panel 1397 1367 1000
Persona 797 797 1000
Pivot 1308 1357 1000
PrimaryButton 1177 1141 5000
SearchBox 1175 1173 5000
Slider 1399 1387 5000
SliderNext 1789 1757 5000
Spinner 397 383 5000
SplitButton 3002 2883 5000
Stack 455 465 5000
Stack with Intrinsic children 1752 1768 5000
Stack with Text children 4689 4656 5000
TagPicker 2658 2685 5000
Text 374 348 5000
TextField 1305 1344 5000
Toggle 809 829 5000
ToggleNext 843 859 5000
button 75 74 5000

Perf Analysis (Fluent)

Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🔧 Avatar.Fluent 0.66 0.45 1.47:1 2000 1323
🦄 Button.Fluent 0.1 0.18 0.56:1 5000 514
🔧 Checkbox.Fluent 1 0.33 3.03:1 1000 1000
🔧 Dialog.Fluent 0.55 0.21 2.62:1 5000 2726
🔧 Dropdown.Fluent 6.07 0.42 14.45:1 1000 6068
🔧 Icon.Fluent 0.13 0.05 2.6:1 5000 648
🎯 Image.Fluent 0.07 0.1 0.7:1 5000 330
🔧 Slider.Fluent 2.69 0.34 7.91:1 1000 2694
🔧 Text.Fluent 0.06 0.02 3:1 5000 323
🦄 Tooltip.Fluent 0.09 16.81 0.01:1 5000 464

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
RadioGroupMinimalPerf.default 390 364 1.07:1
GridMinimalPerf.default 1282 1214 1.06:1
PopupMinimalPerf.default 261 249 1.05:1
TextMinimalPerf.default 331 314 1.05:1
Tooltip.Fluent 464 443 1.05:1
BoxMinimalPerf.default 312 301 1.04:1
FlexMinimalPerf.default 262 253 1.04:1
HeaderSlotsPerf.default 736 706 1.04:1
TableMinimalPerf.default 370 355 1.04:1
Button.Fluent 514 492 1.04:1
FormMinimalPerf.default 358 347 1.03:1
HierarchicalTreeMinimalPerf.default 387 376 1.03:1
PortalMinimalPerf.default 103 100 1.03:1
Dialog.Fluent 2726 2657 1.03:1
AccordionMinimalPerf.default 131 129 1.02:1
CarouselMinimalPerf.default 495 487 1.02:1
CheckboxMinimalPerf.default 4938 4860 1.02:1
DropdownMinimalPerf.default 6031 5937 1.02:1
ImageMinimalPerf.default 343 337 1.02:1
InputMinimalPerf.default 1525 1493 1.02:1
LabelMinimalPerf.default 367 360 1.02:1
ListNestedPerf.default 1045 1028 1.02:1
MenuMinimalPerf.default 785 767 1.02:1
TextAreaMinimalPerf.default 425 416 1.02:1
ToolbarMinimalPerf.default 766 752 1.02:1
Dropdown.Fluent 6068 5948 1.02:1
Icon.Fluent 648 637 1.02:1
AttachmentMinimalPerf.default 139 138 1.01:1
EmbedMinimalPerf.default 3252 3210 1.01:1
LoaderMinimalPerf.default 1076 1067 1.01:1
ReactionMinimalPerf.default 346 343 1.01:1
StatusMinimalPerf.default 618 612 1.01:1
CustomToolbarPrototype.default 4625 4596 1.01:1
AlertMinimalPerf.default 312 312 1:1
AvatarMinimalPerf.default 688 688 1:1
ChatWithPopoverPerf.default 648 650 1:1
DropdownManyItemsPerf.default 1980 1984 1:1
ItemLayoutMinimalPerf.default 2535 2543 1:1
ListCommonPerf.default 1085 1090 1:1
ProviderMergeThemesPerf.default 1723 1719 1:1
SplitButtonMinimalPerf.default 3713 3720 1:1
IconMinimalPerf.default 611 611 1:1
Image.Fluent 330 330 1:1
Text.Fluent 323 322 1:1
AnimationMinimalPerf.default 678 685 0.99:1
ButtonSlotsPerf.default 690 695 0.99:1
DialogMinimalPerf.default 2655 2679 0.99:1
MenuButtonMinimalPerf.default 1590 1598 0.99:1
RefMinimalPerf.default 184 185 0.99:1
TreeWith60ListItems.default 257 259 0.99:1
VideoMinimalPerf.default 543 546 0.99:1
Avatar.Fluent 1323 1335 0.99:1
Slider.Fluent 2694 2730 0.99:1
CardMinimalPerf.default 514 526 0.98:1
ChatDuplicateMessagesPerf.default 517 530 0.98:1
ChatMinimalPerf.default 533 545 0.98:1
HeaderMinimalPerf.default 315 322 0.98:1
LayoutMinimalPerf.default 363 369 0.98:1
TooltipMinimalPerf.default 670 683 0.98:1
AttachmentSlotsPerf.default 1119 1157 0.97:1
DividerMinimalPerf.default 314 325 0.97:1
ListMinimalPerf.default 427 441 0.97:1
ListWith60ListItems.default 1488 1532 0.97:1
ProviderMinimalPerf.default 761 788 0.97:1
SegmentMinimalPerf.default 300 309 0.97:1
SliderMinimalPerf.default 2709 2793 0.97:1
Checkbox.Fluent 1000 1034 0.97:1
ButtonMinimalPerf.default 153 160 0.96:1
TreeMinimalPerf.default 1193 1257 0.95:1

@size-auditor
Copy link

size-auditor bot commented May 27, 2020 •
edited

Asset size changes

⚠️ Insufficient baseline data to detect size changes

Unable to find bundle size details for Baseline commit: 1dc036b

Possible causes

  • The baseline build 1dc036b is broken
  • The Size Auditor run for the baseline build 1dc036b was not triggered

Recommendations

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

@layershifter layershifter changed the title perf: fix focusVisibleEnhancer perf: omit "_.findKey" usage in focusVisibleEnhancer() May 27, 2020
@layershifter layershifter merged commit b4f0766 into master May 27, 2020
@layershifter layershifter deleted the perf/fix-visible-enhancer branch May 27, 2020 13:23
miroslavstastny pushed a commit to levithomason/fluentui that referenced this pull request Jun 8, 2020
@layershifter @miroslavstastny
perf: omit "_.findKey" usage in focusVisibleEnhancer() (microsoft#13343)
31e869c 
* perf: fix focusVisibleEnhancer

* fix rebase issue

* fix unused import

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

@miroslavstastny miroslavstastny miroslavstastny approved these changes

@dvdzkwsk dvdzkwsk Awaiting requested review from dvdzkwsk

@dzearing dzearing Awaiting requested review from dzearing

@jurokapsiar jurokapsiar Awaiting requested review from jurokapsiar

@levithomason levithomason Awaiting requested review from levithomason

@mnajdova mnajdova Awaiting requested review from mnajdova

@pompomon pompomon Awaiting requested review from pompomon

Assignees

@ecraig12345 ecraig12345

Labels
Fluent UI react-northstar (v0) Work related to Fluent UI V0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@layershifter @msft-github-bot @miroslavstastny @ecraig12345