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

Don't use rx for ExpressionNodes. #1694

Merged
merged 6 commits into from Jun 29, 2018

Conversation

Projects
None yet
2 participants
@grokys
Member

grokys commented Jun 25, 2018

Following on from #1690, another PR to reduce memory allocations. This makes ExpressionNodes no longer ISubject<>s and instead transitions them to using an explicit Subscribe/Unsubscribe model.

ExpressionNodes were always single-subscriber and making them use IObservable<> meant that we had to have extra allocations in order to return IDisposables.

This saves a bunch more memory.

Memory Usage

Memory usage was measured via the VS2017 diagnostic tools by running ControlCatalog in Release mode, opening all pages and then taking a memory snapshot:

On master (after #1690):

Objects Heap Size (KB)
1,478,158 79,936

This PR:

Objects Heap Size (KB)
1,381,936 76,310
Don't use rx for ExpressionNodes.
`ExpressionNode`s were always single-subscriber and making them use `IObservable<>` meant that we had to have extra allocations in order to return `IDisposable`s. Instead of using `IObservable` use a simpler `Subscribe`/`Unsubscribe` pattern. This saves a bunch more memory.

@grokys grokys requested review from jkoritzinsky and AvaloniaUI/core Jun 25, 2018

@grokys grokys referenced this pull request Jun 25, 2018

Merged

Simplify TemplateBindings #1695

Throw if no matching property accessor found.
This shouldn't happen normally as `InpcPropertyAcessorPlugin` matches everything.
@wieslawsoltes

This comment has been minimized.

Contributor

wieslawsoltes commented Jun 25, 2018

LGTM
Tested with Core2D, nothing breaks.

@wieslawsoltes

This comment has been minimized.

Contributor

wieslawsoltes commented Jun 25, 2018

Before:
build5521
After:
build5528

@grokys grokys referenced this pull request Jun 26, 2018

Merged

Allow LINQ Expressions for Binding Expressions #1667

2 of 3 tasks complete

grokys added some commits Jun 26, 2018

@grokys grokys merged commit de9644f into master Jun 29, 2018

3 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@grokys grokys deleted the nonrx-expressionnode branch Jun 29, 2018

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