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

[WIP] Fela optimizations #12445

Closed
wants to merge 2 commits into from
Closed

[WIP] Fela optimizations #12445

wants to merge 2 commits into from

Conversation

dvdzkwsk
Copy link
Contributor

@dvdzkwsk dvdzkwsk commented Mar 26, 2020 •
edited by msft-github-bot
Loading

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

Microsoft Reviewers: Open in CodeFlow

@dvdzkwsk dvdzkwsk added the Fluent UI react-northstar (v0) Work related to Fluent UI V0 label Mar 26, 2020
@dvdzkwsk dvdzkwsk mentioned this pull request Mar 26, 2020
Closed
2 tasks
@DustyTheBot
Copy link

DustyTheBot commented Mar 26, 2020 •
edited
Loading

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

Generated by 🚫 dangerJS against d075b33

@msft-github-bot
Copy link
Contributor

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

Perf Analysis

No significant results to display.

All results

Scenario Master Ticks PR Ticks Status
BaseButton 831 830
BaseButton (experiments) 1035 990
DefaultButton 1145 1110
DefaultButton (experiments) 1963 1910
DetailsRow 3496 3419
DetailsRow (fast icons) 3439 3479
DetailsRow without styles 3249 3242
DocumentCardTitle with truncation 1565 1543
MenuButton 1452 1433
MenuButton (experiments) 3486 3573
PrimaryButton 1222 1251
PrimaryButton (experiments) 1954 2001
SplitButton 3049 3107
SplitButton (experiments) 7039 6968
Stack 499 478
Stack with Intrinsic children 1106 1106
Stack with Text children 4287 4307
Text 393 425
Toggle 952 924
Toggle (experiments) 2315 2394
button 77 73

Perf Analysis (Fluent)

⚠️ 17 potential perf regressions detected

Potential regressions comparing to master

Scenario Current PR Ticks Baseline Ticks Ratio Regression Analysis
SliderMinimalPerf.default 1919 1593 1.2:1 analysis
Slider.Fluent 1913 1637 1.17:1 analysis
GridMinimalPerf.default 979 926 1.06:1 analysis
ProviderMinimalPerf.default 697 658 1.06:1 analysis
AnimationMinimalPerf.default 750 736 1.02:1 analysis
AttachmentMinimalPerf.default 1007 992 1.02:1 analysis
HeaderMinimalPerf.default 605 608 1:1 analysis
ReactionMinimalPerf.default 2561 2608 0.98:1 analysis
EmbedMinimalPerf.default 5487 5689 0.96:1 analysis
LayoutMinimalPerf.default 688 723 0.95:1 analysis
LoaderMinimalPerf.default 1065 1118 0.95:1 analysis
Dropdown.Fluent 3321 3747 0.89:1 analysis
InputMinimalPerf.default 966 1112 0.87:1 analysis
VideoMinimalPerf.default 865 996 0.87:1 analysis
TextAreaMinimalPerf.default 2686 3299 0.81:1 analysis
DividerMinimalPerf.default 879 1202 0.73:1 analysis
SegmentMinimalPerf.default 738 1232 0.6:1 analysis
Perf comparison
Status Scenario Fluent TPI Fabric TPI Ratio Iterations Ticks
🎯 Avatar.Fluent 0.43 0.51 0.84:1 2000 869
🦄 Button.Fluent 0.12 0.2 0.6:1 5000 582
🔧 Checkbox.Fluent 0.6 0.42 1.43:1 1000 595
🔧 Dialog.Fluent 0.3 0.2 1.5:1 5000 1508
🔧 Dropdown.Fluent 3.32 0.51 6.51:1 1000 3321
🔧 Icon.Fluent 0.19 0.05 3.8:1 5000 950
🎯 Image.Fluent 0.09 0.11 0.82:1 5000 460
🔧 Slider.Fluent 1.91 0.43 4.44:1 1000 1913
🔧 Text.Fluent 0.09 0.02 4.5:1 5000 451
🦄 Tooltip.Fluent 0.13 19.04 0.01:1 5000 643

🔧 Needs work     🎯 On target     🦄 Amazing

