Using ArrayPool in RenderData by lindexi · Pull Request #5392 · dotnet/wpf

lindexi · 2021-09-26T00:51:29Z

Description

Using ArrayPool in RenderData. The max length is 72, see MILCMD_DRAW_ROUNDED_RECTANGLE_ANIMATE

wpf/src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Media/Generated/RenderData.cs

Lines 149 to 184 in 89d172d

    
           [StructLayout(LayoutKind.Explicit)] 
        
           internal struct MILCMD_DRAW_ROUNDED_RECTANGLE_ANIMATE 
        
           { 
        
              public MILCMD_DRAW_ROUNDED_RECTANGLE_ANIMATE ( 
        
                  UInt32 hBrush, 
        
                  UInt32 hPen, 
        
                  Rect rectangle, 
        
                  UInt32 hRectangleAnimations, 
        
                  double radiusX, 
        
                  UInt32 hRadiusXAnimations, 
        
                  double radiusY, 
        
                  UInt32 hRadiusYAnimations 
        
                  ) 
        
              { 
        
                  this.hBrush = hBrush; 
        
                  this.hPen = hPen; 
        
                  this.rectangle = rectangle; 
        
                  this.hRectangleAnimations = hRectangleAnimations; 
        
                  this.radiusX = radiusX; 
        
                  this.hRadiusXAnimations = hRadiusXAnimations; 
        
                  this.radiusY = radiusY; 
        
                  this.hRadiusYAnimations = hRadiusYAnimations; 
        
                  this.QuadWordPad0 = 0; 
        
              } 
        
              [FieldOffset(0)] public Rect rectangle; 
        
              [FieldOffset(32)] public double radiusX; 
        
              [FieldOffset(40)] public double radiusY; 
        
              [FieldOffset(48)] public UInt32 hBrush; 
        
              [FieldOffset(52)] public UInt32 hPen; 
        
              [FieldOffset(56)] public UInt32 hRectangleAnimations; 
        
              [FieldOffset(60)] public UInt32 hRadiusXAnimations; 
        
              [FieldOffset(64)] public UInt32 hRadiusYAnimations; 
        
              [FieldOffset(68)] private UInt32 QuadWordPad0; 
        
           }

Customer Impact

None. But it will affect the rendering of all DrawingVisual and UIElement

Regression

dotnet 7

Testing

Just CI and my demo

Risk

Low

The max length is 72, see MILCMD_DRAW_ROUNDED_RECTANGLE_ANIMATE

dotnet/wpf#5392

gfoidl · 2021-09-28T11:59:42Z

src/Microsoft.DotNet.Wpf/src/PresentationCore/System/Windows/Media/RenderData.cs


+        ~RenderData()
+        {
+            ArrayPool<byte>.Shared.Return(_buffer);


Can this just be "disposed" normally? Using a finalizer is quite heavy and puts pressure on the GC's finalizer queue. Especially with

The max length is 72

in mind, maybe it's just cheaper to allocate and let the GC clean up.
Or as there is unsafe code already involved use a struct with fixed sized buffer of that size, and wrap by a span for access outside that class?

@gfoidl Thank you and I update the PR

kronic · 2021-09-29T11:32:29Z

@lindexi maybe you should delete the field altogether _buffer

lindexi · 2021-09-29T11:50:36Z

@kronic Thank you. But I find it hard to delete the _buffer field

omariom · 2022-01-03T11:06:38Z

@dipeshmsft
What does the team use for perf benchmarks?

dipeshmsft · 2022-01-03T11:18:15Z

@omariom We mainly use sample applications to review the performance benefits and we rely on our regression suite to check any performance regressions.

lindexi · 2022-01-05T00:36:53Z

@dipeshmsft Thank you.

omariom · 2022-01-16T21:53:35Z

@dipeshmsft
Can their source be opened so we could use them in our PRs?

dipeshmsft · 2022-01-17T16:15:24Z

@omariom , we are working on open-sourcing the complete test-suite. You can check the repo dotnet/wpf-test , but the task is still in progress. @gurpreet-wpf

lindexi · 2022-01-18T00:42:44Z

@dipeshmsft Can I publish my private WPF framework version?

dipeshmsft · 2022-01-19T17:25:50Z

@vishalmsft @gurpreet-wpf Could you take a look at this ??

vishalmsft · 2022-01-20T06:39:37Z

@lindexi do you mean tests written and available in your private repo?

lindexi · 2022-01-20T06:44:52Z

@vishalmsft I mean that can I publish my nuget package which include my private WPF framework version? Some of my friends can't wait for the official WPF release. I expect to provide a private version for them to use. Thank you.

lindexi · 2022-01-20T06:47:19Z

And this version wpf framework may used in the business application.

Using ArrayPool in RenderData

d664200

The max length is 72, see MILCMD_DRAW_ROUNDED_RECTANGLE_ANIMATE

lindexi requested a review from a team as a code owner September 26, 2021 00:51

ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Sep 26, 2021

ghost requested review from SamBent, fabiant3 and ryalanms September 26, 2021 00:51

lindexi added a commit to lindexi/lindexi_gd that referenced this pull request Sep 26, 2021

Test using ArrayPool in RenderData

d40a0dd

dotnet/wpf#5392

gfoidl reviewed Sep 28, 2021

View reviewed changes

Remove the finalizer code in RenderData

60fd847

vishalmsft added the Queue for test pass label Dec 2, 2021

dipeshmsft merged commit b3fee31 into dotnet:main Jan 3, 2022

lindexi deleted the t/lindexi/RenderDataArrayPool branch January 5, 2022 00:36

dipeshmsft removed the Queue for test pass label Jan 19, 2022

lindexi mentioned this pull request Mar 11, 2022

Improve allocation with System.Buffers #769

Open

ghost locked as resolved and limited conversation to collaborators Apr 6, 2022

	[StructLayout(LayoutKind.Explicit)]
	internal struct MILCMD_DRAW_ROUNDED_RECTANGLE_ANIMATE
	{
	public MILCMD_DRAW_ROUNDED_RECTANGLE_ANIMATE (
	UInt32 hBrush,
	UInt32 hPen,
	Rect rectangle,
	UInt32 hRectangleAnimations,
	double radiusX,
	UInt32 hRadiusXAnimations,
	double radiusY,
	UInt32 hRadiusYAnimations
	)
	{
	this.hBrush = hBrush;
	this.hPen = hPen;
	this.rectangle = rectangle;
	this.hRectangleAnimations = hRectangleAnimations;
	this.radiusX = radiusX;
	this.hRadiusXAnimations = hRadiusXAnimations;
	this.radiusY = radiusY;
	this.hRadiusYAnimations = hRadiusYAnimations;
	this.QuadWordPad0 = 0;
	}

	[FieldOffset(0)] public Rect rectangle;
	[FieldOffset(32)] public double radiusX;
	[FieldOffset(40)] public double radiusY;
	[FieldOffset(48)] public UInt32 hBrush;
	[FieldOffset(52)] public UInt32 hPen;
	[FieldOffset(56)] public UInt32 hRectangleAnimations;
	[FieldOffset(60)] public UInt32 hRadiusXAnimations;
	[FieldOffset(64)] public UInt32 hRadiusYAnimations;
	[FieldOffset(68)] private UInt32 QuadWordPad0;
	}

Conversation

lindexi commented Sep 26, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Customer Impact

Regression

Testing

Risk

Uh oh!

gfoidl Sep 28, 2021

Choose a reason for hiding this comment

Uh oh!

lindexi Sep 29, 2021

Choose a reason for hiding this comment

Uh oh!

kronic commented Sep 29, 2021

Uh oh!

lindexi commented Sep 29, 2021

Uh oh!

omariom commented Jan 3, 2022

Uh oh!

dipeshmsft commented Jan 3, 2022

Uh oh!

lindexi commented Jan 5, 2022

Uh oh!

omariom commented Jan 16, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dipeshmsft commented Jan 17, 2022

Uh oh!

lindexi commented Jan 18, 2022

Uh oh!

dipeshmsft commented Jan 19, 2022

Uh oh!

vishalmsft commented Jan 20, 2022

Uh oh!

lindexi commented Jan 20, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lindexi commented Jan 20, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

lindexi commented Sep 26, 2021 •

edited

Loading

omariom commented Jan 16, 2022 •

edited

Loading

lindexi commented Jan 20, 2022 •

edited

Loading