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

Remove virtual generic methods from ValueStore. #7980

Merged
merged 7 commits into from
Apr 20, 2022
Merged

Remove virtual generic methods from ValueStore. #7980

merged 7 commits into from
Apr 20, 2022

Conversation

grokys
Copy link
Member

@grokys grokys commented Apr 14, 2022 •
edited
Loading

What does the pull request do?

Because we've now removed virtual generics from #7969 and #7979, we can make now remove all generic virtual methods from our private ValueStore implementation and associated classes.

This finishes the first batch of changes relating to the use of virtual generic methods in property storage.

Benchmarks

Comparing the PR with master:

Faster base/diff Base Median (ns) Diff Median (ns) Modality
Avalonia.Benchmarks.Base.AvaloniaObject_GetValueInherited.GetInheritedValues(Dep 3.54 1118526.17 315742.92
Avalonia.Benchmarks.Base.AvaloniaObject_GetValueInherited.GetInheritedValues(Dep 3.42 569585.30 166525.42 several?
Avalonia.Benchmarks.Base.AvaloniaObject_GetValueInherited.GetInheritedValues(Dep 3.03 305479.79 100816.45
Avalonia.Benchmarks.Base.AvaloniaObject_GetValueInherited.GetInheritedValues(Dep 2.03 88732.81 43763.00
Avalonia.Benchmarks.Base.DirectPropertyBenchmark.SetAndRaiseSimple 1.73 5414.40 3126.07
Avalonia.Benchmarks.Base.DirectPropertyBenchmark.SetAndRaiseOriginal 1.64 4978.60 3030.24
Avalonia.Benchmarks.Base.StyledPropertyBenchmarks.Bind_Int_Property_LocalValue 1.48 9449.12 6369.65
Avalonia.Benchmarks.Base.AvaloniaObjectBenchmark.BindIntProperty 1.44 17882.59 12387.81
Avalonia.Benchmarks.Base.StyledPropertyBenchmarks.Bind_Int_Property_Multiple_Pri 1.44 64824.01 45122.53
Avalonia.Benchmarks.Base.AvaloniaObjectBenchmark.ClearAndSetIntProperty 1.36 322.27 236.14
Avalonia.Benchmarks.Base.AvaloniaObject_Binding.Fire_LocalValue_Bindings 1.36 145961.40 107162.02
Avalonia.Benchmarks.Base.AvaloniaObject_SetValue.SetValues 1.30 173551.20 133515.77
Avalonia.Benchmarks.Base.StyledPropertyBenchmarks.Set_Validated_Int_Property_Loc 1.28 16382.64 12793.98
Avalonia.Benchmarks.Base.AvaloniaObject_SetValue.Set_Local_Values_With_Style_Val 1.25 259311.18 207091.31
Avalonia.Benchmarks.Base.AvaloniaObject_Construct.Construct_And_Set_Values 1.24 2233.89 1794.94
Avalonia.Benchmarks.Base.StyledPropertyBenchmarks.Set_Int_Property_LocalValue 1.24 16185.05 13085.51
Avalonia.Benchmarks.Base.StyledPropertyBenchmarks.Set_Coerced_Int_Property_Local 1.23 19717.65 15989.32
Avalonia.Benchmarks.Base.AvaloniaObject_Binding.Fire_LocalValue_Bindings_With_St 1.22 251702.05 205522.68
Avalonia.Benchmarks.Styling.Style_NonActive.Toggle_NonActive_Style_Activation 1.22 60623.71 49605.51
Avalonia.Benchmarks.Styling.Style_Activation.Toggle_Style_Activation_Via_Class 1.21 68165.42 56235.29
Avalonia.Benchmarks.Base.AvaloniaObject_GetValueInherited.GetInheritedValues(Dep 1.21 46735.96 38705.37
Avalonia.Benchmarks.Base.AvaloniaObject_GetObservable.PropertyChangedSubscriptio 1.21 215904.52 178851.54
Avalonia.Benchmarks.Themes.FluentBenchmark.RepeatButton 1.18 360388.53 305121.14
Avalonia.Benchmarks.Layout.ControlsBenchmark.CreateCalendar 1.17 25806300.00 22021000.00 several?
Avalonia.Benchmarks.Data.BindingsBenchmark.UpdateTwoWayBinding_Via_Binding 1.16 135675.56 116789.24
Avalonia.Benchmarks.Layout.Measure.Remeasure 1.16 9131111.72 7885725.78
Avalonia.Benchmarks.Base.AvaloniaObject_GetValueInherited.GetInheritedValues(Dep 1.15 43954.98 38251.68
Avalonia.Benchmarks.Layout.ControlsBenchmark.CreateButton 1.14 218367.19 191501.81
Avalonia.Benchmarks.Styling.StyleAttachBenchmark.AttachTextBoxStyles 1.14 28622.00 25115.74
Avalonia.Benchmarks.Base.AvaloniaObject_GetObservable.GetObservables 1.14 460883.40 404677.29
Avalonia.Benchmarks.Base.StyledPropertyBenchmarks.Set_Int_Property_TemplatedPare 1.12 38780.52 34665.55
Avalonia.Benchmarks.Traversal.VisualTreeTraversal.FindAncestorOfType_Linq 1.11 609951.27 547943.70
Avalonia.Benchmarks.Themes.ThemeBenchmark.InitFluentTheme(themeUri: "avares://Av 1.11 24836243.75 22436526.56
Avalonia.Benchmarks.Styling.Style_Apply.Apply_Simple_Styles(MatchingStyles: 1, N 1.11 2502.46 2261.51
Avalonia.Benchmarks.Styling.SelectorBenchmark.ClassSelector_Match 1.10 34.74 31.51
Avalonia.Benchmarks.Styling.SelectorBenchmark.ClassSelector_NoMatch 1.10 34.42 31.41
Avalonia.Benchmarks.Styling.Style_Apply.Apply_Simple_Styles(MatchingStyles: 1, N 1.09 953.13 872.35
Avalonia.Benchmarks.Base.StyledPropertyBenchmarks.Set_Int_Property_Multiple_Prio 1.09 249623.97 228651.31
Avalonia.Benchmarks.Themes.ThemeBenchmark.InitFluentTheme(themeUri: "avares://Av 1.09 24651918.75 22607860.94
Avalonia.Benchmarks.Base.AvaloniaObjectInitializationBenchmark.InitializeButton 1.08 209.87 194.28
Avalonia.Benchmarks.Styling.Style_Apply.Apply_Simple_Styles(MatchingStyles: 1, N 1.08 1115.10 1033.45
Avalonia.Benchmarks.Visuals.Media.PathMarkupParserTests.Parse_Large_Path 1.08 13393.64 12455.11
Avalonia.Benchmarks.Data.BindingsBenchmark.TwoWayBinding_Via_Binding 1.07 4373.57 4076.48
Avalonia.Benchmarks.Rendering.ShapeRendering.Render_Line_NoBrushes 1.07 47.67 44.53
Avalonia.Benchmarks.Data.PropertyAccessorPluginBenchmarks.MatchAccessorOld 1.06 136.06 127.93
Avalonia.Benchmarks.Base.AvaloniaObject_GetValue.GetClrPropertyValues 1.06 433.89 408.77
Slower diff/base Base Median (ns) Diff Median (ns) Modality
Avalonia.Benchmarks.Layout.ControlsBenchmark.CreateTextBox 1.18 2360600.00 2782200.00 bimodal
Avalonia.Benchmarks.Data.PropertyAccessorBenchmarks.InpcAccessorStart 1.08 79.99 86.31

Breaking changes

No additional breaking changes (though breaking changes from #7969 and #7979) are included here.

Notes

Depends on #7969 and #7979; the diff for this PR includes changes from that PR (I wish GitHub had support for dependent PRs)

@grokys grokys mentioned this pull request Apr 14, 2022
Closed
@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 0.10.999-cibuild0019909-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

@grokys grokys mentioned this pull request Apr 14, 2022
Closed
@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 0.10.999-cibuild0019922-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

MarchingCube
MarchingCube approved these changes Apr 15, 2022
View reviewed changes
Copy link
Collaborator

@MarchingCube MarchingCube left a comment

Choose a reason for hiding this comment

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

Perf numbers look good, TextBox case needs to be checked later though.

src/Avalonia.Base/Animation/Animatable.cs
var newTransitions = change.NewValue.GetValueOrDefault<Transitions>();
var e = (AvaloniaPropertyChangedEventArgs<Transitions?>)change;
var oldTransitions = e.OldValue.GetValueOrDefault();
var newTransitions = e.NewValue.GetValueOrDefault();
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can use that new GetOldAndNewValue helper right?

@hez2010
Copy link
Contributor

hez2010 commented Apr 17, 2022

Well done! This PR also managed to shrink the NativeAOTed CC.NetCore from 130615KB down to 124605KB (~6mb saved).

@MarchingCube MarchingCube merged commit 8bd9cb1 into master Apr 20, 2022
@MarchingCube MarchingCube deleted the refactor/valuestore-nongeneric branch April 20, 2022 13:25
@avaloniaui-team
Copy link
Contributor

You can test this PR using the following package version. 0.10.999-cibuild0020060-beta. (feed url: https://nuget.avaloniaui.net/repository/avalonia-all/index.json) [PRBUILDID]

@grokys
Copy link
Member Author

grokys commented Apr 20, 2022

Great to hear @hez2010 - there are more changes along these lines still to come!

This was referenced Apr 20, 2022
Merged
Merged
@melMass
Copy link

melMass commented May 17, 2022

Hey trying to follow all this related PR's, is this in 0.10.14?

image

@maxkatz6
Copy link
Member

No, that's nightly builds of 11.0.

We don't have that big breaking changes in stable releases.

@maxkatz6 maxkatz6 mentioned this pull request Sep 26, 2022
Closed
DamianSuess added a commit to DamianSuess/AvaloniaILSpy that referenced this pull request Jul 21, 2023
@DamianSuess
Avalonia v11 - Styles.TryGetResource(..) now use ThemeVariant. `O…
3480227 
…nPropertyChanged` deprecated virtual generic `..Changed<t>()` as per (AvaloniaUI/Avalonia#7980)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kekekeks kekekeks kekekeks approved these changes

@MarchingCube MarchingCube MarchingCube approved these changes

@maxkatz6 maxkatz6 Awaiting requested review from maxkatz6

Assignees
No one assigned
Labels
breaking-change enhancement
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@grokys @avaloniaui-team @hez2010 @melMass @maxkatz6 @kekekeks @MarchingCube