Perf tests with no regressions
Scenario Current PR Ticks Baseline Ticks Ratio
Button.Fluent 582 517 1.13:1
Image.Fluent 460 410 1.12:1
ChatMinimalPerf.default 706 648 1.09:1
IconMinimalPerf.default 514 483 1.06:1
ListMinimalPerf.default 576 543 1.06:1
CustomToolbarPrototype.default 3948 3811 1.04:1
ButtonMinimalPerf.default 174 169 1.03:1
LabelMinimalPerf.default 476 461 1.03:1
ImageMinimalPerf.default 422 415 1.02:1
PopupMinimalPerf.default 268 262 1.02:1
StatusMinimalPerf.default 792 774 1.02:1
TooltipMinimalPerf.default 931 914 1.02:1
PortalMinimalPerf.default 326 325 1:1
CardMinimalPerf.default 449 453 0.99:1
FlexMinimalPerf.default 327 330 0.99:1
ProviderMergeThemesPerf.default 1468 1485 0.99:1
RefMinimalPerf.default 205 207 0.99:1
TextMinimalPerf.default 453 457 0.99:1
Icon.Fluent 950 968 0.98:1
Text.Fluent 451 467 0.97:1
RadioGroupMinimalPerf.default 606 633 0.96:1
Tooltip.Fluent 643 672 0.96:1
BoxMinimalPerf.default 405 428 0.95:1
ChatWithPopoverPerf.default 597 645 0.93:1
AttachmentSlotsPerf.default 3533 3834 0.92:1
TreeWith60ListItems.default 223 242 0.92:1
ButtonSlotsPerf.default 594 651 0.91:1
MenuButtonMinimalPerf.default 1544 1701 0.91:1
SplitButtonMinimalPerf.default 3487 3870 0.9:1
DropdownMinimalPerf.default 3344 3751 0.89:1
TreeMinimalPerf.default 1176 1340 0.88:1
ChatDuplicateMessagesPerf.default 398 456 0.87:1
ListCommonPerf.default 1005 1168 0.86:1
HeaderSlotsPerf.default 1570 1869 0.84:1
HierarchicalTreeMinimalPerf.default 979 1169 0.84:1
ListNestedPerf.default 871 1066 0.82:1
CarouselMinimalPerf.default 561 695 0.81:1
ToolbarMinimalPerf.default 1014 1249 0.81:1
AvatarMinimalPerf.default 478 611 0.78:1
ListWith60ListItems.default 1037 1334 0.78:1
Checkbox.Fluent 595 771 0.77:1
DropdownManyItemsPerf.default 1175 1560 0.75:1
MenuMinimalPerf.default 1632 2188 0.75:1
CheckboxMinimalPerf.default 2521 3394 0.74:1
AccordionMinimalPerf.default 196 269 0.73:1
DialogMinimalPerf.default 1524 2098 0.73:1
Avatar.Fluent 869 1190 0.73:1
AlertMinimalPerf.default 464 658 0.71:1
Dialog.Fluent 1508 2126 0.71:1
ItemLayoutMinimalPerf.default 1555 2300 0.68:1
TableMinimalPerf.default 521 784 0.66:1
FormMinimalPerf.default 652 1009 0.65:1

@size-auditor
Copy link

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

Asset size changes

Size Auditor did not detect a change in bundle size for any component!

Baseline commit: aa90e07c6f8e27a28a3a883fb4febc2cd2eb8ae2 (build)

David Zukowski added 2 commits March 27, 2020 00:46
Enable faster rule caching for fela renderer
1b534ea 
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.
DEBUG: expand shorthands before checking style cache
d075b33
@layershifter
Copy link
Member

Hey @davezuko, can you please open a new PR and include these changes and #12289. What impact on performance will be?

@dvdzkwsk dvdzkwsk mentioned this pull request Mar 30, 2020
Closed
@dvdzkwsk
Copy link
Contributor Author

@layershifter I see you've already done some benchmarks of your own. For clarity, I'll cherry-pick your changes into my branch to compare here as well.

@ecraig12345
Copy link
Member

@davezuko Are you still working on this or can we close it? (Or close and then reopen when it's ready?)

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

@dzearing dzearing Awaiting requested review from dzearing

@jurokapsiar jurokapsiar Awaiting requested review from jurokapsiar

@layershifter layershifter Awaiting requested review from layershifter

@levithomason levithomason Awaiting requested review from levithomason

@miroslavstastny miroslavstastny Awaiting requested review from miroslavstastny

@mnajdova mnajdova Awaiting requested review from mnajdova

@pompomon pompomon Awaiting requested review from pompomon

@silviuaavram silviuaavram Awaiting requested review from silviuaavram

Assignees

@aneeshack4 aneeshack4

Labels
Fluent UI react-northstar (v0) Work related to Fluent UI V0 Needs: Author Feedback Status: No Recent Activity
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@dvdzkwsk @DustyTheBot @msft-github-bot @layershifter @ecraig12345 @aneeshack4