Avoid intermediate allocations in MethodInfo/ConstructorInfo.Invoke by GrabYourPitchforks · Pull Request #50814 · dotnet/runtime

GrabYourPitchforks · 2021-04-06T21:43:38Z

Resolves #12832.

The managed reflection stack creates a defensive copy of the parameters object[] array on calls to MethodInfo.Invoke and ConstructorInfo.Invoke to prevent the parameter values changing between the initial type safety check and the real invocation of the target method. This PR special-cases "small" arrays (currently defined as <= 8 arguments) and creates a stack-based defensive copy instead of a heap-based defensive copy. This saves up to 88 bytes of heap allocations (on x64) per MethodInfo.Invoke or ConstructorInfo.Invoke call.

If 9 or more parameters are present, we fall back to the normal "allocate an array on the heap" code path.

Method	Job	Toolchain	ArgCount	Mean	Error	StdDev	Ratio	RatioSD	Gen 0	Gen 1	Gen 2	Allocated
MethodInfoInvoke	Job-ZXIRIY	main	0	69.00 ns	1.142 ns	1.068 ns	1.00	0.00	-	-	-	-
MethodInfoInvoke	Job-TABAFA	proto	0	64.27 ns	0.509 ns	0.451 ns	0.93	0.02	-	-	-	-

ConstructorInfoInvoke	Job-ZXIRIY	main	0	90.13 ns	0.719 ns	0.672 ns	1.00	0.00	0.0029	-	-	24 B
ConstructorInfoInvoke	Job-TABAFA	proto	0	91.63 ns	0.337 ns	0.298 ns	1.02	0.01	0.0029	-	-	24 B

MethodInfoInvoke	Job-ZXIRIY	main	1	123.50 ns	2.510 ns	5.917 ns	1.00	0.00	0.0038	-	-	32 B
MethodInfoInvoke	Job-TABAFA	proto	1	114.30 ns	1.356 ns	1.202 ns	0.93	0.04	-	-	-	-

ConstructorInfoInvoke	Job-ZXIRIY	main	1	142.11 ns	1.344 ns	1.191 ns	1.00	0.00	0.0067	-	-	56 B
ConstructorInfoInvoke	Job-TABAFA	proto	1	131.96 ns	1.575 ns	1.474 ns	0.93	0.01	0.0029	-	-	24 B

MethodInfoInvoke	Job-ZXIRIY	main	8	358.50 ns	2.529 ns	2.365 ns	1.00	0.00	0.0105	-	-	88 B
MethodInfoInvoke	Job-TABAFA	proto	8	358.44 ns	6.179 ns	5.478 ns	1.00	0.02	-	-	-	-

ConstructorInfoInvoke	Job-ZXIRIY	main	8	383.78 ns	4.111 ns	3.846 ns	1.00	0.00	0.0134	-	-	112 B
ConstructorInfoInvoke	Job-TABAFA	proto	8	377.16 ns	1.993 ns	1.864 ns	0.98	0.01	0.0029	-	-	24 B

MethodInfoInvoke	Job-ZXIRIY	main	10	438.32 ns	5.554 ns	5.196 ns	1.00	0.00	0.0124	-	-	104 B
MethodInfoInvoke	Job-TABAFA	proto	10	434.74 ns	3.087 ns	2.888 ns	0.99	0.01	0.0124	-	-	104 B

ConstructorInfoInvoke	Job-ZXIRIY	main	10	454.83 ns	1.713 ns	1.431 ns	1.00	0.00	0.0153	-	-	128 B
ConstructorInfoInvoke	Job-TABAFA	proto	10	465.04 ns	4.878 ns	4.563 ns	1.02	0.01	0.0153	-	-	128 B

I also took this opportunity to refactor some code that was on the Invoke hot paths, such as lazy initialization of the InvocationFlags and Signature properties. These help offset some of the cost of setting up the struct-based argument holder.

GrabYourPitchforks · 2021-04-06T21:51:58Z

Note to reviewers: This will be easier to review with hide whitespace changes set.

There's already significant unit and functional test coverage over all the reflection invocation code paths. I can't think offhand of any useful tests I could introduce alongside this change. But please feel free to offer suggestions and I can try to get them in.

stephentoub · 2021-04-07T14:41:19Z

+            private object? _arg6;
+            private object? _arg7;
+#pragma warning restore CA1823, CS0169, IDE0051
+        }


We now have a few of these floating around, e.g.

runtime/src/libraries/Common/src/Interop/Windows/SspiCli/SSPIWrapper.cs

Lines 218 to 224 in 79ae74f

private ref struct ThreeByteArrays

{

public const int NumItems = 3;

internal byte[] _item0;

private byte[] _item1;

private byte[] _item2;

}

runtime/src/libraries/System.Text.RegularExpressions/src/System/Text/RegularExpressions/RegexReplacement.cs

Lines 97 to 103 in 79ae74f

private struct FourStackStrings // used to do the equivalent of: Span<string> strings = stackalloc string[4];

{

public string Item1;

public string Item2;

public string Item3;

public string Item4;

}

and other places we'd wanted to but just didn't go to the lengths of creating these (there's also the additional zero'ing overhead that comes if you overestimate). At some point, we should think about doing something more general, whether it be enabling runtime support for stackalloc to work with reference types, or language support to create these types for stackallocs with const sizes, or a set of helper types we simply share either in Common or even publicly.

davidfowl · 2021-04-07T14:44:40Z

Can we add overloads to make these APIs support Span<object>, that would at least be a good first step.

GrabYourPitchforks · 2021-04-07T16:13:29Z

@davidfowl Feel free to bring that to API review. Though I suspect you'll be more interested in the other "fast reflection" work being considered, which significantly cuts the remaining overhead from these methods. See other reflection issues assigned to Steve or me for more info.

GrabYourPitchforks · 2021-04-12T21:32:20Z

Changes in latest update:

Knocked max stack allocation back from 8 to 4 elements. From what I've observed in the wild, most people pass a small number of parameters (usually 1 - 2) into the reflection APIs. If we get evidence that bumping this limit up would be beneficial, we could do that relatively easily.
Implemented Span<T> at the native layer per Jan's suggestion at Avoid intermediate allocations in MethodInfo/ConstructorInfo.Invoke #50814 (comment). I went a little further than what was minimally required for this scenario, as the logic introduced in object.h should be generalizable enough where we can immediately start using it in other code paths if desired.

My benchmark machine is currently producing lots of noise at the moment (WU, perhaps?). But as a rough estimate it looks about inline with the previous iteration.

jkotas

LGTM. Thank you!

GrabYourPitchforks added 5 commits April 5, 2021 18:43

Avoid temporary array allocations in reflection

034a0bf

Suppress more build warnings

9d6b0e8

Add nullability annotations

e91d773

Move some lazy property getters out of the hot path

d32a92a

More factoring out common fast paths

cf192b6

GrabYourPitchforks added area-System.Reflection optimization labels Apr 6, 2021

GrabYourPitchforks added this to the 6.0.0 milestone Apr 6, 2021

GrabYourPitchforks requested a review from steveharter April 6, 2021 21:43

GrabYourPitchforks commented Apr 6, 2021

View reviewed changes

Comment thread src/coreclr/System.Private.CoreLib/src/System/Reflection/MethodBase.CoreCLR.cs

stephentoub reviewed Apr 7, 2021

View reviewed changes

Comment thread src/coreclr/System.Private.CoreLib/src/System/Reflection/MethodBase.CoreCLR.cs Outdated

stephentoub reviewed Apr 7, 2021

View reviewed changes

Comment thread src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs Outdated

GrabYourPitchforks added 2 commits April 7, 2021 12:10

Comment cleanup - no functional changes

8689f6e

Moved some function blocks around for clarity - no functional changes

1bb9948

This was referenced Apr 8, 2021

Runtime jobs being abandoned due to infra #50746

Closed

Runtime jobs being abandoned due to infra #50747

Closed

jkotas reviewed Apr 11, 2021

View reviewed changes

Comment thread src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs Outdated

jkotas reviewed Apr 11, 2021

View reviewed changes

Comment thread src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs Outdated

steveharter reviewed Apr 12, 2021

View reviewed changes

Comment thread src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.cs

GrabYourPitchforks added 3 commits April 12, 2021 13:41

Pass span into native stack byref

9657151

Knock max stackalloced ctor parameters back to 4

fcfd776

Move all CheckConsistency logic inline

1332492

jkotas reviewed Apr 12, 2021

View reviewed changes

Comment thread src/coreclr/vm/object.h Outdated

jkotas reviewed Apr 12, 2021

View reviewed changes

Comment thread src/coreclr/vm/object.h Outdated

Simplify native Span<T> and remove 'delete' ctors

7dbafcb

jkotas approved these changes Apr 13, 2021

View reviewed changes

jkotas merged commit 880903b into dotnet:main Apr 13, 2021

GrabYourPitchforks deleted the refl_mi_invoke_mem branch April 13, 2021 05:55

ghost locked as resolved and limited conversation to collaborators May 13, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Avoid intermediate allocations in MethodInfo/ConstructorInfo.Invoke#50814

Avoid intermediate allocations in MethodInfo/ConstructorInfo.Invoke#50814
jkotas merged 11 commits intodotnet:mainfrom
GrabYourPitchforks:refl_mi_invoke_mem

GrabYourPitchforks commented Apr 6, 2021 •

edited

Loading

Uh oh!

GrabYourPitchforks commented Apr 6, 2021 •

edited

Loading

Uh oh!

Uh oh!

stephentoub Apr 7, 2021

Uh oh!

davidfowl commented Apr 7, 2021

Uh oh!

Uh oh!

Uh oh!

GrabYourPitchforks commented Apr 7, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GrabYourPitchforks commented Apr 12, 2021 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

jkotas left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

	private ref struct ThreeByteArrays
	{
	public const int NumItems = 3;
	internal byte[] _item0;
	private byte[] _item1;
	private byte[] _item2;
	}

	private struct FourStackStrings // used to do the equivalent of: Span<string> strings = stackalloc string[4];
	{
	public string Item1;
	public string Item2;
	public string Item3;
	public string Item4;
	}

Conversation

GrabYourPitchforks commented Apr 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

GrabYourPitchforks commented Apr 6, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

stephentoub Apr 7, 2021

Choose a reason for hiding this comment

Uh oh!

davidfowl commented Apr 7, 2021

Uh oh!

Uh oh!

Uh oh!

GrabYourPitchforks commented Apr 7, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

GrabYourPitchforks commented Apr 12, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jkotas left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

GrabYourPitchforks commented Apr 6, 2021 •

edited

Loading

GrabYourPitchforks commented Apr 6, 2021 •

edited

Loading

GrabYourPitchforks commented Apr 12, 2021 •

edited

Loading