Skip to content

Don't expand pinvokes in Tier0 - #77545

Closed
EgorBo wants to merge 1 commit into
dotnet:mainfrom
EgorBo:dont-expand-pinvokes-tier0
Closed

Don't expand pinvokes in Tier0#77545
EgorBo wants to merge 1 commit into
dotnet:mainfrom
EgorBo:dont-expand-pinvokes-tier0

Conversation

@EgorBo

Copy link
Copy Markdown
Member
GetId(false);staticintGetId(boolcondition){if(condition)returnGetCurrentThreadId();return0;}[DllImport("kernel32.dll")]staticexternintGetCurrentThreadId();

Current Tier0 codegen for GetID:

; Assembly listing for method Program:GetId(bool):int; Tier-0 compilation; MinOpts codeG_M000_IG01: ;; offset=0000H55pushrbp4157pushr154156pushr144155pushr134154pushr1257pushrdi56pushrsi53pushrbx 4883EC78 subrsp,120 488DAC24B0000000 learbp,[rsp+B0H] 894D10 mov dword ptr [rbp+10H],ecxG_M000_IG02: ;; offset=001BH 488D8D78FFFFFF learcx,[rbp-88H] 498BD2 movrdx,r10 E81628A55F call CORINFO_HELP_INIT_PINVOKE_FRAME 488945B8 mov qword ptr [rbp-48H],rax 488BC4 movrax,rsp48894598mov qword ptr [rbp-68H],rax 488BC5 movrax,rbp 488945A8 mov qword ptr [rbp-58H],rax 8B4510 moveax, dword ptr [rbp+10H] 0FB6C0 movzxrax,al 85C0 testeax,eax 745E je SHORT G_M000_IG06 48B8F0ECDC5BF87F0000 movrax,0x7FF85BDCECF048894588mov qword ptr [rbp-78H],rax 488D0521000000 learax, G_M000_IG04 488945A0 mov qword ptr [rbp-60H],rax 488B45B8 movrax, qword ptr [rbp-48H] 488D9578FFFFFF leardx, bword ptr [rbp-88H]48895010mov qword ptr [rax+10H],rdx 488B45B8 movrax, qword ptr [rbp-48H] C6400C00 mov byte ptr [rax+0CH],0G_M000_IG03: ;; offset=0076H FF15E4E50B00 call[Program:GetCurrentThreadId():int]G_M000_IG04: ;; offset=007CH 488B55B8 movrdx, qword ptr [rbp-48H] C6420C01 mov byte ptr [rdx+0CH],1 833D4903E35F00 cmp dword ptr [(reloc 0x7ff8bbb40ab4)],07406je SHORT G_M000_IG05 FF15052CE25F call[CORINFO_HELP_STOP_FOR_GC]G_M000_IG05: ;; offset=0093H 488B55B8 movrdx, qword ptr [rbp-48H] 488B4D80 movrcx, bword ptr [rbp-80H] 48894A10 mov qword ptr [rdx+10H],rcx 8945C4 mov dword ptr [rbp-3CH],eax EB05 jmp SHORT G_M000_IG07G_M000_IG06: ;; offset=00A4H 33C0 xoreax,eax 8945C4 mov dword ptr [rbp-3CH],eaxG_M000_IG07: ;; offset=00A9H 8B45C4 moveax, dword ptr [rbp-3CH]G_M000_IG08: ;; offset=00ACH 4883C478 addrsp,120 5B poprbx 5E poprsi 5F poprdi 415C popr12 415D popr13 415E popr14 415F popr15 5D poprbp C3 ret; Total bytes of code 189

New Tier0 codegen for GetID:

; Assembly listing for method Program:GetId(bool):int; Tier-0 compilation; MinOpts codeG_M31913_IG01: ;; offset=0000H55pushrbp 4883EC20 subrsp,32 488D6C2420 learbp,[rsp+20H] 894D10 mov dword ptr [rbp+10H],ecxG_M31913_IG02: ;; offset=000DH 8B4510 moveax, dword ptr [rbp+10H] 0FB6C0 movzxrax,al 85C0 testeax,eax 740C je SHORT G_M31913_IG04 E8D47BE7FF call Program:GetCurrentThreadId():int90nopG_M31913_IG03: ;; offset=001DH 4883C420 addrsp,32 5D poprbp C3 retG_M31913_IG04: ;; offset=0023H 33C0 xoreax,eaxG_M31913_IG05: ;; offset=0025H 4883C420 addrsp,32 5D poprbp C3 ret; Total bytes of code 43

