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

Remove some calls to wstrcpy #32342

Merged
merged 3 commits into from
Feb 21, 2020

Conversation

GrabYourPitchforks
Copy link
Member

Some modest codegen + perf improvements to existing string functions. For instance, calling the string(char[]) ctor:

Method Toolchain CharArrayLength Mean Error StdDev Median Ratio RatioSD
FromCharArrCtor master 0 2.035 ns 0.1192 ns 0.0931 ns 2.018 ns 1.00 0.00
FromCharArrCtor wstrcpy_2 0 1.651 ns 0.0824 ns 0.0643 ns 1.652 ns 0.81 0.05
FromCharArrCtor master 4 10.552 ns 0.2164 ns 0.2024 ns 10.490 ns 1.00 0.00
FromCharArrCtor wstrcpy_2 4 9.084 ns 0.2481 ns 0.2199 ns 9.081 ns 0.86 0.03
FromCharArrCtor master 16 12.158 ns 0.2545 ns 0.2256 ns 12.099 ns 1.00 0.00
FromCharArrCtor wstrcpy_2 16 10.760 ns 0.2939 ns 0.4308 ns 10.603 ns 0.90 0.05
FromCharArrCtor master 71 24.151 ns 1.3100 ns 3.6299 ns 22.395 ns 1.00 0.00
FromCharArrCtor wstrcpy_2 71 19.614 ns 0.4503 ns 0.4818 ns 19.516 ns 0.80 0.13
; -- CODEGEN BEFORE --

00007ff8`f97c9e80 57              push    rdi
00007ff8`f97c9e81 56              push    rsi
00007ff8`f97c9e82 53              push    rbx
00007ff8`f97c9e83 4883ec30        sub     rsp,30h
00007ff8`f97c9e87 33c0            xor     eax,eax
00007ff8`f97c9e89 4889442428      mov     qword ptr [rsp+28h],rax
00007ff8`f97c9e8e 4889442420      mov     qword ptr [rsp+20h],rax
00007ff8`f97c9e93 488bf2          mov     rsi,rdx
00007ff8`f97c9e96 4885f6          test    rsi,rsi ; if (value == null)
00007ff8`f97c9e99 7407            je      System_Private_CoreLib!System.String.Ctor(Char[])+0xffffffff`b417d9b2 (00007ff8`f97c9ea2)
00007ff8`f97c9e9b 8b7e08          mov     edi,dword ptr [rsi+8]
00007ff8`f97c9e9e 85ff            test    edi,edi ; if (value.Length == 0)
00007ff8`f97c9ea0 7515            jne     System_Private_CoreLib!System.String.Ctor(Char[])+0xffffffff`b417d9c7 (00007ff8`f97c9eb7)
00007ff8`f97c9ea2 48b86030905850020000 mov rax,25058903060h ; return string.Empty
00007ff8`f97c9eac 488b00          mov     rax,qword ptr [rax]
00007ff8`f97c9eaf 4883c430        add     rsp,30h
00007ff8`f97c9eb3 5b              pop     rbx
00007ff8`f97c9eb4 5e              pop     rsi
00007ff8`f97c9eb5 5f              pop     rdi
00007ff8`f97c9eb6 c3              ret
00007ff8`f97c9eb7 8bcf            mov     ecx,edi
00007ff8`f97c9eb9 e8caa9d8ff      call    <FastAllocateString>
00007ff8`f97c9ebe 488bd8          mov     rbx,rax
00007ff8`f97c9ec1 391b            cmp     dword ptr [rbx],ebx ; null check on return value from FastAllocateString
00007ff8`f97c9ec3 4c8d430c        lea     r8,[rbx+0Ch] ; &return._firstChar
00007ff8`f97c9ec7 4c89442428      mov     qword ptr [rsp+28h],r8
00007ff8`f97c9ecc 488b4c2428      mov     rcx,qword ptr [rsp+28h]
00007ff8`f97c9ed1 4889742420      mov     qword ptr [rsp+20h],rsi
00007ff8`f97c9ed6 4c8b442420      mov     r8,qword ptr [rsp+20h]
00007ff8`f97c9edb 4183780800      cmp     dword ptr [r8+8],0
00007ff8`f97c9ee0 7504            jne     System_Private_CoreLib!System.String.Ctor(Char[])+0xffffffff`b417d9f6 (00007ff8`f97c9ee6)
00007ff8`f97c9ee2 33d2            xor     edx,edx
00007ff8`f97c9ee4 eb14            jmp     System_Private_CoreLib!System.String.Ctor(Char[])+0xffffffff`b417da0a (00007ff8`f97c9efa)
00007ff8`f97c9ee6 488b542420      mov     rdx,qword ptr [rsp+20h]
00007ff8`f97c9eeb 837a0800        cmp     dword ptr [rdx+8],0
00007ff8`f97c9eef 762c            jbe     System_Private_CoreLib!System.String.Ctor(Char[])+0xffffffff`b417da2d (00007ff8`f97c9f1d)
00007ff8`f97c9ef1 488b542420      mov     rdx,qword ptr [rsp+20h]
00007ff8`f97c9ef6 4883c210        add     rdx,10h
00007ff8`f97c9efa d1e7            shl     edi,1
00007ff8`f97c9efc 448bc7          mov     r8d,edi
00007ff8`f97c9eff e8b407d9ff      call    <Memmove> ; wstrcpy was inlined into caller
00007ff8`f97c9f04 33c0            xor     eax,eax
00007ff8`f97c9f06 4889442428      mov     qword ptr [rsp+28h],rax
00007ff8`f97c9f0b 33c0            xor     eax,eax
00007ff8`f97c9f0d 4889442420      mov     qword ptr [rsp+20h],rax
00007ff8`f97c9f12 488bc3          mov     rax,rbx
00007ff8`f97c9f15 4883c430        add     rsp,30h
00007ff8`f97c9f19 5b              pop     rbx
00007ff8`f97c9f1a 5e              pop     rsi
00007ff8`f97c9f1b 5f              pop     rdi
00007ff8`f97c9f1c c3              ret
00007ff8`f97c9f1d e88ecca25f      call    CoreCLR!GetCLRRuntimeHost+0x64390 (00007ff9`591f6bb0)
00007ff8`f97c9f22 cc              int     3

; -- CODEGEN AFTER --

00007ff8`f97a9e80 57              push    rdi
00007ff8`f97a9e81 56              push    rsi
00007ff8`f97a9e82 4883ec28        sub     rsp,28h
00007ff8`f97a9e86 488bf2          mov     rsi,rdx
00007ff8`f97a9e89 4885f6          test    rsi,rsi ; if (value == null)
00007ff8`f97a9e8c 7407            je      System_Private_CoreLib!System.String.Ctor(Char[])+0xffffffff`ae5edc05 (00007ff8`f97a9e95)
00007ff8`f97a9e8e 8b4e08          mov     ecx,dword ptr [rsi+8]
00007ff8`f97a9e91 85c9            test    ecx,ecx ; if (value.Length == 0)
00007ff8`f97a9e93 7514            jne     System_Private_CoreLib!System.String.Ctor(Char[])+0xffffffff`ae5edc19 (00007ff8`f97a9ea9)
00007ff8`f97a9e95 48b860302154a4020000 mov rax,2A454213060h ; return string.Empty
00007ff8`f97a9e9f 488b00          mov     rax,qword ptr [rax]
00007ff8`f97a9ea2 4883c428        add     rsp,28h
00007ff8`f97a9ea6 5e              pop     rsi
00007ff8`f97a9ea7 5f              pop     rdi
00007ff8`f97a9ea8 c3              ret
00007ff8`f97a9ea9 e8daa9d8ff      call    <FastAllocateString>
00007ff8`f97a9eae 488bf8          mov     rdi,rax
00007ff8`f97a9eb1 448b4708        mov     r8d,dword ptr [rdi+8] ; no extra indirection to check for null 'result'
00007ff8`f97a9eb5 488d4f0c        lea     rcx,[rdi+0Ch]
00007ff8`f97a9eb9 488d5610        lea     rdx,[rsi+10h]
00007ff8`f97a9ebd 49d1e0          shl     r8,1
00007ff8`f97a9ec0 e80308d9ff      call    <Memmove(byte)> ; Memmove(T) was inlined into caller
00007ff8`f97a9ec5 488bc7          mov     rax,rdi
00007ff8`f97a9ec8 4883c428        add     rsp,28h
00007ff8`f97a9ecc 5e              pop     rsi
00007ff8`f97a9ecd 5f              pop     rdi
00007ff8`f97a9ece c3              ret

I didn't remove all calls to wstrcpy because there were some places (like below) where the pointer math would have made the calls to Unsafe.Add(...) unreadable.

fixed (char* src = &_firstChar)
{
fixed (char* dst = &result._firstChar)
{
wstrcpy(dst, src, startIndex);
wstrcpy(dst + startIndex, src + startIndex + count, newLength - startIndex);
}
}

@AaronRobinsonMSFT AaronRobinsonMSFT added the tenet-performance Performance related issue label Feb 15, 2020
@jkotas
Copy link
Member

jkotas commented Feb 15, 2020

didn't remove all calls to wstrcpy

I think changing the remaining two wstrcpy uses in string.cs would be fine.

I guess that you have not touched Copy because of we consider it to be obsolete ... but we still carry the slightly bigger code around and there is value in consistency.

@GrabYourPitchforks
Copy link
Member Author

Tests are flaky on my machine for some reason (probably some stale files in the build) - so relying on the CI to catch any problems here.

@GrabYourPitchforks GrabYourPitchforks merged commit 70c1e9d into dotnet:master Feb 21, 2020
@GrabYourPitchforks GrabYourPitchforks deleted the wstrcpy_2 branch February 21, 2020 04:54
@ghost ghost locked as resolved and limited conversation to collaborators Dec 10, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants