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

[WIP] Enable faster rule caching for fela renderer #12412

Closed
wants to merge 4 commits into from

Conversation

dvdzkwsk
Copy link
Contributor

@dvdzkwsk dvdzkwsk commented Mar 25, 2020

Opening this PR up early so I can get feedback from the PR bots on performance impacts.

Updates fela's renderRule method to compute classNames on a more granular level: by property rather than by style object.

Rather than generating a full className for an entire style object at once, it now walks the style object and generates classes for each individual property. This allows us to avoid reprocessing styles that should already be cached.

NOTE: this approach does not work for monolithic classNames, since that treats an entire style object as a single entity. So, do not use this if you are using that mode.

Pull request checklist

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

Description of changes

(give an overview)

Focus areas to test

(optional)

Microsoft Reviewers: Open in CodeFlow

@DustyTheBot
Copy link

DustyTheBot commented Mar 25, 2020

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

Generated by 🚫 dangerJS against e92d26f

@msft-github-bot
Copy link
Contributor

msft-github-bot commented Mar 25, 2020

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Status
BaseButton 891 855
BaseButton (experiments) 1076 1108
DefaultButton 1165 1178
DefaultButton (experiments) 2050 2080
DetailsRow 3770 3694
DetailsRow (fast icons) 3607 3678
DetailsRow without styles 3463 3431
DocumentCardTitle with truncation 1665 1566
MenuButton 1535 1543
MenuButton (experiments) 3824 3707
PrimaryButton 1306 1347
PrimaryButton (experiments) 2171 2159
SplitButton 3290 3257
SplitButton (experiments) 7351 7375
Stack 519 528
Stack with Intrinsic children 1248 1279
Stack with Text children 4578 4608
Text 411 387
Toggle 1025 961
Toggle (experiments) 2428 2422
button 64 67

Perf Analysis (Fluent)

⚠️ 22 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
SliderMinimalPerf.default 1731 1676 1.03:1 analysis
Slider.Fluent 1768 1740 1.02:1 analysis
ProviderMinimalPerf.default 707 697 1.01:1 analysis
AnimationMinimalPerf.default 805 818 0.98:1 analysis
LayoutMinimalPerf.default 760 773 0.98:1 analysis
LoaderMinimalPerf.default 1130 1161 0.97:1 analysis
GridMinimalPerf.default 994 1036 0.96:1 analysis
HeaderMinimalPerf.default 621 694 0.89:1 analysis
VideoMinimalPerf.default 977 1142 0.86:1 analysis
ReactionMinimalPerf.default 2329 2784 0.84:1 analysis
EmbedMinimalPerf.default 4875 6130 0.8:1 analysis
SplitButtonMinimalPerf.default 10730 13839 0.78:1 analysis
AttachmentSlotsPerf.default 3122 4097 0.76:1 analysis
CarouselMinimalPerf.default 1635 2251 0.73:1 analysis
AttachmentMinimalPerf.default 747 1042 0.72:1 analysis
DividerMinimalPerf.default 872 1212 0.72:1 analysis
DropdownMinimalPerf.default 2846 3951 0.72:1 analysis
InputMinimalPerf.default 865 1207 0.72:1 analysis
Dropdown.Fluent 2791 3905 0.71:1 analysis
TextAreaMinimalPerf.default 1997 3465 0.58:1 analysis
Checkbox.Fluent 426 800 0.53:1 analysis
CheckboxMinimalPerf.default 1766 3682 0.48:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.43 0.55 0.78:1 2000 867
🦄 Button.Fluent 0.12 0.21 0.57:1 5000 613
🎯 Checkbox.Fluent 0.43 0.47 0.91:1 1000 426
🔧 Dialog.Fluent 0.25 0.24 1.04:1 5000 1251
🔧 Dropdown.Fluent 2.79 0.56 4.98:1 1000 2791
🔧 Icon.Fluent 0.21 0.05 4.2:1 5000 1030
🎯 Image.Fluent 0.09 0.12 0.75:1 5000 458
🔧 Slider.Fluent 1.77 0.48 3.69:1 1000 1768
🔧 Text.Fluent 0.09 0.03 3:1 5000 469
🦄 Tooltip.Fluent 0.13 17.62 0.01:1 5000 665

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
ButtonMinimalPerf.default 203 174 1.17:1
StatusMinimalPerf.default 924 808 1.14:1
CardMinimalPerf.default 516 478 1.08:1
ListMinimalPerf.default 580 549 1.06:1
PortalMinimalPerf.default 358 337 1.06:1
RefMinimalPerf.default 221 212 1.04:1
ImageMinimalPerf.default 445 430 1.03:1
TooltipMinimalPerf.default 1014 984 1.03:1
PopupMinimalPerf.default 271 269 1.01:1
Icon.Fluent 1030 1020 1.01:1
Image.Fluent 458 454 1.01:1
LabelMinimalPerf.default 487 487 1:1
Button.Fluent 613 613 1:1
Tooltip.Fluent 665 668 1:1
ChatMinimalPerf.default 732 738 0.99:1
ProviderMergeThemesPerf.default 1550 1561 0.99:1
IconMinimalPerf.default 515 532 0.97:1
Text.Fluent 469 482 0.97:1
FlexMinimalPerf.default 354 372 0.95:1
CustomToolbarPrototype.default 3803 3994 0.95:1
BoxMinimalPerf.default 421 452 0.93:1
TreeWith60ListItems.default 236 255 0.93:1
ChatDuplicateMessagesPerf.default 441 479 0.92:1
RadioGroupMinimalPerf.default 607 677 0.9:1
ButtonSlotsPerf.default 618 711 0.87:1
ChatWithPopoverPerf.default 598 687 0.87:1
MenuButtonMinimalPerf.default 1634 1879 0.87:1
TextMinimalPerf.default 439 510 0.86:1
TreeMinimalPerf.default 1251 1452 0.86:1
HierarchicalTreeMinimalPerf.default 1033 1236 0.84:1
ListCommonPerf.default 1011 1226 0.82:1
ListNestedPerf.default 917 1137 0.81:1
HeaderSlotsPerf.default 1607 2074 0.77:1
AccordionMinimalPerf.default 209 279 0.75:1
ListWith60ListItems.default 1048 1402 0.75:1
ToolbarMinimalPerf.default 1024 1362 0.75:1
Avatar.Fluent 867 1222 0.71:1
AvatarMinimalPerf.default 475 687 0.69:1
TableMinimalPerf.default 523 798 0.66:1
ItemLayoutMinimalPerf.default 1531 2444 0.63:1
DropdownManyItemsPerf.default 1008 1711 0.59:1
FormMinimalPerf.default 649 1109 0.59:1
DialogMinimalPerf.default 1266 2229 0.57:1
AlertMinimalPerf.default 374 676 0.55:1
Dialog.Fluent 1251 2284 0.55:1
MenuMinimalPerf.default 1032 2317 0.45:1
SegmentMinimalPerf.default 478 1346 0.36:1

@dvdzkwsk dvdzkwsk force-pushed the optimize-fela-rule-cache branch 3 times, most recently from b425f45 to 2f1a84e Compare March 25, 2020 04:37
David Zukowski added 3 commits March 26, 2020 14:53
Updates fela's `renderRule` method to compute classNames on a more granular level: by property rather than by style object.

Rather than generating a full className for an entire style object at once, it now walks the style object and generates classes for each individual property.  This allows us to avoid reprocessing styles that should already be cached.

NOTE: this approach does not work for monolithic classNames, since that treats an entire style object as a single entity. So, do not use this if you are using that mode.
@dvdzkwsk dvdzkwsk force-pushed the optimize-fela-rule-cache branch from 43aec76 to 816b7cc Compare March 26, 2020 19:54
@dvdzkwsk
Copy link
Contributor Author

Closing in favor of #12445.

@dvdzkwsk dvdzkwsk closed this Mar 26, 2020
@dvdzkwsk dvdzkwsk deleted the optimize-fela-rule-cache branch March 26, 2020 21:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Fluent UI react-northstar (v0) Work related to Fluent UI V0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants