Allow to inline MethodGroup/Lamda directly passed as an argument #10049
Comments
|
This is similar to #7948 and we lack compelling examples. |
|
@AndyAyersMS As far as I can tell this is slightly different in that in requires to inline the 2 delegate calls as well. #7948 is basically a prerequisite. |
|
Yes, this issue is not about "lock". using System;
using System.Runtime.CompilerServices;
public class C {
public void InlineTest() {
// This method should not call anything
// Everything can be inlined
DoTwice(PrintHello);
//DoTwice(()=>{Console.WriteLine("bye");});
}
public void PrintHello()
{
Console.WriteLine("Hello");
}
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public void DoTwice(/*Probably with some hint here*/ Action a)
{
a();
a();
}
}And also not about double call. Double call just simplifies to see inlining in the x86 ASM code. |
|
I think this could be a big win in some situations. The inlining process already combines constants passed as parameters that are involved in arithmetic with other constants or other parameters passed as constants into a single constant value and that delegate is essentially passed as a constant method token so it could be treated similarly to eliminate the delegate invoke call. |
|
As far as I know, methods with delegate calls don't get inlined at all though, so I guess that would need to be addressed first. I can't find info on why that is though. Is that still the case? |
Yes, it seems odd. Something I'm looking into... |
|
I've experimentally unblocked inlining of methods with delegate invokes in the jit. Not surprisingly this leads to more inlining. I am not yet sure if this leads to correctness issues, but setting that aside for the moment, we can at least assess the likely code size impact: The jit is not accurately modelling the size expansion that comes with delegate invocation and so we're probably a bit too aggressive here. But there aren't many opportunities so perhaps it doesn't matter, except where stylistic issues lead to heavy use of delegates as seems to be happening in the CommandLine assembly. It appears from the above that the inlining of methods that invoke delegates rarely leads to knock-on optimizations. So at this point I still don't see great opportunity here. While it's true that locally constructed and consumed delegates should be optimizable, there are a number of other issues we need to sort through first before we can tackle that. |
|
I doubt there are many BCL opportunities here for optimization but for some library authors it could be very useful, even if only applied when aggressive inlining is requested. There are several places in my DB / entity library codebase that would benefit with this, particularly if/when the delegate call can be eliminated for a direct method call when passed as a constant method token argument. |
|
Another good use case is a parameter validation library we've standardized on that can take delegates for exception message generation, i.e: _count = count.ThrowIfOutOfRange(0, 10, nameof(count), GetLocalizedOutOfRangeMessage);Even if the delegate call isn't eliminated in the "exception" case that's fine as that should be rare, but we use these throw helpers for all validated parameters in all our methods which adds up so we would like them inlined to eliminate method call overhead. We currently can't use the delegate versions in performance-sensitive areas. Once again, this could be enabled only for methods marked for aggressive inlining as we do this proactively for such methods. |
|
Can you point me at an example...? I agree that jit-diff might not see enough stylistic variety. |
|
Sorry I'm not sure I understand, what would you like an example of? The parameter validator extension methods like the one above are probably the simplest and most obvious examples I can give. The other cases follow similar patterns, in the sense that they will also be inlinable if the example above is inlinable. Are you just asking me to expand the above example to a complete code example or do you want something else? |
Sorry for the confusion. I'd be grateful if you could point me at a complete code example. |
|
Hey sorry for the delay, got busy handling a few other priorities so I put this off. For this particular case, we implemented a simple workaround by refactoring the code that calls the delegate into a separate method. That works here as the delegate call is not on a hot path, though I haven't had a chance to fully analyze some of the other places where this might be desirable yet. I imagine the benefit of inlining a method that has a delegate call on a hot path is going to be much smaller, possibly negligible. It's not too painful to refactor out the delegate call (especially with local methods) in the very few places where this level of micro-optimization makes sense so this is less of an issue for me at this point. That said, a full example before refactoring would look something like this: public static class ThrowExtensions
{
[DebuggerHidden]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static string ThrowIfNullOrWhiteSpace(this string value, string paramName, Func<string> whitespaceMessage)
{
if (value == null)
throw new ArgumentNullException(paramName);
if (string.IsNullOrWhiteSpace(value))
throw new ArgumentException(whitespaceMessage?.Invoke(), paramName);
return value;
}
[DebuggerHidden]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static int ThrowIfOutOfRange(this int value, int min, int max, string paramName, Func<int, int, string> message)
{
if (value < min || value > max)
throw new ArgumentOutOfRangeException(paramName, message?.Invoke(min, max));
return value;
}
}
public class SeniorCitizen
{
public static int MinAllowedAge { get; set; } = 60;
public static int MaxAllowedAge { get; set; } = 120;
private string _name;
private int _age;
public string Name
{
get => _name;
set => _name = value.ThrowIfNullOrWhiteSpace(nameof(value), GetNameRequiredMessage);
}
public int Age
{
get => _age;
set => _age = value.ThrowIfOutOfRange(MinAllowedAge, MaxAllowedAge, nameof(value), GetAgeOutOfRangeMessage);
}
// logic to pull messages out of resource strings or whatever
private static string GetNameRequiredMessage();
private static string GetAgeOutOfRangeMessage(int min, int max);
}This is made up to simplify the example obviously. The performance for a case like this wouldn't need to be micro-optimized, but we have performance-sensitive code in our DB engine core that uses similar patterns. Refactored code looks like: [DebuggerHidden]
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static int ThrowIfOutOfRange(this int value, int min, int max, string paramName, Func<int, int, string> message)
{
if (value < min || value > max)
ThrowException(min, max, paramName, message);
return value;
// Note: workaround for delegate invoke preventing inlining and also reduces the size
// of code that is inlined, so leave this as a separate method.
static void ThrowException(int min, int max, string paramName, Func<int, int, string> message)
=> throw new ArgumentOutOfRangeException(paramName, message?.Invoke(min, max));
} |
|
Random side note: a neat optimization I noticed in the JIT assembly is that exception throwing code always seems to get shuffled to the bottom of methods so that non-exceptional execution flow doesn't need to jump...how cool is that? |
|
FYI: In microbenchmarks on .NET Core 3, the simplest throw extension methods (i.e. |
|
It is already possible to achieve functional inlining via JIT reification with generics+structs, but it is rather verbose code. // this top-level program compiles in .NET 5
C c = new();
c.DoWithGuardTwice<ConsoleWriteHelloWorld>();
interface IAction
{
void Invoke();
}
class C
{
public void DoWithGuardTwice<TAction>(TAction action = default)
where TAction : struct, IAction
{
lock (this)
{
action.Invoke();
action.Invoke();
}
}
}
struct ConsoleWriteHelloWorld : IAction
{
public void Invoke() => System.Console.WriteLine("hello world");
}Instead of using a delegate you use a pass in a struct that implements an But what about passing the delegate into a constructor? I have use cases where I would want that to be inlined too, but I don't know if that would be possible. For example, I have code that looks something like this: public class HeapArray<T, TCompare>
where TCompare : struct, IFunc<T, T, CompareResult>
{
internal readonly TCompare _compare;
internal T[] _array;
internal int _count;
public HeapArray(TCompare compare = default)
{
_compare = compare;
_array = new T[1];
_count = 0;
}
// code that eventually calls: _compare.Invoke
}and the public class HeapArray<T>
{
internal readonly Func<T, T, int> _compare;
internal T[] _array;
internal int _count;
public HeapArray(Func<T, T, int> compare)
{
_compare = compare;
_array = new T[1];
_count = 0;
}
// code that eventually calls: _compare.Invoke
}I don't know if it would be possible for I created this issue/discussion on csharplang which is an explicit syntax for this topic: dotnet/csharplang#2904 Would it be possible to inline delegate invocations that are passed into constructors and stored as
|
This is an example where inlining logically possible, but currently not supported by JIT:
https://sharplab.io/#v2:C4LghgzgtgPgAgBgARwIwG4CwAoRLUB0ASgK4B2wAllAKYEDCA9lAA6UA2NATgMrcBulAMY0IWbDjgBmFACYk9JAG8cOJOpQy4AFiQBJMu0pkaAFVHAAFAEplagJD2A9E6SmAFpQhJawd4wATJAh/EnYgskZgJCEwdnYkMDIATz9jAHM1DWyXJABRfm5UzzJ0mKSkACMaJGMjEwCs7PUAEUYAdUo/AHESMC4A0wB3YRpLAAUuY2AACRp4xmtxZo0m5pc2zp6+geHRyxsAXgA+JTQATksAIkrkmiulgF8ltce1tekUXUnpuYWbNYqbArdQXa5/diMB7LDRvYGreHqADaAFkaH5AnpWOxLGiMQEsSx2AB5FhURhkCAEACC6XSXFEEEohQM9Qy1gAuh8tLpNl13L1+oMRiJLE4AFSTRiVMCVdjJJAjPzBZg1ErRdzcGji1xoeRgayAtbZSFCADWljSEENiOaQJBzTANhhDvUTpetuycK92RwcKAA===
This is a complex type of inlining. As an alternative this type of inlining was proposed to be implemented at the C# compiler level: dotnet/csharplang#1413
category:cq
theme:inlining
skill-level:expert
cost:medium
The text was updated successfully, but these errors were encountered: