Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inline BufferWriter .ctor #7674

Merged
merged 1 commit into from
Feb 18, 2019
Merged

Conversation

benaadams
Copy link
Member

@benaadams benaadams commented Feb 18, 2019

Follow up to #7659

Was trying to track down some of these other WriteBarrier assigns in the traces where they didn't need to be there.

The BufferWriter .ctor performs a WriteBarrier assign as it doesn't know if its copying the reference to stack or to heap:

; Assembly listing for method BufferWriter`1:.ctor(ref):this
; Lcl frame size = 40

G_M42621_IG01:
       57                   push     rdi
       56                   push     rsi
       4883EC28             sub      rsp, 40
       488BF1               mov      rsi, rcx
       498BF8               mov      rdi, r8

G_M42621_IG02:
       33D2                 xor      edx, edx
       895610               mov      dword ptr [rsi+16], edx
       48895608             mov      qword ptr [rsi+8], rdx
       488BCE               mov      rcx, rsi
       488BD7               mov      rdx, rdi
       E84094685F           call     CORINFO_HELP_CHECKED_ASSIGN_REF ; Write barrier assign
       488D5618             lea      rdx, bword ptr [rsi+24]
       488BCF               mov      rcx, rdi
       49BB6811DDDAF87F0000 mov      r11, 0x7FF8DADD1168
       4533C0               xor      r8d, r8d
       3909                 cmp      dword ptr [rcx], ecx
       FF155CEBA3FF         call     [IBufferWriter`1:GetSpan(int):struct:this]
       90                   nop      

G_M42621_IG03:
       4883C428             add      rsp, 40
       5E                   pop      rsi
       5F                   pop      rdi
       C3                   ret      

; Total bytes of code 68, prolog size 6 for method BufferWriter`1:.ctor(ref):this

You might think as its a ref struct it can only be stack so doesn't need to use a writebarrrier assign in case of heap; and you'd be right and there is a Jit issue for that: https://github.com/dotnet/coreclr/issues/15755

Inlining the .ctor changes the call site from (write barrier behind BufferWriter'1:.ctor(ref):this):

G_M62269_IG09:
       4C8B4640             mov      r8, gword ptr [rsi+64]
       488D4DA8             lea      rcx, bword ptr [rbp-58H]
       48BAD88042DBF87F0000 mov      rdx, 0x7FF8DB4280D8
       E8DAE7FFFF           call     BufferWriter`1:.ctor(ref):this   ; Write barrier hidden here
       4C8B7D30             mov      r15, gword ptr [rbp+30H]
       4C897C2420           mov      gword ptr [rsp+20H], r15
       448B7D38             mov      r15d, dword ptr [rbp+38H]
       410FB6D7             movzx    rdx, r15b
       89542428             mov      dword ptr [rsp+28H], edx
       488D55A8             lea      rdx, bword ptr [rbp-58H]
       488BCE               mov      rcx, rsi
       458BC6               mov      r8d, r14d
       4C8BCF               mov      r9, rdi
       E87362FEFF           call     Http1OutputProducer:WriteResponseHeadersInternal(byref,int,ref,ref,bool):this

To the following where there is no WriteBarrier as the Jit can "see" its lifetime is stack only:

G_M62270_IG09:
       488B4E40             mov      rcx, gword ptr [rsi+64]
       33D2                 xor      edx, edx
       8955B8               mov      dword ptr [rbp-48H], edx
       488955B0             mov      qword ptr [rbp-50H], rdx
       48894DA8             mov      gword ptr [rbp-58H], rcx
       488D55C0             lea      rdx, bword ptr [rbp-40H]
       49BB28119CD9F87F0000 mov      r11, 0x7FF8D99C1128
       4533C0               xor      r8d, r8d
       3909                 cmp      dword ptr [rcx], ecx
       FF15B5F4A2FF         call     [IBufferWriter`1:GetSpan(int):struct:this]
       4C8B7D30             mov      r15, gword ptr [rbp+30H]
       4C897C2420           mov      gword ptr [rsp+20H], r15
       448B7D38             mov      r15d, dword ptr [rbp+38H]
       410FB6D7             movzx    rdx, r15b
       89542428             mov      dword ptr [rsp+28H], edx
       488D55A8             lea      rdx, bword ptr [rbp-58H]
       488BCE               mov      rcx, rsi
       458BC6               mov      r8d, r14d
       4C8BCF               mov      r9, rdi
       E85E62FEFF           call     Http1OutputProducer:WriteResponseHeadersInternal(byref,int,ref,ref,bool):this

/cc @davidfowl @jkotalik

@davidfowl
Copy link
Member

cc @AArnott might be worth doing in your copied code

@davidfowl davidfowl merged commit 3e47fa7 into dotnet:master Feb 18, 2019
@benaadams benaadams deleted the BufferWriter.ctor branch February 18, 2019 12:39
AArnott added a commit to AArnott/MessagePack-CSharp that referenced this pull request Feb 18, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants