Skip to content

Commit 2537bea

Browse files
authored
Merge pull request #3794 from jskeet/reflection
Change C# reflection to avoid using expression trees
2 parents 63bba9b + 9c05c35 commit 2537bea

File tree

4 files changed

+166
-40
lines changed

4 files changed

+166
-40
lines changed

Makefile.am

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ csharp_EXTRA_DIST= \
133133
csharp/src/Google.Protobuf/Collections/ProtobufEqualityComparers.cs \
134134
csharp/src/Google.Protobuf/Collections/ReadOnlyDictionary.cs \
135135
csharp/src/Google.Protobuf/Collections/RepeatedField.cs \
136+
csharp/src/Google.Protobuf/Compatibility/MethodInfoExtensions.cs \
136137
csharp/src/Google.Protobuf/Compatibility/PropertyInfoExtensions.cs \
137138
csharp/src/Google.Protobuf/Compatibility/StreamExtensions.cs \
138139
csharp/src/Google.Protobuf/Compatibility/TypeExtensions.cs \
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
#region Copyright notice and license
2+
// Protocol Buffers - Google's data interchange format
3+
// Copyright 2017 Google Inc. All rights reserved.
4+
// https://developers.google.com/protocol-buffers/
5+
//
6+
// Redistribution and use in source and binary forms, with or without
7+
// modification, are permitted provided that the following conditions are
8+
// met:
9+
//
10+
// * Redistributions of source code must retain the above copyright
11+
// notice, this list of conditions and the following disclaimer.
12+
// * Redistributions in binary form must reproduce the above
13+
// copyright notice, this list of conditions and the following disclaimer
14+
// in the documentation and/or other materials provided with the
15+
// distribution.
16+
// * Neither the name of Google Inc. nor the names of its
17+
// contributors may be used to endorse or promote products derived from
18+
// this software without specific prior written permission.
19+
//
20+
// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
21+
// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
22+
// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
23+
// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
24+
// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
25+
// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
26+
// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
27+
// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
28+
// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
29+
// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
30+
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
31+
#endregion
32+
33+
#if NET35
34+
using System;
35+
using System.Reflection;
36+
37+
namespace Google.Protobuf.Compatibility
38+
{
39+
// .NET Core (at least netstandard1.0) doesn't have Delegate.CreateDelegate, and .NET 3.5 doesn't have
40+
// MethodInfo.CreateDelegate. Proxy from one to the other on .NET 3.5...
41+
internal static class MethodInfoExtensions
42+
{
43+
internal static Delegate CreateDelegate(this MethodInfo method, Type type) =>
44+
Delegate.CreateDelegate(type, method);
45+
}
46+
}
47+
#endif

csharp/src/Google.Protobuf/Reflection/OneofAccessor.cs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,7 @@ internal OneofAccessor(PropertyInfo caseProperty, MethodInfo clearMethod, OneofD
5252
throw new ArgumentException("Cannot read from property");
5353
}
5454
this.descriptor = descriptor;
55-
caseDelegate = ReflectionUtil.CreateFuncIMessageT<int>(caseProperty.GetGetMethod());
55+
caseDelegate = ReflectionUtil.CreateFuncIMessageInt32(caseProperty.GetGetMethod());
5656

5757
this.descriptor = descriptor;
5858
clearDelegate = ReflectionUtil.CreateActionIMessage(clearMethod);

csharp/src/Google.Protobuf/Reflection/ReflectionUtil.cs

Lines changed: 117 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -30,11 +30,14 @@
3030
// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
3131
#endregion
3232

33+
using Google.Protobuf.Compatibility;
3334
using System;
34-
using System.Collections.Generic;
35-
using System.Linq.Expressions;
3635
using System.Reflection;
3736

37+
#if NET35
38+
using Google.Protobuf.Compatibility;
39+
#endif
40+
3841
namespace Google.Protobuf.Reflection
3942
{
4043
/// <summary>
@@ -53,55 +56,130 @@ internal static class ReflectionUtil
5356
internal static readonly Type[] EmptyTypes = new Type[0];
5457

5558
/// <summary>
56-
/// Creates a delegate which will cast the argument to the appropriate method target type,
59+
/// Creates a delegate which will cast the argument to the type that declares the method,
5760
/// call the method on it, then convert the result to object.
5861
/// </summary>
59-
internal static Func<IMessage, object> CreateFuncIMessageObject(MethodInfo method)
60-
{
61-
ParameterExpression parameter = Expression.Parameter(typeof(IMessage), "p");
62-
Expression downcast = Expression.Convert(parameter, method.DeclaringType);
63-
Expression call = Expression.Call(downcast, method);
64-
Expression upcast = Expression.Convert(call, typeof(object));
65-
return Expression.Lambda<Func<IMessage, object>>(upcast, parameter).Compile();
66-
}
62+
/// <param name="method">The method to create a delegate for, which must be declared in an IMessage
63+
/// implementation.</param>
64+
internal static Func<IMessage, object> CreateFuncIMessageObject(MethodInfo method) =>
65+
GetReflectionHelper(method.DeclaringType, method.ReturnType).CreateFuncIMessageObject(method);
6766

6867
/// <summary>
69-
/// Creates a delegate which will cast the argument to the appropriate method target type,
70-
/// call the method on it, then convert the result to the specified type.
68+
/// Creates a delegate which will cast the argument to the type that declares the method,
69+
/// call the method on it, then convert the result to the specified type. The method is expected
70+
/// to actually return an enum (because of where we're calling it - for oneof cases). Sometimes that
71+
/// means we need some extra work to perform conversions.
7172
/// </summary>
72-
internal static Func<IMessage, T> CreateFuncIMessageT<T>(MethodInfo method)
73-
{
74-
ParameterExpression parameter = Expression.Parameter(typeof(IMessage), "p");
75-
Expression downcast = Expression.Convert(parameter, method.DeclaringType);
76-
Expression call = Expression.Call(downcast, method);
77-
Expression upcast = Expression.Convert(call, typeof(T));
78-
return Expression.Lambda<Func<IMessage, T>>(upcast, parameter).Compile();
79-
}
73+
/// <param name="method">The method to create a delegate for, which must be declared in an IMessage
74+
/// implementation.</param>
75+
internal static Func<IMessage, int> CreateFuncIMessageInt32(MethodInfo method) =>
76+
GetReflectionHelper(method.DeclaringType, method.ReturnType).CreateFuncIMessageInt32(method);
8077

8178
/// <summary>
8279
/// Creates a delegate which will execute the given method after casting the first argument to
83-
/// the target type of the method, and the second argument to the first parameter type of the method.
80+
/// the type that declares the method, and the second argument to the first parameter type of the method.
8481
/// </summary>
85-
internal static Action<IMessage, object> CreateActionIMessageObject(MethodInfo method)
86-
{
87-
ParameterExpression targetParameter = Expression.Parameter(typeof(IMessage), "target");
88-
ParameterExpression argParameter = Expression.Parameter(typeof(object), "arg");
89-
Expression castTarget = Expression.Convert(targetParameter, method.DeclaringType);
90-
Expression castArgument = Expression.Convert(argParameter, method.GetParameters()[0].ParameterType);
91-
Expression call = Expression.Call(castTarget, method, castArgument);
92-
return Expression.Lambda<Action<IMessage, object>>(call, targetParameter, argParameter).Compile();
93-
}
82+
/// <param name="method">The method to create a delegate for, which must be declared in an IMessage
83+
/// implementation.</param>
84+
internal static Action<IMessage, object> CreateActionIMessageObject(MethodInfo method) =>
85+
GetReflectionHelper(method.DeclaringType, method.GetParameters()[0].ParameterType).CreateActionIMessageObject(method);
9486

9587
/// <summary>
9688
/// Creates a delegate which will execute the given method after casting the first argument to
97-
/// the target type of the method.
89+
/// type that declares the method.
9890
/// </summary>
99-
internal static Action<IMessage> CreateActionIMessage(MethodInfo method)
91+
/// <param name="method">The method to create a delegate for, which must be declared in an IMessage
92+
/// implementation.</param>
93+
internal static Action<IMessage> CreateActionIMessage(MethodInfo method) =>
94+
GetReflectionHelper(method.DeclaringType, typeof(object)).CreateActionIMessage(method);
95+
96+
/// <summary>
97+
/// Creates a reflection helper for the given type arguments. Currently these are created on demand
98+
/// rather than cached; this will be "busy" when initially loading a message's descriptor, but after that
99+
/// they can be garbage collected. We could cache them by type if that proves to be important, but creating
100+
/// an object is pretty cheap.
101+
/// </summary>
102+
private static IReflectionHelper GetReflectionHelper(Type t1, Type t2) =>
103+
(IReflectionHelper) Activator.CreateInstance(typeof(ReflectionHelper<,>).MakeGenericType(t1, t2));
104+
105+
// Non-generic interface allowing us to use an instance of ReflectionHelper<T1, T2> without statically
106+
// knowing the types involved.
107+
private interface IReflectionHelper
108+
{
109+
Func<IMessage, int> CreateFuncIMessageInt32(MethodInfo method);
110+
Action<IMessage> CreateActionIMessage(MethodInfo method);
111+
Func<IMessage, object> CreateFuncIMessageObject(MethodInfo method);
112+
Action<IMessage, object> CreateActionIMessageObject(MethodInfo method);
113+
}
114+
115+
private class ReflectionHelper<T1, T2> : IReflectionHelper
100116
{
101-
ParameterExpression targetParameter = Expression.Parameter(typeof(IMessage), "target");
102-
Expression castTarget = Expression.Convert(targetParameter, method.DeclaringType);
103-
Expression call = Expression.Call(castTarget, method);
104-
return Expression.Lambda<Action<IMessage>>(call, targetParameter).Compile();
105-
}
117+
118+
public Func<IMessage, int> CreateFuncIMessageInt32(MethodInfo method)
119+
{
120+
// On pleasant runtimes, we can create a Func<int> from a method returning
121+
// an enum based on an int. That's the fast path.
122+
if (CanConvertEnumFuncToInt32Func)
123+
{
124+
var del = (Func<T1, int>) method.CreateDelegate(typeof(Func<T1, int>));
125+
return message => del((T1) message);
126+
}
127+
else
128+
{
129+
// On some runtimes (e.g. old Mono) the return type has to be exactly correct,
130+
// so we go via boxing. Reflection is already fairly inefficient, and this is
131+
// only used for one-of case checking, fortunately.
132+
var del = (Func<T1, T2>) method.CreateDelegate(typeof(Func<T1, T2>));
133+
return message => (int) (object) del((T1) message);
134+
}
135+
}
136+
137+
public Action<IMessage> CreateActionIMessage(MethodInfo method)
138+
{
139+
var del = (Action<T1>) method.CreateDelegate(typeof(Action<T1>));
140+
return message => del((T1) message);
141+
}
142+
143+
public Func<IMessage, object> CreateFuncIMessageObject(MethodInfo method)
144+
{
145+
var del = (Func<T1, T2>) method.CreateDelegate(typeof(Func<T1, T2>));
146+
return message => del((T1) message);
147+
}
148+
149+
public Action<IMessage, object> CreateActionIMessageObject(MethodInfo method)
150+
{
151+
var del = (Action<T1, T2>) method.CreateDelegate(typeof(Action<T1, T2>));
152+
return (message, arg) => del((T1) message, (T2) arg);
153+
}
154+
}
155+
156+
// Runtime compatibility checking code - see ReflectionHelper<T1, T2>.CreateFuncIMessageInt32 for
157+
// details about why we're doing this.
158+
159+
// Deliberately not inside the generic type. We only want to check this once.
160+
private static bool CanConvertEnumFuncToInt32Func { get; } = CheckCanConvertEnumFuncToInt32Func();
161+
162+
private static bool CheckCanConvertEnumFuncToInt32Func()
163+
{
164+
try
165+
{
166+
MethodInfo method = typeof(ReflectionUtil).GetMethod(nameof(SampleEnumMethod));
167+
// If this passes, we're in a reasonable runtime.
168+
method.CreateDelegate(typeof(Func<int>));
169+
return true;
170+
}
171+
catch (ArgumentException)
172+
{
173+
return false;
174+
}
175+
}
176+
177+
public enum SampleEnum
178+
{
179+
X
180+
}
181+
182+
// Public to make the reflection simpler.
183+
public static SampleEnum SampleEnumMethod() => SampleEnum.X;
106184
}
107-
}
185+
}

0 commit comments

Comments
 (0)