Skip to content
This repository was archived by the owner on Jan 23, 2023. It is now read-only.

Streamline RuntimeMethodInfo.Invoke#21847

Closed
benaadams wants to merge 2 commits intodotnet:masterfrom
benaadams:RuntimeMethodInfo.Invoke
Closed

Streamline RuntimeMethodInfo.Invoke#21847
benaadams wants to merge 2 commits intodotnet:masterfrom
benaadams:RuntimeMethodInfo.Invoke

Conversation

@benaadams
Copy link
Copy Markdown
Member

@benaadams benaadams commented Jan 7, 2019

Shaves a little off RuntimeMethodInfo.Invoke for startup (used when looking up CustomAttributes for EventSources that don't use the Guid .ctor)

image

There's not a lot to do here, assuming the defensive copy of params in CheckArguments is required (checked set of params, used to invoke method detached from user modification between the call to invoke and the invoke)

Could introduce an UnsafeInvoke that doesn't do a defensive copy for the runtime; however not sure there are that many paths that would call it; and since its only a single element object[] being created probably wouldn't save much (method is 21% of .Invoke).

image

@benaadams
Copy link
Copy Markdown
Member Author

Creating an UnsafeInvoke method doesn't give much back

@benaadams
Copy link
Copy Markdown
Member Author

benaadams commented Jan 7, 2019

@AndyAyersMS @mikedn bit weird this code change in PR to MethodBase.CheckArguments cause the loop body to repeat twice (whereas it wasn't prior)

I've only seen this by finally cloning before, but there is no error handling here?

Code

internal object[] CheckArguments(object[] parameters, Binder binder,
    BindingFlags invokeAttr, CultureInfo culture, Signature sig)
{
    // copy the arguments in a different array so we detach from any user changes 
    object[] copyOfParameters = new object[parameters.Length];

    ParameterInfo[] p = null;
    RuntimeType[] runtimeTypes = sig.Arguments;
    for (int i = 0; i < parameters.Length; i++)
    {
        object arg = parameters[i];
        RuntimeType argRT = runtimeTypes[i];

        if (arg == Type.Missing)
        {
            if (p is null)
            {
                p = GetParametersNoCopy();
            }

            object defaultValue = p[i].DefaultValue;
            if (defaultValue == DBNull.Value)
            {
                throw new ArgumentException(SR.Arg_VarMissNull, nameof(parameters));
            }

            arg = defaultValue;
        }

        copyOfParameters[i] = argRT.CheckValue(arg, binder, culture, invokeAttr);
    }

    return copyOfParameters;
}

Asm

; Assembly listing for method MethodBase:CheckArguments(ref,ref,int,ref,ref):ref:this
; Emitting BLENDED_CODE for X64 CPU with SSE2 - Windows
; optimized code
; rsp based frame
; partially interruptible
; Final local variable assignments
;
;  V00 this         [V00,T10] (  6, 10   )     ref  ->  rdi         this class-hnd
;  V01 arg1         [V01,T08] (  5, 11   )     ref  ->  rsi         class-hnd
;  V02 arg2         [V02,T13] (  4, 10   )     ref  ->  rbx         class-hnd
;  V03 arg3         [V03,T14] (  4, 10   )     int  ->  rbp        
;  V04 arg4         [V04,T16] (  2,  8   )     ref  ->  r14         class-hnd
;  V05 arg5         [V05,T17] (  1,  1   )     ref  ->  [rsp+0xD8]   class-hnd
;  V06 loc0         [V06,T15] (  4, 10   )     ref  ->  r12         class-hnd
;  V07 loc1         [V07,T04] (  9, 17   )     ref  ->  r13         class-hnd
;  V08 loc2         [V08,T06] (  6, 15   )     ref  ->  [rsp+0x50]   class-hnd
;  V09 loc3         [V09,T00] ( 18, 61   )     int  ->  [rsp+0x64]  
;  V10 loc4         [V10,T02] (  8, 28   )     ref  ->  [rsp+0x48]   class-hnd
;  V11 loc5         [V11,T05] (  4, 16   )     ref  ->  [rsp+0x40]   class-hnd
;  V12 loc6         [V12,T11] (  6, 12   )     ref  ->  [rsp+0x38]   class-hnd
;  V13 OutArgs      [V13    ] (  1,  1   )  lclBlk (40) [rsp+0x00]   "OutgoingArgSpace"
;  V14 tmp1         [V14,T18] (  7,  0   )     ref  ->  rsi         class-hnd exact "NewObj constructor temp"
;  V15 tmp2         [V15,T19] (  2,  0   )     ref  ->  rdi         class-hnd "Inlining Arg"
;  V16 tmp3         [V16,T20] (  2,  0   )     ref  ->  rcx         "argument with side effect"
;  V17 tmp4         [V17,T01] (  4, 32   )     ref  ->  rax         "argument with side effect"
;  V18 cse0         [V18,T09] (  4, 13   )   byref  ->  [rsp+0x30]   "ValNumCSE"
;  V19 cse1         [V19,T07] (  6, 14   )    long  ->  [rsp+0x58]   "ValNumCSE"
;  V20 cse2         [V20,T12] (  6, 12   )     int  ->  r15         "ValNumCSE"
;  V21 rat0         [V21,T03] (  6, 24   )     ref  ->  rcx         "virtual vtable call"
;
; Lcl frame size = 104
G_M57801_IG01:
       push     r15
       push     r14
       push     r13
       push     r12
       push     rdi
       push     rsi
       push     rbp
       push     rbx
       sub      rsp, 104
       mov      rdi, rcx
       mov      rsi, rdx
       mov      rbx, r8
       mov      ebp, r9d
       mov      r14, gword ptr [rsp+D0H]
G_M57801_IG02:
       mov      r15d, dword ptr [rsi+8]
       movsxd   rdx, r15d
       lea      rcx, [(reloc)]
       call     CORINFO_HELP_NEWARR_1_OBJ
       mov      r12, rax
       xor      r13, r13
       mov      rcx, gword ptr [rsp+D8H]
       mov      rax, gword ptr [rcx+8]
       xor      r8d, r8d
       test     r15d, r15d
       jle      G_M57801_IG12
       test     rax, rax
       setne    cl
       movzx    rcx, cl
       test     cl, 1
       je       G_M57801_IG08
G_M57801_IG03:
       cmp      dword ptr [rax+8], r15d
       jl       G_M57801_IG08
       mov      dword ptr [rsp+64H], r8d
       mov      gword ptr [rsp+50H], rax
       mov      rcx, qword ptr [(reloc)]
       mov      r9, rcx
       mov      qword ptr [rsp+58H], r9
       mov      rcx, r9
       mov      edx, 139
       call     CORINFO_HELP_GETSHARED_GCSTATIC_BASE
G_M57801_IG04:
       mov      edx, dword ptr [rsp+64H]
       movsxd   rcx, edx
       mov      rcx, gword ptr [rsi+8*rcx+16]
       mov      r8, rcx
       mov      dword ptr [rsp+64H], edx
       movsxd   rcx, edx
       mov      r9, gword ptr [rsp+50H]
       mov      rcx, gword ptr [r9+8*rcx+16]
       mov      gword ptr [rsp+40H], rcx
       mov      bword ptr [rsp+30H], rax
       cmp      gword ptr [rax+248], r8
       mov      r10, gword ptr [rsp+40H]
       jne      SHORT G_M57801_IG07
       test     r13, r13
       mov      gword ptr [rsp+40H], r10
       jne      SHORT G_M57801_IG05
       mov      rcx, rdi
       mov      r8, qword ptr [rdi]
       mov      r8, qword ptr [r8+80]
       call     gword ptr [r8+16]MethodBase:GetParametersNoCopy():ref:this
       mov      r13, rax
G_M57801_IG05:
       mov      edx, dword ptr [rsp+64H]
       cmp      edx, dword ptr [r13+8]
       jae      G_M57801_IG15
       mov      dword ptr [rsp+64H], edx
       movsxd   rcx, edx
       mov      rcx, gword ptr [r13+8*rcx+16]
       mov      r8, qword ptr [rcx]
       mov      r8, qword ptr [r8+72]
       call     gword ptr [r8+8]ParameterInfo:get_DefaultValue():ref:this
       mov      gword ptr [rsp+38H], rax
       mov      rcx, qword ptr [rsp+58H]
       mov      edx, 206
       call     CORINFO_HELP_GETSHARED_GCSTATIC_BASE
       mov      r8, gword ptr [rsp+38H]
       cmp      gword ptr [rax+504], r8
       je       G_M57801_IG14
G_M57801_IG06:
       mov      rdx, r8
       mov      r8, rdx
       mov      r10, gword ptr [rsp+40H]
G_M57801_IG07:
       mov      dword ptr [rsp+20H], ebp
       mov      rcx, r10
       mov      rdx, r8
       mov      r8, rbx
       mov      r9, r14
       cmp      dword ptr [rcx], ecx
       call     RuntimeType:CheckValue(ref,ref,ref,int):ref:this
       mov      r8, rax
       mov      edx, dword ptr [rsp+64H]
       mov      rcx, r12
       call     CORINFO_HELP_ARRADDR_ST
       mov      ecx, dword ptr [rsp+64H]
       inc      ecx
       cmp      r15d, ecx
       mov      dword ptr [rsp+64H], ecx
       mov      rax, bword ptr [rsp+30H]
       jg       G_M57801_IG04
       jmp      G_M57801_IG12
G_M57801_IG08:
       movsxd   rcx, r8d
       mov      r9, gword ptr [rsi+8*rcx+16]
       mov      gword ptr [rsp+48H], r9
       cmp      r8d, dword ptr [rax+8]
       jae      G_M57801_IG15
       mov      dword ptr [rsp+64H], r8d
       movsxd   rcx, r8d
       mov      gword ptr [rsp+50H], rax
       mov      r10, gword ptr [rax+8*rcx+16]
       mov      gword ptr [rsp+40H], r10
       mov      r11, qword ptr [(reloc)]
       mov      qword ptr [rsp+58H], r11
       mov      rcx, r11
       mov      edx, 139
       call     CORINFO_HELP_GETSHARED_GCSTATIC_BASE
       mov      rdx, gword ptr [rsp+48H]
       cmp      gword ptr [rax+248], rdx
       jne      SHORT G_M57801_IG11
       test     r13, r13
       jne      SHORT G_M57801_IG09
       mov      rcx, rdi
       mov      rax, qword ptr [rdi]
       mov      rax, qword ptr [rax+80]
       call     gword ptr [rax+16]MethodBase:GetParametersNoCopy():ref:this
       mov      r13, rax
G_M57801_IG09:
       mov      eax, dword ptr [rsp+64H]
       cmp      eax, dword ptr [r13+8]
       jae      G_M57801_IG15
       mov      dword ptr [rsp+64H], eax
       movsxd   rcx, eax
       mov      rcx, gword ptr [r13+8*rcx+16]
       mov      rdx, qword ptr [rcx]
       mov      rdx, qword ptr [rdx+72]
       call     gword ptr [rdx+8]ParameterInfo:get_DefaultValue():ref:this
       mov      gword ptr [rsp+38H], rax
       mov      rcx, qword ptr [rsp+58H]
       mov      edx, 206
       call     CORINFO_HELP_GETSHARED_GCSTATIC_BASE
       mov      rdx, gword ptr [rsp+38H]
       cmp      gword ptr [rax+504], rdx
       je       G_M57801_IG14
G_M57801_IG10:
       mov      rax, rdx
       mov      rdx, rax
G_M57801_IG11:
       mov      dword ptr [rsp+20H], ebp
       mov      rcx, gword ptr [rsp+40H]
       mov      r8, rbx
       mov      r9, r14
       cmp      dword ptr [rcx], ecx
       call     RuntimeType:CheckValue(ref,ref,ref,int):ref:this
       mov      r8, rax
       mov      edx, dword ptr [rsp+64H]
       mov      rcx, r12
       call     CORINFO_HELP_ARRADDR_ST
       mov      r8d, dword ptr [rsp+64H]
       inc      r8d
       cmp      r15d, r8d
       mov      rax, gword ptr [rsp+50H]
       jg       G_M57801_IG08
G_M57801_IG12:
       mov      rax, r12
G_M57801_IG13:
       add      rsp, 104
       pop      rbx
       pop      rbp
       pop      rsi
       pop      rdi
       pop      r12
       pop      r13
       pop      r14
       pop      r15
       ret      
************** Beginning of cold code **************
G_M57801_IG14:
       lea      rcx, [(reloc)]
       call     CORINFO_HELP_NEWSFAST
       mov      rsi, rax
       mov      ecx, 0x3587
       call     CORINFO_HELP_STRCNS_CURRENT_MODULE
       mov      rcx, rax
       xor      rdx, rdx
       call     SR:GetResourceString(ref,ref):ref
       mov      rdi, rax
       mov      rcx, rsi
       call     Exception:.ctor():this
       lea      rcx, bword ptr [rsi+24]
       mov      rdx, rdi
       call     CORINFO_HELP_ASSIGN_REF
       mov      dword ptr [rsi+124], 0xD1FFAB1E
       mov      ecx, 0x12D8C
       call     CORINFO_HELP_STRCNS_CURRENT_MODULE
       lea      rcx, bword ptr [rsi+136]
       mov      rdx, rax
       call     CORINFO_HELP_ASSIGN_REF
       mov      dword ptr [rsi+124], 0xD1FFAB1E
       mov      rcx, rsi
       call     CORINFO_HELP_THROW
       int3     
G_M57801_IG15:
       call     CORINFO_HELP_RNGCHKFAIL
       int3     
; Total bytes of code 764, prolog size 16 for method MethodBase:CheckArguments(ref,ref,int,ref,ref):ref:this

@AndyAyersMS
Copy link
Copy Markdown
Member

Probably loop cloning kicking in... the heuristic there is not very well tuned. See #12836.

@benaadams
Copy link
Copy Markdown
Member Author

Ah, couldn't work out what it was testing on and both loops make all the same method calls.

Can see now that one loop has one less jmp to CORINFO_HELP_RNGCHKFAIL

@benaadams
Copy link
Copy Markdown
Member Author

Will need to play with this more; doubling the method size isn't ideal 😄

@benaadams benaadams closed this Jan 7, 2019
@mikedn
Copy link
Copy Markdown

mikedn commented Jan 7, 2019

FWIW you can avoid loop cloning by wrapping the array in a span. But unfortunately I don't know of a way to avoid a range check when dealing with parallel arrays.

@benaadams
Copy link
Copy Markdown
Member Author

FWIW you can avoid loop cloning by wrapping the array in a span. But unfortunately I don't know of a way to avoid a range check when dealing with parallel arrays.

Could do some upfront length checks + throws; then use Unsafe.Add. Would throw at the start rather than the n-th element, but that's ok.

I assume verifying all the arrays lengths are the same and throwing if not, wouldn't naturally propagate and allow regular indexing?

@mikedn
Copy link
Copy Markdown

mikedn commented Jan 7, 2019

then use Unsafe.Add

Well, yes, but I usually don't consider that a way to avoid range checks, more like a hack 😁

I assume verifying all the arrays lengths are the same and throwing if not, wouldn't naturally propagate and allow regular indexing?

I've tried a few things but I've yet to get that to work.

But in general, yes, when there's no way for the JIT to prove that indices are in range (it cannot prove that those 2 arrays have the same length) there may be a way to do some checks before the loop so that the loop doesn't contain any checks. Basically, it's a way to help the JIT hoist checks out of loop because it cannot by itself due to the "precise exceptions" requirement of the ECMA spec.

Something like this works like a charm when you want to traverse only a part of an array - make a span out of it and then loop other the span. All the range checks will be out of the loop.

But I don't know of a similar solution for parallel arrays.

@mikedn
Copy link
Copy Markdown

mikedn commented Jan 7, 2019

One nice solution would be something like

if (a.Length != b.Length) throwfor (int i = 0; i < a.Length; i++)
    b[i] = a[i];

It would be perfectly reasonable to put such a check in code. But the JIT doesn't get it unfortunately.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants