Skip to content

Commit aa0af5e

Browse files
daxian-dbwdaxian-dbw
authored
Further improve PSMethod to Delegate conversion (#6851)
Refactor code to make it easier to maintain and a little faster. Changes are as follows: 1. Support finding a matching signature with variance. But make PowerShell prefer exact match over a match with variance. 2. The metadata signatures in `PSMethod<..>` are generated based on the array of method overloads in `MethodCacheEntry.MethodInformationStructures`, in the exact same order. So in `LanguagePrimitive.ConvertViaParseMethod`, when we try to figure out if there is a match using the metadata signatures in `PSMethod<..>`, we can get the index of the matching signature, and the same index should locate the matching method in `MethodCacheEntry.MethodInformationStructures`. Therefore, we don't need to compare signatures again in the actual conversion method, and instead, we can just leverage the index we found when figuring out the conversion in `ConvertViaParseMethod`. - This gets rid of the reflection call `GetMethod("Invoke")` and the subsequent signature comparisons in the final conversion method. - Also, when comparing signatures using `PSMethod<..>` in `ConvertViaParseMethod`, we can just use the generic argument types of each `Func<..>` metadata type, instead of calling `GetMethod("Invoke")` and then `GetParameters()`. This makes the code for comparing signatures simpler (the type `SignatureComparator`). - Move `MatchesPSMethodProjectedType` from `PSMemberInfo.cs` to the type `SignatureComparator` in `LanguagePrimitives.cs`, as it's closely related to the signature comparison. Also, renamed it to `ProjectedTypeMatchesTargetType`. - These changes make PSMethod-to-Delegate conversion a little faster, but no big improvement, as the true bottleneck probably is in delegate creation(?). Actually, the performance of this conversion is not critical at all at this moment because this feature should rarely be used in any hot script path. So this exercise is mainly for fun. 3. Remove `PSEnum<T>`. We can directly use enum types when constructing the metadata type `Func<..>`. 4. Remove the code that generates metadata signatures for generic method definitions (call `MakeGenericMethod` with fake types like `GenericType0`, `GenericType1`). This is because: - We don't support convert generic method to delegate today, so may be better not spending time on preparing the metadata signature types for those methods. - When the day comes that we need to support it, it's better to use generic argument types directly to construct the `Func<..>` metadata types. I left comments in `GetMethodGroupType` method in `PSMemberInfo.cs` to explain why that approach is better.
1 parent 472b5f7 commit aa0af5e

4 files changed

Lines changed: 308 additions & 279 deletions

File tree

‎src/System.Management.Automation/engine/LanguagePrimitives.cs‎

Lines changed: 175 additions & 77 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
using System.Text;
2020
using System.Management.Automation.Internal;
2121
using System.Management.Automation.Runspaces;
22-
using System.Diagnostics.CodeAnalysis; // for fxcop
22+
using System.Diagnostics.CodeAnalysis;
2323
using Dbg = System.Management.Automation.Diagnostics;
2424
using MethodCacheEntry = System.Management.Automation.DotNetAdapter.MethodCacheEntry;
2525

@@ -3202,49 +3202,6 @@ private static Delegate ConvertScriptBlockToDelegate(object valueToConvert,
32023202
valueToConvert.ToString(), resultType.ToString(), exception.Message);
32033203
}
32043204

3205-
private static Delegate ConvertPSMethodInfoToDelegate(object valueToConvert,
3206-
Type resultType,
3207-
bool recurse,
3208-
PSObject originalValueToConvert,
3209-
IFormatProvider formatProvider,
3210-
TypeTable backupTable)
3211-
{
3212-
// We can only possibly convert PSMethod instance of the type PSMethod<T>.
3213-
// Such a PSMethod essentially represents a set of .NET method overloads.
3214-
var psMethod = (PSMethod)valueToConvert;
3215-
3216-
try
3217-
{
3218-
var methods = (MethodCacheEntry)psMethod.adapterData;
3219-
var isStatic = psMethod.instance is Type;
3220-
var targetMethodInfo = resultType.GetMethod("Invoke");
3221-
var comparator = new DelegateArgsComparator(targetMethodInfo);
3222-
3223-
foreach (var methodInformation in methods.methodInformationStructures)
3224-
{
3225-
var candidate = (MethodInfo)methodInformation.method;
3226-
if (comparator.SignatureMatches(candidate.ReturnType, candidate.GetParameters()))
3227-
{
3228-
return isStatic ? candidate.CreateDelegate(resultType)
3229-
: candidate.CreateDelegate(resultType, psMethod.instance);
3230-
}
3231-
}
3232-
}
3233-
catch (Exception e)
3234-
{
3235-
typeConversion.WriteLine("PSMethod to Delegate exception: \"{0}\".", e.Message);
3236-
throw new PSInvalidCastException("InvalidCastExceptionPSMethodToDelegate", e,
3237-
ExtendedTypeSystem.InvalidCastExceptionWithInnerException,
3238-
valueToConvert.ToString(), resultType.ToString(), e.Message);
3239-
}
3240-
3241-
var msg = String.Format(ExtendedTypeSystem.PSMethodToDelegateNoMatchingOverLoad, psMethod, resultType);
3242-
typeConversion.WriteLine($"PSMethod to Delegate exception: \"{msg}\".");
3243-
throw new PSInvalidCastException("InvalidCastExceptionPSMethodToDelegate", null,
3244-
ExtendedTypeSystem.InvalidCastExceptionWithInnerException,
3245-
valueToConvert.ToString(), resultType.ToString(), msg);
3246-
}
3247-
32483205
private static object ConvertToNullable(object valueToConvert,
32493206
Type resultType,
32503207
bool recursion,
@@ -3486,6 +3443,65 @@ private static object ConvertEnumerableToEnum(object valueToConvert,
34863443
return ConvertStringToEnum(sbResult.ToString(), resultType, recursion, originalValueToConvert, formatProvider, backupTable);
34873444
}
34883445

3446+
private class PSMethodToDelegateConverter
3447+
{
3448+
// Index of the matching overload method.
3449+
private readonly int _matchIndex;
3450+
// Size of the cache. It's rare to have more than 10 overloads for a method.
3451+
private const int CacheSize = 10;
3452+
private static readonly PSMethodToDelegateConverter[] s_converterCache = new PSMethodToDelegateConverter[CacheSize];
3453+
3454+
private PSMethodToDelegateConverter(int matchIndex)
3455+
{
3456+
_matchIndex = matchIndex;
3457+
}
3458+
3459+
internal static PSMethodToDelegateConverter GetConverter(int matchIndex)
3460+
{
3461+
if (matchIndex >= CacheSize) { return new PSMethodToDelegateConverter(matchIndex); }
3462+
3463+
var result = s_converterCache[matchIndex];
3464+
if (result == null)
3465+
{
3466+
// If the cache entry is null, generate a new instance for the cache slot.
3467+
var converter = new PSMethodToDelegateConverter(matchIndex);
3468+
Threading.Interlocked.CompareExchange(ref s_converterCache[matchIndex], converter, null);
3469+
result = s_converterCache[matchIndex];
3470+
}
3471+
3472+
return result;
3473+
}
3474+
3475+
internal Delegate Convert(object valueToConvert,
3476+
Type resultType,
3477+
bool recursion,
3478+
PSObject originalValueToConvert,
3479+
IFormatProvider formatProvider,
3480+
TypeTable backupTable)
3481+
{
3482+
// We can only possibly convert PSMethod instance of the type PSMethod<T>.
3483+
// Such a PSMethod essentially represents a set of .NET method overloads.
3484+
var psMethod = (PSMethod)valueToConvert;
3485+
3486+
try
3487+
{
3488+
var methods = (MethodCacheEntry)psMethod.adapterData;
3489+
var isStatic = psMethod.instance is Type;
3490+
3491+
var candidate = (MethodInfo)methods.methodInformationStructures[_matchIndex].method;
3492+
return isStatic ? candidate.CreateDelegate(resultType)
3493+
: candidate.CreateDelegate(resultType, psMethod.instance);
3494+
}
3495+
catch (Exception e)
3496+
{
3497+
typeConversion.WriteLine("PSMethod to Delegate exception: \"{0}\".", e.Message);
3498+
throw new PSInvalidCastException("InvalidCastExceptionPSMethodToDelegate", e,
3499+
ExtendedTypeSystem.InvalidCastExceptionWithInnerException,
3500+
valueToConvert.ToString(), resultType.ToString(), e.Message);
3501+
}
3502+
}
3503+
}
3504+
34893505
private class ConvertViaParseMethod
34903506
{
34913507
// TODO - use an ETS wrapper that generates a dynamic method
@@ -4772,66 +4788,148 @@ private static ConversionData FigureLanguageConversion(Type fromType, Type toTyp
47724788
return CacheConversion<object>(fromType, toType, LanguagePrimitives.ConvertIntegerToEnum, ConversionRank.Language);
47734789
}
47744790

4775-
if (fromType.IsSubclassOf(typeof(PSMethod)) && toType.IsSubclassOf(typeof(Delegate)))
4791+
if (fromType.IsSubclassOf(typeof(PSMethod)) && toType.IsSubclassOf(typeof(Delegate)) && !toType.IsAbstract)
47764792
{
4777-
var mi = toType.GetMethod("Invoke");
4778-
4779-
var comparator = new DelegateArgsComparator(mi);
4793+
var targetMethod = toType.GetMethod("Invoke");
4794+
var comparator = new SignatureComparator(targetMethod);
47804795
var signatureEnumerator = new PSMethodSignatureEnumerator(fromType);
4796+
int index = -1, matchedIndex = -1;
4797+
47814798
while (signatureEnumerator.MoveNext())
47824799
{
4783-
var candidate = signatureEnumerator.Current.GetMethod("Invoke");
4784-
if (comparator.SignatureMatches(candidate.ReturnType, candidate.GetParameters()))
4800+
index++;
4801+
var signatureType = signatureEnumerator.Current;
4802+
// Skip the non-bindable signatures
4803+
if (signatureType == typeof(Func<PSNonBindableType>)) { continue; }
4804+
4805+
Type[] argumentTypes = signatureType.GenericTypeArguments;
4806+
if (comparator.ProjectedSignatureMatchesTarget(argumentTypes, out bool signaturesMatchExactly))
47854807
{
4786-
return CacheConversion<Delegate>(fromType, toType, LanguagePrimitives.ConvertPSMethodInfoToDelegate, ConversionRank.Language);
4808+
if (signaturesMatchExactly)
4809+
{
4810+
// We prefer the signature that exactly matches the target delegate.
4811+
matchedIndex = index;
4812+
break;
4813+
}
4814+
4815+
// If there is no exact match, then we use the first compatible signature we found.
4816+
if (matchedIndex == -1) { matchedIndex = index; }
47874817
}
47884818
}
4819+
4820+
if (matchedIndex > -1)
4821+
{
4822+
// We got the index of the matching method signature based on the PSMethod<..> type.
4823+
// Signatures in PSMethod<..> type were constructed based on the array of method overloads,
4824+
// in the exact order. So we can use this index directly to locate the matching overload in
4825+
// the converter, without having to compare the signature again.
4826+
var converter = PSMethodToDelegateConverter.GetConverter(matchedIndex);
4827+
return CacheConversion<Delegate>(fromType, toType, converter.Convert, ConversionRank.Language);
4828+
}
47894829
}
47904830

47914831
return null;
47924832
}
47934833

4794-
struct DelegateArgsComparator
4834+
private struct SignatureComparator
47954835
{
4796-
private readonly ParameterInfo[] _targetParametersInfos;
4797-
private readonly Type _returnType;
4798-
4799-
public DelegateArgsComparator(MethodInfo targetMethodInfo)
4836+
enum TypeMatchingContext
48004837
{
4801-
_returnType = targetMethodInfo.ReturnType;
4802-
_targetParametersInfos = targetMethodInfo.GetParameters();
4838+
ReturnType,
4839+
ParameterType,
4840+
OutParameterType
48034841
}
48044842

4805-
public bool SignatureMatches(Type returnType, ParameterInfo[] arguments)
4843+
private readonly ParameterInfo[] targetParameters;
4844+
private readonly Type targetReturnType;
4845+
4846+
internal SignatureComparator(MethodInfo targetMethodInfo)
48064847
{
4807-
return ReturnTypeMatches(returnType) && ParameterTypesMatches(arguments);
4848+
targetReturnType = targetMethodInfo.ReturnType;
4849+
targetParameters = targetMethodInfo.GetParameters();
48084850
}
48094851

4810-
private bool ReturnTypeMatches(Type returnType)
4811-
{
4812-
return PSMethod.MatchesPSMethodProjectedType(_returnType, returnType, testAssignment: true);
4852+
/// <summary>
4853+
/// Check if a projected signature matches the target method.
4854+
/// </summary>
4855+
/// <param name="argumentTypes">
4856+
/// The type arguments from the metadata type 'Func[..]' that represents the projected signature.
4857+
/// It contains the return type as the last item in the array.
4858+
/// </param>
4859+
/// <param name="signaturesMatchExactly">
4860+
/// Set by this method to indicate if it's an exact match.
4861+
/// </param>
4862+
internal bool ProjectedSignatureMatchesTarget(Type[] argumentTypes, out bool signaturesMatchExactly)
4863+
{
4864+
signaturesMatchExactly = false;
4865+
int length = argumentTypes.Length;
4866+
if (length != targetParameters.Length + 1) { return false; }
4867+
4868+
bool typesMatchExactly, allTypesMatchExactly;
4869+
Type sourceReturnType = argumentTypes[length - 1];
4870+
4871+
if (ProjectedTypeMatchesTargetType(sourceReturnType, targetReturnType, TypeMatchingContext.ReturnType, out typesMatchExactly))
4872+
{
4873+
allTypesMatchExactly = typesMatchExactly;
4874+
for (int i = 0; i < targetParameters.Length; i++)
4875+
{
4876+
var targetParam = targetParameters[i];
4877+
var sourceType = argumentTypes[i];
4878+
var matchContext = targetParam.IsOut ? TypeMatchingContext.OutParameterType : TypeMatchingContext.ParameterType;
4879+
4880+
if (!ProjectedTypeMatchesTargetType(sourceType, targetParam.ParameterType, matchContext, out typesMatchExactly))
4881+
{
4882+
return false;
4883+
}
4884+
allTypesMatchExactly &= typesMatchExactly;
4885+
}
4886+
4887+
signaturesMatchExactly = allTypesMatchExactly;
4888+
return true;
4889+
}
4890+
4891+
return false;
48134892
}
48144893

4815-
private bool ParameterTypesMatches(ParameterInfo[] arguments)
4894+
private static bool ProjectedTypeMatchesTargetType(Type sourceType, Type targetType, TypeMatchingContext matchContext, out bool matchExactly)
48164895
{
4817-
var argsCount = _targetParametersInfos.Length;
4818-
// void is encoded as typeof(VOID) in the PSMethod<MethodGroup<>> as the last parameter
4819-
if (arguments.Length != argsCount)
4896+
matchExactly = false;
4897+
if (targetType.IsByRef || targetType.IsPointer)
48204898
{
4899+
if (!sourceType.IsGenericType) { return false; }
4900+
4901+
var sourceTypeDef = sourceType.GetGenericTypeDefinition();
4902+
bool isOutParameter = matchContext == TypeMatchingContext.OutParameterType;
4903+
4904+
if (targetType.IsByRef && sourceTypeDef == (isOutParameter ? typeof(PSOutParameter<>) : typeof(PSReference<>)) ||
4905+
targetType.IsPointer && sourceTypeDef == typeof(PSPointer<>))
4906+
{
4907+
// For ref/out parameter types and pointer types, the element types need to match exactly.
4908+
if (targetType.GetElementType() == sourceType.GenericTypeArguments[0])
4909+
{
4910+
matchExactly = true;
4911+
return true;
4912+
}
4913+
}
48214914
return false;
48224915
}
4823-
for (int i = 0; i < arguments.Length; i++)
4916+
4917+
if (targetType == sourceType ||
4918+
targetType == typeof(void) && sourceType == typeof(VOID) ||
4919+
targetType == typeof(TypedReference) && sourceType == typeof(PSTypedReference))
48244920
{
4825-
var arg = arguments[i];
4826-
var argType = arg.ParameterType;
4827-
var targetParamType = _targetParametersInfos[i].ParameterType;
4828-
var isOut = (arg.Attributes | ParameterAttributes.Out) == ParameterAttributes.Out;
4829-
if (!PSMethod.MatchesPSMethodProjectedType(targetParamType, argType, isOut: isOut))
4830-
{
4831-
return false;
4832-
}
4921+
matchExactly = true;
4922+
return true;
48334923
}
4834-
return true;
4924+
4925+
if (targetType == typeof(void) || targetType == typeof(TypedReference))
4926+
{
4927+
return false;
4928+
}
4929+
4930+
return matchContext == TypeMatchingContext.ReturnType
4931+
? targetType.IsAssignableFrom(sourceType)
4932+
: sourceType.IsAssignableFrom(targetType);
48354933
}
48364934
}
48374935

0 commit comments

Comments
 (0)