Skip to content
New issue

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

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

Already on GitHub? # 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 as# 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
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants