Repository navigation
Calls to Interlocked.CompareExchange<T> prevents inlining #8662
Copy link
Copy link
Closed
Labels
area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMICLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMIenhancementProduct code improvement that does NOT require public API changes/additionsProduct code improvement that does NOT require public API changes/additionsin-prThere is an active PR which will close this issue when it is mergedThere is an active PR which will close this issue when it is mergedoptimizationtenet-performancePerformance related issuePerformance related issue
Milestone
Description
This is the smaller code that showcase the issue. This code is working very hard to avoid cache issues but still have to pay for the call because it refuses to inline in CoreCLR 1.1 and CoreCLR 2.0 (1w old build).
I may be missing something, but dont see any reason why it shouldn't inline.
using System;
using System.Runtime.CompilerServices;
using System.Runtime.InteropServices;
using System.Threading;
namespace ConsoleApp1
{
class Program
{
static void Main(string[] args)
{
var p = new Program<object>();
p.Test();
Console.WriteLine("Hello World!");
}
}
public class Program<T> where T : class, new()
{
[StructLayout(LayoutKind.Sequential, Size = 128)]
private struct CacheAwareElement
{
private readonly long _pad1;
private T Value;
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static bool TryClaim(ref CacheAwareElement bucket, out T item)
{
// Note that the initial read is optimistically not synchronized. That is intentional.
// We will interlock only when we have a candidate. in a worst case we may miss some
// recently returned objects. Not a big deal.
T inst = bucket.Value;
if (inst != null && inst == Interlocked.CompareExchange(ref bucket.Value, null, inst))
goto Done;
item = null;
return false;
Done:
item = inst;
return true;
}
}
private CacheAwareElement _element;
[MethodImpl(MethodImplOptions.NoInlining)]
public T Test()
{
T instance;
CacheAwareElement.TryClaim(ref _element, out instance);
return instance;
}
}
}cc @AndyAyersMS
category:cq
theme:inlining
skill-level:expert
cost:medium
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMICLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMIenhancementProduct code improvement that does NOT require public API changes/additionsProduct code improvement that does NOT require public API changes/additionsin-prThere is an active PR which will close this issue when it is mergedThere is an active PR which will close this issue when it is mergedoptimizationtenet-performancePerformance related issuePerformance related issue
Activity
mikedn commentedon Jul 31, 2017
Inlining isn't blocked by CompareExchange but by runtime dictionary lookup that's needed in the generic code.
redknightlois commentedon Jul 31, 2017
I would have never expect a dictionary lookup underlying
Interlocked.CompareExchangewhen T is class and therefore known to be 8 bytes on 64 bits (or 4 bytes when 32bits architecture).mikedn commentedon Jul 31, 2017
It is exactly when
Tis a class that you do get runtime lookups, that's how shared generic code works. But yes, the JIT could probably try a bit harder in the specific case ofInterlocked.CompareExchange<T>. Probably it could make it look like a call toInterlocked.CompareExchange(ref object, object object)so runtime lookup is not required.For example, if you change your code to not be generic you get the following code for
Test:AndyAyersMS commentedon Jul 31, 2017
The inlines done are:
You often can't inline from one shared generic class into another, so
Program<T>andCacheAwareElement<T>are the culprits blocking the inline ofTryClaim. The jit cannot foresee that if it did this inline and then the second-level inline ofCompareExchange, the runtime lookups would not be required or would go unused.Making the inner class non-generic or instantiating the outer class over a value type (eg via the struct wrapped ref class trick if you really wanted a ref class all along) would likely unblock this case.
From the second inline you can see that interlocked compare exchange is inlineable (and there's already a magic transformation to call the non-generic version thanks to IL intrinsics).
The inlined version calls into native code so there's nothing more the jit can do here. Even if all the inlines were unblocked you would still see a call to do the compare exchange.
I haven't verified this, and my understanding of this part of the runtime is sketchy, but the fact that
TryClaimthinks it needs a runtime lookup to callCompareExchangemay come from fact that the runtime's code-sharing analysis reflects the IL in the original method body forCompareExchangeand not the less demanding IL intrinsic version. Since we don't ever expect the original body to be used it might be interesting to replace it with something more minimal and see if that also unblocks things.mikedn commentedon Jul 31, 2017
Tried, same result.
redknightlois commentedon Jul 31, 2017
No luck either.
AndyAyersMS commentedon Nov 25, 2020
This is a runtime restriction so not much the jit can do here until there is runtime support.
@davidwrighton has prototyped some changes that would unblock such cases (#31833).
AndyAyersMS commentedon Aug 16, 2023
Nothing much has changed here. @EgorBo weren't you working on some changes to remove the shared-shared inlining restrictions?
EgorBo commentedon Aug 16, 2023
Perhaps, for .NET 9.0 🙂