dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.96k stars 4.65k forks source link

JIT: reconsider inlining methods that must throw #101777

Open AndyAyersMS opened 4 months ago

AndyAyersMS commented 4 months ago

Current ASP.NET spmi collection method 89486:

Inlines into 06000000 [via ExtendedDefaultPolicy] System.Net.Sockets.Socket+AwaitableSocketAsyncEventArgs:System.Threading.Tasks.Sources.IValueTaskSource.GetResult(short):this:
  [INL01 IL=0007 TR=000003 06000000] [INLINED: callee: below ALWAYS_INLINE size] System.Threading.Tasks.Sources.ManualResetValueTaskSourceCore`1[ubyte]:get_Version():short:this
  [INL00 IL=0014 TR=000026 06000000] [FAILED: callee: does not return] System.Net.Sockets.Socket+AwaitableSocketAsyncEventArgs:ThrowIncorrectTokenException()
  [INL02 IL=0020 TR=000008 06000000] [INLINED: callee: below ALWAYS_INLINE size] System.Net.Sockets.SocketAsyncEventArgs:get_SocketError():int:this
  [INL03 IL=0034 TR=000016 06000000] [INLINED: call site: profitable inline] System.Net.Sockets.Socket+AwaitableSocketAsyncEventArgs:ReleaseForAsyncCompletion():this
    [INL04 IL=0018 TR=000040 06000000] [INLINED: call site: profitable inline] System.Threading.Tasks.Sources.ManualResetValueTaskSourceCore`1[ubyte]:Reset():this
    [INL05 IL=0024 TR=000042 06000000] [INLINED: callee: aggressive inline attribute] System.Net.Sockets.Socket+AwaitableSocketAsyncEventArgs:ReleaseForSyncCompletion():this
      [INL06 IL=0036 TR=000095 06000000] [INLINED: callee: aggressive inline attribute] System.Threading.Interlocked:CompareExchange[System.__Canon](byref,System.__Canon,System.__Canon):System.__Canon
        [INL07 IL=0003 TR=000113 06000000] [INLINED: callee: aggressive inline attribute] System.Threading.Interlocked:CompareExchange(byref,System.Object,System.Object):System.Object
          [INL00 IL=0008 TR=000124 06000000] [FAILED: callee: does not return] System.ThrowHelper:ThrowNullReferenceException()
      [INL00 IL=0044 TR=000102 06000000] [FAILED: call site: unprofitable inline] System.Net.Sockets.SocketAsyncEventArgs:Dispose():this
  ** [INL08 IL=0045 TR=000025 06000000] [INLINED: call site: profitable inline] System.Net.Sockets.Socket+AwaitableSocketAsyncEventArgs:ThrowException(int,System.Threading.CancellationToken):this **
    [INL09 IL=0027 TR=000140 06000000] [INLINED: callee: below ALWAYS_INLINE size] System.Threading.CancellationToken:ThrowIfCancellationRequested():this
      [INL10 IL=0001 TR=000150 06000000] [INLINED: call site: profitable inline] System.Threading.CancellationToken:get_IsCancellationRequested():ubyte:this
        [INL11 IL=0014 TR=000171 06000000] [INLINED: callee: below ALWAYS_INLINE size] System.Threading.CancellationTokenSource:get_IsCancellationRequested():ubyte:this
      [INL00 IL=0009 TR=000156 06000000] [FAILED: call site: unprofitable inline] System.Threading.CancellationToken:ThrowOperationCanceledException():this
    [INL00 IL=0035 TR=000137 06000000] [FAILED: callee: noinline per IL/cached result] <unknown method>

The no return inhibition in inlining only kicks in for callees with one basic block; here we have 7:

BB01 [0021]  1                           100    [000..008)-> BB03(0.5),BB02(0.5)     ( cond )                     
BB02 [0022]  1       BB01                100    [008..010)-> BB04(0.5),BB03(0.5)     ( cond )                     
BB03 [0023]  2       BB01,BB02           100    [010..014)-> BB05(1)                 (always)                     
BB04 [0024]  1       BB02                100    [014..016)-> BB05(1)                 (always)                     
BB05 [0025]  2       BB03,BB04           100    [016..019)-> BB07(0),BB06(1)         ( cond )                     
BB06 [0026]  1       BB05                100    [019..020)-> BB07(1)                 (always)                     
BB07 [0027]  2       BB05,BB06             0    [020..029)                           (throw )                     rare

but I think we should reconsider this and perhaps disable inlining unless the callee method has a backwards branch (that is, if it explicitly can loop).

Note the profile above is messed up, later on synthesis changes it to be

BB01 [0021]  1                           100    100 [000..008)-> BB03(1),BB02(0)         ( cond )                     IBC
BB02 [0022]  1       BB01                  0      0 [008..010)-> BB04(0.48),BB03(0.52)   ( cond )                     IBC rare
BB03 [0023]  2       BB01,BB02           100    100 [010..014)-> BB05(1)                 (always)                     IBC
BB04 [0024]  1       BB02                  0      0 [014..016)-> BB05(1)                 (always)                     IBC rare
BB05 [0025]  2       BB03,BB04           100    100 [016..019)-> BB07(0),BB06(1)         ( cond )                     IBC
BB06 [0026]  1       BB05                100    100 [019..020)-> BB07(1)                 (always)                     IBC
BB07 [0027]  2       BB05,BB06           100    100 [020..029)                           (throw )                     IBC

which is self-consistent but arbitrary -- it is not yet very smart about handling must throws either.

dotnet-policy-service[bot] commented 4 months ago

Tagging subscribers to this area: @JulieLeeMSFT, @jakobbotsch See info in area-owners.md if you want to be subscribed.

AndyAyersMS commented 4 months ago

Seems to be dependent on https://github.com/dotnet/runtime/pull/101739

AndyAyersMS commented 4 months ago

@egorbo above is merged so you ought to be able to repro this...