-146 bytes of codegen for an used codepath. If it turns out to be used an IL_STUB_PInvoke will be compiled on demand.

Pros:

  • We spend less time in Tier0 if it's not used
  • We don't invoke that common prologue overhead if it the code path with pinvoke is not used
  • IL_STUB_Pinvoke can already be compiled by someone else so we won't have to jit it again if it's used

Cons:

  • If the codpath is used we pay FullOpts prices compiling it (if it's not compiled yet).

Overall I think it's a reasonable tradeoff

PTAL @dotnet/jit-contrib

@ghostghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Oct 27, 2022
@ghostghost assigned EgorBoOct 27, 2022
@ghost

Copy link
Copy Markdown

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

Issue Details
GetId(false);staticintGetId(boolcondition){if(condition)returnGetCurrentThreadId();return0;}[DllImport("kernel32.dll")]staticexternintGetCurrentThreadId();

Current Tier0 codegen for GetID:

; Assembly listing for method Program:GetId(bool):int; Tier-0 compilation; MinOpts codeG_M000_IG01: ;; offset=0000H55pushrbp4157pushr154156pushr144155pushr134154pushr1257pushrdi56pushrsi53pushrbx 4883EC78 subrsp,120 488DAC24B0000000 learbp,[rsp+B0H] 894D10 mov dword ptr [rbp+10H],ecxG_M000_IG02: ;; offset=001BH 488D8D78FFFFFF learcx,[rbp-88H] 498BD2 movrdx,r10 E81628A55F call CORINFO_HELP_INIT_PINVOKE_FRAME 488945B8 mov qword ptr [rbp-48H],rax 488BC4 movrax,rsp48894598mov qword ptr [rbp-68H],rax 488BC5 movrax,rbp 488945A8 mov qword ptr [rbp-58H],rax 8B4510 moveax, dword ptr [rbp+10H] 0FB6C0 movzxrax,al 85C0 testeax,eax 745E je SHORT G_M000_IG06 48B8F0ECDC5BF87F0000 movrax,0x7FF85BDCECF048894588mov qword ptr [rbp-78H],rax 488D0521000000 learax, G_M000_IG04 488945A0 mov qword ptr [rbp-60H],rax 488B45B8 movrax, qword ptr [rbp-48H] 488D9578FFFFFF leardx, bword ptr [rbp-88H]48895010mov qword ptr [rax+10H],rdx 488B45B8 movrax, qword ptr [rbp-48H] C6400C00 mov byte ptr [rax+0CH],0G_M000_IG03: ;; offset=0076H FF15E4E50B00 call[Program:GetCurrentThreadId():int]G_M000_IG04: ;; offset=007CH 488B55B8 movrdx, qword ptr [rbp-48H] C6420C01 mov byte ptr [rdx+0CH],1 833D4903E35F00 cmp dword ptr [(reloc 0x7ff8bbb40ab4)],07406je SHORT G_M000_IG05 FF15052CE25F call[CORINFO_HELP_STOP_FOR_GC]G_M000_IG05: ;; offset=0093H 488B55B8 movrdx, qword ptr [rbp-48H] 488B4D80 movrcx, bword ptr [rbp-80H] 48894A10 mov qword ptr [rdx+10H],rcx 8945C4 mov dword ptr [rbp-3CH],eax EB05 jmp SHORT G_M000_IG07G_M000_IG06: ;; offset=00A4H 33C0 xoreax,eax 8945C4 mov dword ptr [rbp-3CH],eaxG_M000_IG07: ;; offset=00A9H 8B45C4 moveax, dword ptr [rbp-3CH]G_M000_IG08: ;; offset=00ACH 4883C478 addrsp,120 5B poprbx 5E poprsi 5F poprdi 415C popr12 415D popr13 415E popr14 415F popr15 5D poprbp C3 ret; Total bytes of code 189

New Tier0 codegen for GetID:

; Assembly listing for method Program:GetId(bool):int; Tier-0 compilation; MinOpts codeG_M31913_IG01: ;; offset=0000H55pushrbp 4883EC20 subrsp,32 488D6C2420 learbp,[rsp+20H] 894D10 mov dword ptr [rbp+10H],ecxG_M31913_IG02: ;; offset=000DH 8B4510 moveax, dword ptr [rbp+10H] 0FB6C0 movzxrax,al 85C0 testeax,eax 740C je SHORT G_M31913_IG04 E8D47BE7FF call Program:GetCurrentThreadId():int90nopG_M31913_IG03: ;; offset=001DH 4883C420 addrsp,32 5D poprbp C3 retG_M31913_IG04: ;; offset=0023H 33C0 xoreax,eaxG_M31913_IG05: ;; offset=0025H 4883C420 addrsp,32 5D poprbp C3 ret; Total bytes of code 43

-146 bytes of codegen for an used codepath. If it turns out to be used an IL_STUB_PInvoke will be compiled on demand.

Pros:

  • We spend less time in Tier0 if it's not used
  • We don't invoke that common prologue overhead if it the code path with pinvoke is not used
  • IL_STUB_Pinvoke can already be compiled by someone else so we won't have to jit it again if it's used

Cons:

  • If the codpath is used we pay FullOpts prices compiling it (if it's not compiled yet).

Overall I think it's a reasonable tradeoff

PTAL @dotnet/jit-contrib

Author:EgorBo
Assignees:EgorBo
Labels:

area-CodeGen-coreclr

Milestone:-

@BruceForstall

Copy link
Copy Markdown
Contributor

cc @AaronRobinsonMSFT

@AaronRobinsonMSFT

Copy link
Copy Markdown
Member

@EgorBo This could have an impact on startup paths for users with many single use and blittable signatures. I assume when we hit tier 1, the inlining will occur, which is good. However, for a Release build blittable P/Invokes have been inlined for a long time, changing that is something to be leery of.

Can you check if there is a behavioral change when the export of the library isn't found?

@jkotas

Copy link
Copy Markdown
Member

Do you some numbers for the cost of pros and cons in some real apps?

@davidwrighton

Copy link
Copy Markdown
Member

If we're going to measure things, I'd like to see 3 measurements

  1. No pinvoke expansion
  2. What we do today
  3. Tier-0 expands pinvokes, but uses the helper methods instead of the inline expansion of everything. (This technique does not work for IL stubs, but works for normal pinvoke expansion.) This may be a really nice middle ground here.

I'm concerned about this change, and am in particular concerned about the startup time impact on things like winforms, as

  1. As I understand, IL Stubs do not currently participate in tiering, so they are always compiled as tier 1.
  2. Winforms, and other partners have spent extensive amounts of time working on making pinvoke apis blittable, so as to improve startup performance. This change has a risk of regressing all of that work.

@EgorBo

EgorBo commented Oct 27, 2022

Copy link
Copy Markdown
MemberAuthor

Can you check if there is a behavioral change when the export of the library isn't found?

Looks like we don't throw DllNotFoundException out of JIT when we inline a pinvoke do we? I've just tested it on this sample and it runs without exceptions (with and without the PR):

publicclassProgram{publicstaticvoidMain(string[]args){if(args.Length==42)Test();}[DllImport("non-existing-lib.dll")]staticexternintTest();}

I'm concerned about this change
Do you some numbers for the cost of pros and cons in some real apps?

Ok, I'm closing this for now and if I have data I'll reopen. It's something I found in #77465 and decided to push separately to raise a discussion. Will workaround in #77465 to keep the current behavior.

@EgorBoEgorBo closed this Oct 27, 2022
@ghostghost locked as resolved and limited conversation to collaborators Nov 27, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

area-CodeGen-coreclrCLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants

@EgorBo@BruceForstall@AaronRobinsonMSFT@jkotas@davidwrighton