Skip to content

Conversation

@stephentoub
Copy link
Member

Description

There are a bunch of copies around the tree of POINT interop types: some are structs (as they should be), some are classes. The latter are resulting in tons of allocations. This standardizes on using structs for all of them.

In a simple WPF app just moving my mouse around for a few seconds, these are the resulting top allocations:
image
With this PR, the highlighted line goes away.

Customer Impact

Less allocation, less GC, less pause time.

Regression

No

Testing

CI, plus validation that the allocations went away in the example scenarios I was looking at.

Risk

There is some risk here. The change itself is sound. However, the interop definitions throughout the repo are in a bit of disarray, with many different copies of the same interop types and functions, all defined subtly differently. I believe I caught all of the relevant paths interacting with these types, but it's definitely possible something could have slipped through.

There are a bunch of copies around the tree of POINT interop types: some are structs (as they should be), some are classes.  The latter are resulting in tons of allocations.  This standardizes on using structs for all of them.
@stephentoub stephentoub requested a review from a team as a code owner June 25, 2021 20:01
@ghost ghost added the PR metadata: Label to tag PRs, to facilitate with triage label Jun 25, 2021
@ghost ghost requested review from SamBent, fabiant3 and ryalanms June 25, 2021 20:01
@lindexi
Copy link
Member

lindexi commented Jun 26, 2021

Can we use the same Point struct type in WPF?

@stephentoub
Copy link
Member Author

Can we use the same Point struct type in WPF?

Do you mean use it for the interop? No, it has a different layout/size, with double rather than int fields.

@lindexi
Copy link
Member

lindexi commented Jun 26, 2021

@stephentoub

Such as:

public struct POINT
{
/// <summary>
///
/// </summary>
public int x;
/// <summary>
///
/// </summary>
public int y;
public POINT(int x, int y)
{
this.x = x;
this.y = y;
}
}

private struct POINT
{
public int x;
public int y;
public POINT(int x, int y)
{
this.x = x;
this.y = y;
}
}

private struct POINT
{
public int x;
public int y;
public POINT(int x, int y)
{
this.x = x;
this.y = y;
}
}

And the POINT in UIAutomation/UIAutomationClient/MS/Win32/UnsafeNativeMethods.cs and src/Microsoft.DotNet.Wpf/src/UIAutomation/UIAutomationClientSideProviders/MS/Win32/UnsafeNativeMethods.cs do not add StructLayout attribute.

@stephentoub
Copy link
Member Author

Thanks. This is what I meant by "There are a bunch of copies around the tree of POINT interop types"; in fact, tons of interop is duplicated around the tree. It could certainly be cleaned up / deduped / etc., but I'm not going to do that here.

@ryalanms ryalanms merged commit 58e7355 into dotnet:main Aug 17, 2021
@ghost ghost locked as resolved and limited conversation to collaborators Apr 8, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

PR metadata: Label to tag PRs, to facilitate with triage

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants