Pool render data nodes to reduce per-frame GC pressure - #20885
Conversation
|
You can test this PR using the following package version. |
|
|
@cla-avalonia agree |
|
Note that this would mean that the memory used by nodes is never reclaimed. We probably need some opt-in flag for that or employ a strategy similar to one used in batch stream pools where we collect item usage statistics and release unused items on timer tick. This will probably require a specialized pool implementation. ThreadSafeObjectPool is rather simple and is designed for types that won't really have lots of instances. |
|
(not a suggestion to go and rewrite everything) We could also use WPF's approach where it doesn't use proper nodes and instead serializes operations into binary representation: https://github.com/dotnet/wpf/blob/5599cc923d6a464ea0afc7864e722a7b57ab4281/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Media/Generated/RenderDataDrawingContext.cs#L55-L69. So instead of lots of individual drawing operation nodes our render data would consists of a |
Would you like me to use BatchStreamPoolBase instead of ThreadSafeObjectPool? |
I wouldn't mind doing this also, since I use Avalonia now in one of my projects I have personal interest in making this as smooth as possible, the project simulates thousands of things and renders them which is how I ran into this issue also. |
|
I've updated the implementation. After experimenting with |
|
You can test this PR using the following package version. |
|
You can test this PR using the following package version. |
|
You can test this PR using the following package version. |
|
You can test this PR using the following package version. |
| if (_count == 0) | ||
| return; | ||
|
|
||
| var release = Math.Max(1, _count / 3); |
There was a problem hiding this comment.
Should we release a bit more than 1 item at a time when the pool has few items remaining? Releasing 1 item per second for 5 seconds does not sound optimal.
There was a problem hiding this comment.
That is not what this does. It divides the total in 3 and uses a minimum of 1 in case _count is smaller than 3.
There was a problem hiding this comment.
To clarify the above: Math.Max(1, _count / 3) is 1 for values from 1 to 5, so that is 1 item released per second in those cases. At some arbitrary threshold, we should just release everything.
There was a problem hiding this comment.
I see what you mean, but I don't think we need an arbitrary threshold, if we make it the minimum of the dividing number then the list will be empty in 2 cycles at most, does that sound reasonable to you?
| return; | ||
| } | ||
|
|
||
| if (_count == 0) |
There was a problem hiding this comment.
Should we always keep a minimal reasonable number of items?
There was a problem hiding this comment.
The goal is to reduce GC pressure when there is a lot of activity and not maintain a cache, when there is no activity it should fully drain it in my opinion. The problem with such things is, what is the reasonable number? Different applications do different things.
| return _items[--_count]; | ||
| } | ||
|
|
||
| return new T(); |
There was a problem hiding this comment.
Activator.CreateInstance<T> is way faster than in the .NET Framework times, but it's still twice as slow as a standard instantiation. Consider passing a factory instead.
There was a problem hiding this comment.
Honestly, not worth it, this adds complexity for a couple nanoseconds and would only happen if the pool adds new elements, in a hot loop it will typically re-use and not instantiate.
|
You can test this PR using the following package version. |
|
You can test this PR using the following package version. |
|
You can test this PR using the following package version. |
|
#21366 supersedes this. |
What does the pull request do?
Adds object pooling for all
IRenderDataItemnode types created byRenderDataDrawingContext. Instead of allocating new node objects every frame, nodes are returned to aRenderDataNodePool<T>after the server-side render data is consumed and reused on subsequent frames. Pools automatically reclaim unused items during idle periods via a single shared cleanup timer.Related to #19363
What is the current behavior?
Every draw call (
DrawRectangle,DrawLine,DrawGlyphRun,PushClip,PushTransform, etc.) allocates a newIRenderDataItemnode vianew. These nodes are created on the UI thread, serialized to the render thread, consumed once, then abandoned to the GC. In high-frequency rendering scenarios (e.g. 26K+ rectangles per frame), this creates significant GC pressure and frame stutter.What is the updated/expected behavior with this PR?
After the first frame, all render data nodes are served from per-type object pools. Nodes are returned to pools when
ServerCompositionRenderData.Reset()orCompositionRenderData.Dispose()runs. Per-frame allocations of render data nodes drop to near zero. When rendering activity stops, pooled items are gradually released (~1/3 per second) so memory is reclaimed during idle periods.How was the solution implemented (if it's not obvious)?
IPoolableRenderDataIteminterface with aReturnToPool()methodRenderDataItemPoolHelper.DisposeAndReturnToPool()which recurses into push node children, then either returns poolable items to their pool or disposes non-poolable itemsRenderDataNodePool<T>— a lightweight array-backed object pool with idle-based reclamation. All pool instances register withRenderDataNodePoolCleanupwhich runs a single sharedSystem.Threading.Timer. During active use the pool retains all items for reuse; once idle,Reduce()gradually releases a third of excess items per cycle. Pools are tracked via weak references so they can be garbage collected if no longer referenced.RenderDataNodePool<T>, aGet()factory method, and aReturnToPool()that resets state and returns to pool. For nodes with disposable resources (GlyphRun,Bitmap,CustomOperation),ReturnToPool()callsDispose()before returningRenderDataDrawingContextusesNodeType.Get()instead ofnew NodeTypePooled node types:
RenderDataRectangleNode,RenderDataEllipseNode,RenderDataLineNode,RenderDataGeometryNode,RenderDataGlyphRunNode,RenderDataBitmapNode,RenderDataCustomNode,RenderDataClipNode,RenderDataPushMatrixNode,RenderDataOpacityNode,RenderDataOpacityMaskNode,RenderDataGeometryClipNode,RenderDataRenderOptionsNode,RenderDataTextOptionsNodeBreaking changes
None. Internal classes only, no public API changes.
Obsoletions / Deprecations
None.
Fixed issues
Addresses some points of #19363 in regards to rendering.