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

string.EndsWith use SequenceEqual not SequenceCompareTo #22207

Merged
merged 5 commits into from
Jan 28, 2019

Conversation

benaadams
Copy link
Member

@benaadams benaadams commented Jan 25, 2019

for Ordinal.

CompareOrdinalHelper goes via SequenceCompareTo and is then tested if zero

Is more efficient to just find if they are the same; as CompareTo needs to find where they differ and then if >/==/< at that character.

/cc @jkotas @stephentoub

@benaadams
Copy link
Member Author

benaadams commented Jan 25, 2019

It now generates these checks

; Assembly listing for method String:EndsWith(ref,int):bool:this
...
G_M23680_IG10:
       mov      ecx, dword ptr [rsi+8]
       sub      ecx, r8d
       cmp      dword ptr [rsi+8], ecx   ; should eliminate one below?
       jb       SHORT G_M23680_IG15
       cmp      dword ptr [rsi+8], ecx   ; should be eliminated?
       jb       G_M23680_IG22
G_M23680_IG11:

I assume the first should eliminate the second? /cc @AndyAyersMS @mikedn

@benaadams
Copy link
Member Author

Otherwise it calls into SequenceEquals happily

G_M23680_IG10:
       mov      ecx, dword ptr [rsi+8]
       sub      ecx, r8d
       cmp      dword ptr [rsi+8], ecx
       jb       SHORT G_M23680_IG15
       cmp      dword ptr [rsi+8], ecx
       jb       G_M23680_IG22
G_M23680_IG11:
       lea      rdx, bword ptr [rsi+12]
       mov      eax, dword ptr [rsi+8]
       sub      eax, ecx
       movsxd   rcx, ecx
       lea      rcx, bword ptr [rdx+2*rcx]
       lea      rdx, bword ptr [rdi+12]
       cmp      eax, r8d
       jne      SHORT G_M23680_IG12
       movsxd   r8, eax
       shl      r8, 1
       call     [SpanHelpers:SequenceEqual(byref,byref,long):bool]
       movzx    rdx, al
       jmp      SHORT G_M23680_IG13
G_M23680_IG12:
       xor      edx, edx
G_M23680_IG13:
       mov      ebp, edx
       jmp      SHORT G_M23680_IG17

@benaadams
Copy link
Member Author

benaadams commented Jan 25, 2019

Two infra? issues on coreclr-ci

Test pri0 Linux arm64 checked Job
Agent: Hosted Ubuntu 1604 6

Send tests to Helix - 1 error

/home/vsts_azpcontainer/.nuget/packages/microsoft.dotnet.helix.sdk/2.0.0-beta.19067.6/tools/Microsoft.DotNet.Helix.Sdk.MultiQueue.targets(87,5): 
 error : RestApiException: An Unexpected error occured when processing the request. [/__w/1/s/tests/helixpublishwitharcade.proj]
 error :    at Microsoft.DotNet.Helix.Client.WorkItem.ListInternalAsync(String job, CancellationToken cancellationToken) in /_/src/Microsoft.DotNet.Helix/Client/CSharp/generated-code/WorkItem.cs:line 89 [/__w/1/s/tests/helixpublishwitharcade.proj]
 error :    at Microsoft.DotNet.Helix.Client.WorkItem.ListAsync(String job, CancellationToken cancellationToken) in /_/src/Microsoft.DotNet.Helix/Client/CSharp/generated-code/WorkItem.cs:line 47 [/__w/1/s/tests/helixpublishwitharcade.proj]
 error :    at Microsoft.DotNet.Helix.Client.HelixApi.RetryAsync[T](Func`1 function, Action`1 logRetry, Func`2 isRetryable) [/__w/1/s/tests/helixpublishwitharcade.proj]
 error :    at Microsoft.DotNet.Helix.Sdk.HelixWait.WaitForHelixJobAsync(String jobName) in /_/src/Microsoft.DotNet.Helix/Sdk/HelixWait.cs:line 70 [/__w/1/s/tests/helixpublishwitharcade.proj]
 error :    at Microsoft.DotNet.Helix.Sdk.HelixWait.ExecuteCore() in /_/src/Microsoft.DotNet.Helix/Sdk/HelixWait.cs:line 60 [/__w/1/s/tests/helixpublishwitharcade.proj]
 error :    at Microsoft.DotNet.Helix.Sdk.HelixTask.Execute() in /_/src/Microsoft.DotNet.Helix/Sdk/HelixTask.cs:line 43 [/__w/1/s/tests/helixpublishwitharcade.proj]
 error :  [/__w/1/s/tests/helixpublishwitharcade.proj]
  Job be118ae5-6965-407c-8b1d-e2ff6eec0922 is completed with 57 finished work items.

Build FAILED.

Test Pri0 Linux x64 checked Job
Agent: Hosted Ubuntu 1604 2

Send tests to Helix - 1 error

...
  Job 0c0c5f5e-9943-4492-bb31-58f4a8c0ccab is completed with 57 finished work items.
/home/vsts_azpcontainer/.nuget/packages/microsoft.dotnet.helix.sdk/2.0.0-beta.19067.6/tools/Microsoft.DotNet.Helix.Sdk.MultiQueue.targets(87,5):
 error : RestApiException: An Unexpected error occured when processing the request. [/__w/1/s/tests/helixpublishwitharcade.proj]
 error :    at Microsoft.DotNet.Helix.Client.WorkItem.ListInternalAsync(String job, CancellationToken cancellationToken) in /_/src/Microsoft.DotNet.Helix/Client/CSharp/generated-code/WorkItem.cs:line 89 [/__w/1/s/tests/helixpublishwitharcade.proj]
 error :    at Microsoft.DotNet.Helix.Client.WorkItem.ListAsync(String job, CancellationToken cancellationToken) in /_/src/Microsoft.DotNet.Helix/Client/CSharp/generated-code/WorkItem.cs:line 47 [/__w/1/s/tests/helixpublishwitharcade.proj]
 error :    at Microsoft.DotNet.Helix.Client.HelixApi.RetryAsync[T](Func`1 function, Action`1 logRetry, Func`2 isRetryable) [/__w/1/s/tests/helixpublishwitharcade.proj]
 error :    at Microsoft.DotNet.Helix.Sdk.HelixWait.WaitForHelixJobAsync(String jobName) in /_/src/Microsoft.DotNet.Helix/Sdk/HelixWait.cs:line 70 [/__w/1/s/tests/helixpublishwitharcade.proj]
 error :    at Microsoft.DotNet.Helix.Sdk.HelixWait.ExecuteCore() in /_/src/Microsoft.DotNet.Helix/Sdk/HelixWait.cs:line 60 [/__w/1/s/tests/helixpublishwitharcade.proj]
 error :    at Microsoft.DotNet.Helix.Sdk.HelixTask.Execute() in /_/src/Microsoft.DotNet.Helix/Sdk/HelixTask.cs:line 43 [/__w/1/s/tests/helixpublishwitharcade.proj]
 error :  [/__w/1/s/tests/helixpublishwitharcade.proj]
  Job 3f5e37d4-3cdd-4fa4-b837-d04fad1dbb04 is completed with 57 finished work items.

Build FAILED.

@benaadams
Copy link
Member Author

Raised issue for the double check https://github.com/dotnet/coreclr/issues/22246

@jkotas jkotas merged commit 57fd77e into dotnet:master Jan 28, 2019
@jkotas
Copy link
Member

jkotas commented Jan 28, 2019

Thanks @benaadams !

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