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

Remove List<IObserver<T>>.ToArray() allocations in LightweightObservableBase #18316

Merged
merged 4 commits into from
Feb 26, 2025

Conversation

xoofx
Copy link
Contributor

@xoofx xoofx commented Feb 25, 2025

Hello folks, ☺️

What does the pull request do?

This PR is removing the List<IObserver<T>>.ToArray() allocations happening in LightweightObservableBase when Routing events are fired (e.g. whenever you move the mouse)

When profiling the memory, I noticed that when generating lots of routing events (e.g. just moving the mouse over a window) several MB of IObserver<ValueTuple<Object, RoutedEventArgs>>[] were created.

See for example here with the ControlDialog application by just moving the mouse on the Window for a few seconds:

image

What is the current behavior?

I noticed 2 problems:

  1. The case of array 0 was not covered and always allocating an empty array (at minimum 32 bytes per alloc on x64)
  2. Several other smaller allocs could happen for size 2/3
  3. Depending on the number of controls, the size of the allocation can grow quickly (several controls having to be notified)

For example, here are some statistics about these array allocations:

Array size to count:

  • Array size: 0 bytes, Count: 31926
  • Array size: 1 bytes, Count: 29254
  • Array size: 2 bytes, Count: 278919
  • Array size: 3 bytes, Count: 5567
  • Array size: 4 bytes, Count: 2141

Total array count: 347807
Total array size in bytes: 13246224 bytes

What is the updated/expected behavior with this PR?

This PR is introducing the case for 0/2/3 elements and pooling the array for other cases with ArrayPool.

After this PR, the allocation is removed:

image

A direct side effect is less GC collections:

Before this PR, a trigger of the GC happens after 4.6s

image

After this PR, a trigger of the GC happens after 7s.

image

How was the solution implemented (if it's not obvious)?

Cases of 0/2/3 and ArrayPool.

I have left the code that can print some statistics commented. If it is a problem, I can remove it from the PR.

Checklist

Breaking changes

None

Obsoletions / Deprecations

None

Fixed issues

None

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.3.999-cibuild0055151-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.3.999-cibuild0055153-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

Copy link
Member

@MrJul MrJul left a comment

Choose a reason for hiding this comment

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

LGTM, thank you! Reducing allocations is always nice :)

@MrJul MrJul enabled auto-merge February 26, 2025 09:55
@avaloniaui-bot
Copy link

You can test this PR using the following package version. 11.3.999-cibuild0055161-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

@grokys
Copy link
Member

grokys commented Feb 26, 2025

Thanks for this! This is one of those small things that I intended to fix every time I stepped through this code but never got round to!

@MrJul MrJul added this pull request to the merge queue Feb 26, 2025
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Feb 26, 2025
@MrJul MrJul added this pull request to the merge queue Feb 26, 2025
Merged via the queue into AvaloniaUI:master with commit 85ffed0 Feb 26, 2025
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants