Skip to content

Commit 345eb8c

Browse files
stephentoubstephentoub
authored
Avoid object[1] allocation in PropertyInfo.SetValue (#54918)
1 parent 8818c15 commit 345eb8c

File tree

5 files changed

+86
-63
lines changed

5 files changed

+86
-63
lines changed

‎src/coreclr/System.Private.CoreLib/src/System/Reflection/Emit/DynamicMethod.cs

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -460,7 +460,12 @@ internal RuntimeMethodHandle GetMethodDescriptor()
460460
bool wrapExceptions = (invokeAttr & BindingFlags.DoNotWrapExceptions) == 0;
461461

462462
StackAllocedArguments stackArgs = default;
463-
Span<object?> arguments = CheckArguments(ref stackArgs, parameters, binder, invokeAttr, culture, sig);
463+
Span<object?> arguments = default;
464+
if (actualCount != 0)
465+
{
466+
arguments = CheckArguments(ref stackArgs, parameters, binder, invokeAttr, culture, sig);
467+
}
468+
464469
object? retValue = RuntimeMethodHandle.InvokeMethod(null, arguments, sig, false, wrapExceptions);
465470

466471
// copy out. This should be made only if ByRef are present.

‎src/coreclr/System.Private.CoreLib/src/System/Reflection/MethodBase.CoreCLR.cs

Lines changed: 23 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -65,42 +65,38 @@ internal virtual Type[] GetParameterTypes()
6565
return parameterTypes;
6666
}
6767

68-
private protected Span<object?> CheckArguments(ref StackAllocedArguments stackArgs, object?[]? parameters, Binder? binder,
68+
private protected Span<object?> CheckArguments(ref StackAllocedArguments stackArgs, ReadOnlySpan<object?> parameters, Binder? binder,
6969
BindingFlags invokeAttr, CultureInfo? culture, Signature sig)
7070
{
7171
Debug.Assert(Unsafe.SizeOf<StackAllocedArguments>() == StackAllocedArguments.MaxStackAllocArgCount * Unsafe.SizeOf<object>(),
7272
"MaxStackAllocArgCount not properly defined.");
73+
Debug.Assert(!parameters.IsEmpty);
7374

74-
Span<object?> copyOfParameters = default;
75+
// We need to perform type safety validation against the incoming arguments, but we also need
76+
// to be resilient against the possibility that some other thread (or even the binder itself!)
77+
// may mutate the array after we've validated the arguments but before we've properly invoked
78+
// the method. The solution is to copy the arguments to a different, not-user-visible buffer
79+
// as we validate them. n.b. This disallows use of ArrayPool, as ArrayPool-rented arrays are
80+
// considered user-visible to threads which may still be holding on to returned instances.
7581

76-
if (parameters is not null)
82+
Span<object?> copyOfParameters = (parameters.Length <= StackAllocedArguments.MaxStackAllocArgCount)
83+
? MemoryMarshal.CreateSpan(ref stackArgs._arg0, parameters.Length)
84+
: new Span<object?>(new object?[parameters.Length]);
85+
86+
ParameterInfo[]? p = null;
87+
for (int i = 0; i < parameters.Length; i++)
7788
{
78-
// We need to perform type safety validation against the incoming arguments, but we also need
79-
// to be resilient against the possibility that some other thread (or even the binder itself!)
80-
// may mutate the array after we've validated the arguments but before we've properly invoked
81-
// the method. The solution is to copy the arguments to a different, not-user-visible buffer
82-
// as we validate them. n.b. This disallows use of ArrayPool, as ArrayPool-rented arrays are
83-
// considered user-visible to threads which may still be holding on to returned instances.
84-
85-
copyOfParameters = (parameters.Length <= StackAllocedArguments.MaxStackAllocArgCount)
86-
? MemoryMarshal.CreateSpan(ref stackArgs._arg0, parameters.Length)
87-
: new Span<object?>(new object?[parameters.Length]);
88-
89-
ParameterInfo[]? p = null;
90-
for (int i = 0; i < parameters.Length; i++)
89+
object? arg = parameters[i];
90+
RuntimeType argRT = sig.Arguments[i];
91+
92+
if (arg == Type.Missing)
9193
{
92-
object? arg = parameters[i];
93-
RuntimeType argRT = sig.Arguments[i];
94-
95-
if (arg == Type.Missing)
96-
{
97-
p ??= GetParametersNoCopy();
98-
if (p[i].DefaultValue == System.DBNull.Value)
99-
throw new ArgumentException(SR.Arg_VarMissNull, nameof(parameters));
100-
arg = p[i].DefaultValue!;
101-
}
102-
copyOfParameters[i] = argRT.CheckValue(arg, binder, culture, invokeAttr);
94+
p ??= GetParametersNoCopy();
95+
if (p[i].DefaultValue == System.DBNull.Value)
96+
throw new ArgumentException(SR.Arg_VarMissNull, nameof(parameters));
97+
arg = p[i].DefaultValue!;
10398
}
99+
copyOfParameters[i] = argRT.CheckValue(arg, binder, culture, invokeAttr);
104100
}
105101

106102
return copyOfParameters;

‎src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeConstructorInfo.cs

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -340,7 +340,12 @@ internal void ThrowNoInvokeException()
340340
bool wrapExceptions = (invokeAttr & BindingFlags.DoNotWrapExceptions) == 0;
341341

342342
StackAllocedArguments stackArgs = default;
343-
Span<object?> arguments = CheckArguments(ref stackArgs, parameters, binder, invokeAttr, culture, sig);
343+
Span<object?> arguments = default;
344+
if (actualCount != 0)
345+
{
346+
arguments = CheckArguments(ref stackArgs, parameters, binder, invokeAttr, culture, sig);
347+
}
348+
344349
object? retValue = RuntimeMethodHandle.InvokeMethod(obj, arguments, sig, false, wrapExceptions);
345350

346351
// copy out. This should be made only if ByRef are present.
@@ -394,7 +399,12 @@ public override object Invoke(BindingFlags invokeAttr, Binder? binder, object?[]
394399
bool wrapExceptions = (invokeAttr & BindingFlags.DoNotWrapExceptions) == 0;
395400

396401
StackAllocedArguments stackArgs = default;
397-
Span<object?> arguments = CheckArguments(ref stackArgs, parameters, binder, invokeAttr, culture, sig);
402+
Span<object?> arguments = default;
403+
if (actualCount != 0)
404+
{
405+
arguments = CheckArguments(ref stackArgs, parameters, binder, invokeAttr, culture, sig);
406+
}
407+
398408
object retValue = RuntimeMethodHandle.InvokeMethod(null, arguments, sig, true, wrapExceptions)!; // ctor must return non-null
399409

400410
// copy out. This should be made only if ByRef are present.

‎src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimeMethodInfo.cs

Lines changed: 32 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -410,8 +410,26 @@ private void ThrowNoInvokeException()
410410
[Diagnostics.DebuggerHidden]
411411
public override object? Invoke(object? obj, BindingFlags invokeAttr, Binder? binder, object?[]? parameters, CultureInfo? culture)
412412
{
413+
// INVOCATION_FLAGS_CONTAINS_STACK_POINTERS means that the struct (either the declaring type or the return type)
414+
// contains pointers that point to the stack. This is either a ByRef or a TypedReference. These structs cannot
415+
// be boxed and thus cannot be invoked through reflection which only deals with boxed value type objects.
416+
if ((InvocationFlags & (INVOCATION_FLAGS.INVOCATION_FLAGS_NO_INVOKE | INVOCATION_FLAGS.INVOCATION_FLAGS_CONTAINS_STACK_POINTERS)) != 0)
417+
ThrowNoInvokeException();
418+
419+
// check basic method consistency. This call will throw if there are problems in the target/method relationship
420+
CheckConsistency(obj);
421+
422+
Signature sig = Signature;
423+
int actualCount = (parameters != null) ? parameters.Length : 0;
424+
if (sig.Arguments.Length != actualCount)
425+
throw new TargetParameterCountException(SR.Arg_ParmCnt);
426+
413427
StackAllocedArguments stackArgs = default; // try to avoid intermediate array allocation if possible
414-
Span<object?> arguments = InvokeArgumentsCheck(ref stackArgs, obj, invokeAttr, binder, parameters, culture);
428+
Span<object?> arguments = default;
429+
if (actualCount != 0)
430+
{
431+
arguments = CheckArguments(ref stackArgs, parameters!, binder, invokeAttr, culture, sig);
432+
}
415433

416434
bool wrapExceptions = (invokeAttr & BindingFlags.DoNotWrapExceptions) == 0;
417435
object? retValue = RuntimeMethodHandle.InvokeMethod(obj, arguments, Signature, false, wrapExceptions);
@@ -426,34 +444,30 @@ private void ThrowNoInvokeException()
426444

427445
[DebuggerStepThroughAttribute]
428446
[Diagnostics.DebuggerHidden]
429-
private Span<object?> InvokeArgumentsCheck(ref StackAllocedArguments stackArgs, object? obj, BindingFlags invokeAttr, Binder? binder, object?[]? parameters, CultureInfo? culture)
447+
internal object? InvokeOneParameter(object? obj, BindingFlags invokeAttr, Binder? binder, object? parameter, CultureInfo? culture)
430448
{
431-
Signature sig = Signature;
432-
433-
// get the signature
434-
int formalCount = sig.Arguments.Length;
435-
int actualCount = (parameters != null) ? parameters.Length : 0;
436-
437-
INVOCATION_FLAGS invocationFlags = InvocationFlags;
438-
439449
// INVOCATION_FLAGS_CONTAINS_STACK_POINTERS means that the struct (either the declaring type or the return type)
440450
// contains pointers that point to the stack. This is either a ByRef or a TypedReference. These structs cannot
441451
// be boxed and thus cannot be invoked through reflection which only deals with boxed value type objects.
442-
if ((invocationFlags & (INVOCATION_FLAGS.INVOCATION_FLAGS_NO_INVOKE | INVOCATION_FLAGS.INVOCATION_FLAGS_CONTAINS_STACK_POINTERS)) != 0)
452+
if ((InvocationFlags & (INVOCATION_FLAGS.INVOCATION_FLAGS_NO_INVOKE | INVOCATION_FLAGS.INVOCATION_FLAGS_CONTAINS_STACK_POINTERS)) != 0)
453+
{
443454
ThrowNoInvokeException();
455+
}
444456

445457
// check basic method consistency. This call will throw if there are problems in the target/method relationship
446458
CheckConsistency(obj);
447459

448-
if (formalCount != actualCount)
449-
throw new TargetParameterCountException(SR.Arg_ParmCnt);
450-
451-
Span<object?> retVal = default;
452-
if (actualCount != 0)
460+
Signature sig = Signature;
461+
if (sig.Arguments.Length != 1)
453462
{
454-
retVal = CheckArguments(ref stackArgs, parameters!, binder, invokeAttr, culture, sig);
463+
throw new TargetParameterCountException(SR.Arg_ParmCnt);
455464
}
456-
return retVal;
465+
466+
StackAllocedArguments stackArgs = default;
467+
Span<object?> arguments = CheckArguments(ref stackArgs, new ReadOnlySpan<object?>(ref parameter, 1), binder, invokeAttr, culture, sig);
468+
469+
bool wrapExceptions = (invokeAttr & BindingFlags.DoNotWrapExceptions) == 0;
470+
return RuntimeMethodHandle.InvokeMethod(obj, arguments, Signature, constructor: false, wrapExceptions);
457471
}
458472

459473
#endregion

‎src/coreclr/System.Private.CoreLib/src/System/Reflection/RuntimePropertyInfo.cs

Lines changed: 13 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -239,15 +239,15 @@ public override MethodInfo[] GetAccessors(bool nonPublic)
239239

240240
public override Type PropertyType => Signature.ReturnType;
241241

242-
public override MethodInfo? GetGetMethod(bool nonPublic)
242+
public override RuntimeMethodInfo? GetGetMethod(bool nonPublic)
243243
{
244244
if (!Associates.IncludeAccessor(m_getterMethod, nonPublic))
245245
return null;
246246

247247
return m_getterMethod;
248248
}
249249

250-
public override MethodInfo? GetSetMethod(bool nonPublic)
250+
public override RuntimeMethodInfo? GetSetMethod(bool nonPublic)
251251
{
252252
if (!Associates.IncludeAccessor(m_setterMethod, nonPublic))
253253
return null;
@@ -282,7 +282,7 @@ internal ParameterInfo[] GetIndexParametersNoCopy()
282282
ParameterInfo[]? methParams = null;
283283

284284
// First try to get the Get method.
285-
MethodInfo? m = GetGetMethod(true);
285+
RuntimeMethodInfo? m = GetGetMethod(true);
286286
if (m != null)
287287
{
288288
// There is a Get method so use it.
@@ -337,7 +337,7 @@ internal ParameterInfo[] GetIndexParametersNoCopy()
337337
[Diagnostics.DebuggerHidden]
338338
public override object? GetValue(object? obj, BindingFlags invokeAttr, Binder? binder, object?[]? index, CultureInfo? culture)
339339
{
340-
MethodInfo? m = GetGetMethod(true);
340+
RuntimeMethodInfo? m = GetGetMethod(true);
341341
if (m == null)
342342
throw new ArgumentException(System.SR.Arg_GetMethNotFnd);
343343
return m.Invoke(obj, invokeAttr, binder, index, null);
@@ -359,28 +359,26 @@ public override void SetValue(object? obj, object? value, object?[]? index)
359359
[Diagnostics.DebuggerHidden]
360360
public override void SetValue(object? obj, object? value, BindingFlags invokeAttr, Binder? binder, object?[]? index, CultureInfo? culture)
361361
{
362-
MethodInfo? m = GetSetMethod(true);
362+
RuntimeMethodInfo? m = GetSetMethod(true);
363363

364364
if (m == null)
365365
throw new ArgumentException(System.SR.Arg_SetMethNotFnd);
366366

367-
object?[] args;
368-
if (index != null)
367+
if (index is null)
369368
{
370-
args = new object[index.Length + 1];
369+
m.InvokeOneParameter(obj, invokeAttr, binder, value, culture);
370+
}
371+
else
372+
{
373+
var args = new object?[index.Length + 1];
371374

372375
for (int i = 0; i < index.Length; i++)
373376
args[i] = index[i];
374377

375378
args[index.Length] = value;
376-
}
377-
else
378-
{
379-
args = new object[1];
380-
args[0] = value;
381-
}
382379

383-
m.Invoke(obj, invokeAttr, binder, args, culture);
380+
m.Invoke(obj, invokeAttr, binder, args, culture);
381+
}
384382
}
385383
#endregion
386384

0 commit comments

Comments
 (0)