-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Remove AvaloniaProperty.Initialized and PseudoClass static methods #3292
Conversation
It was incorrect: invoking the method doesn't create 1000 buttons.
It was only needed for pseudoclasses. Move pseudoclass registration to an engine that doesn't require `AvaloniaProperty.Initialized`, implemented in `PseudoclassEngine`.
It makes creating new controls a _lot_ faster.
Never needed |
I think that the benefit of dropping |
@grokys So what is the verdict here? Should we merge this in? |
@grokys Since other PRs got merged are we planning to resume working on this? |
Yeah I think this is worth doing, despite the breakage it will cause. |
Ok, updated this to target master. Now ready for review. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Generally LGTM, but one issue that I've found must be fixed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
What does the pull request do?
I'd like to get some thoughts on this, it's removing functionality in the pursuit of performance.
Every time an
AvaloniaObject
is constructed we fire anAvaloniaProperty.Initialized
observable for each property registered on the object. This observable is used to automatically apply pseudoclasses to controls using the staticPseudoClass()
method calls. We've tried to optimize this process, but the performance hit is still quite large.In this PR, I removed the
AvaloniaProperty.Initialized
observable and related machinery. I then tried to implement thePseudoClass
functionality in what I hoped would be a more performant way - lets call this attemptPseudoClassEngine
, you can see the code here. This did indeed improve performance a little, but there was still an overhead.I then tried completely removing the static
PseudoClass
helper methods and managing pseudoclasses in code. Performance was much better. Here are some benchmarks:master
PseudoclassEngine
Pseudoclasses in code
As you can see, creating an instance of a
Button
is now 10x faster.The question is: are we willing to remove functionality like this to get better performance?
Breaking changes
AvaloniaProperty.Initialized
removedPseudoClass
methods removedNote
Targets #3287