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

Intrinsify Interlocked.CompareExchange and Interlocked.Exchange for null #79181

Merged
merged 3 commits into from Dec 5, 2022

Conversation

EgorBo
Copy link
Member

@EgorBo EgorBo commented Dec 3, 2022

Closes #69000

object Foo(ref object a)
    => Interlocked.Exchange(ref a, null);

object Foo(ref object a, object b)
    => Interlocked.CompareExchange(ref a, null, b);
}

Was:

; Assembly listing for method Prog:Foo(byref):System.Object:this
       488BCA               mov      rcx, rdx
       33D2                 xor      rdx, rdx
       E93616AC5F           jmp      Exchange(byref,System.Object):System.Object
; Total bytes of code 10


; Assembly listing for method Prog:Foo(byref,System.Object):System.Object:this
       488BCA               mov      rcx, rdx
       498BD0               mov      rdx, r8
       4533C0               xor      r8, r8
       E9A216AC5F           jmp      CompareExchange(byref,System.Object,System.Object):System.Object
; Total bytes of code 14

Now:

; Method Prog:Foo(byref):System.Object:this
       33C0                 xor      rax, rax
       488702               xchg     gword ptr [rdx], rax
       C3                   ret      
; Total bytes of code: 6


; Method Prog:Foo(byref,System.Object):System.Object:this
       33C9                 xor      rcx, rcx
       498BC0               mov      rax, r8
       F0                   lock     
       480FB10A             cmpxchg  gword ptr [rdx], rcx
       C3                   ret         
; Total bytes of code: 11

@dotnet-issue-labeler dotnet-issue-labeler bot added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Dec 3, 2022
@ghost ghost assigned EgorBo Dec 3, 2022
@ghost
Copy link

ghost commented Dec 3, 2022

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

Issue Details

Closes #69000

object Foo(ref object a)
    => Interlocked.Exchange(ref a, null);

object Foo(ref object a, object b)
    => Interlocked.CompareExchange(ref a, b, null);
}

Was:

; Assembly listing for method Prog:Foo(byref):System.Object:this
       488BCA               mov      rcx, rdx
       33D2                 xor      rdx, rdx
       E93616AC5F           jmp      Exchange(byref,System.Object):System.Object
; Total bytes of code 10


; Assembly listing for method Prog:Foo(byref,System.Object):System.Object:this
       488BCA               mov      rcx, rdx
       498BD0               mov      rdx, r8
       4533C0               xor      r8, r8
       E9A216AC5F           jmp      CompareExchange(byref,System.Object,System.Object):System.Object
; Total bytes of code 14

Now:

; Method Prog:Foo(byref):System.Object:this
       33C0                 xor      rax, rax
       488702               xchg     gword ptr [rdx], rax
       C3                   ret      
; Total bytes of code: 6


; Method Prog:Foo(byref,System.Object):System.Object:this
       33C0                 xor      rax, rax
       F0                   lock     
       4C0FB102             cmpxchg  gword ptr [rdx], r8
       C3                   ret      
; Total bytes of code: 8
Author: EgorBo
Assignees: -
Labels:

area-CodeGen-coreclr

Milestone: -

@jkotas
Copy link
Member

jkotas commented Dec 3, 2022

Interlocked.CompareExchange(ref a, b, null);

This needs write barrier for b. I do not think you can optimize it like this.

You can optimize Interlocked.CompareExchange(ref a, null, b). It is much less common though.

@EgorBo EgorBo marked this pull request as ready for review December 3, 2022 11:14
@EgorBo
Copy link
Member Author

EgorBo commented Dec 3, 2022

Interlocked.CompareExchange(ref a, b, null);

This needs write barrier for b. I do not think you can optimize it like this.

You can optimize Interlocked.CompareExchange(ref a, null, b). It is much less common though.

Thanks! Fixed, grep: https://grep.app/search?q=CompareExchange%5C%28ref%20%5Cw%2B%2C%20null%2C%20&regexp=true

@EgorBo
Copy link
Member Author

EgorBo commented Dec 3, 2022

/azp list

@azure-pipelines

This comment was marked as outdated.

@EgorBo
Copy link
Member Author

EgorBo commented Dec 3, 2022

/azp run runtime-coreclr outerloop, runtime-coreclr gcstress0x3-gcstress0xc, runtime-extra-platforms

@azure-pipelines
Copy link

Azure Pipelines successfully started running 3 pipeline(s).

@EgorBo
Copy link
Member Author

EgorBo commented Dec 5, 2022

@dotnet/jit-contrib PTAL, simple change

Copy link
Member

@kunalspathak kunalspathak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks!

@EgorBo EgorBo merged commit 60d00c4 into dotnet:main Dec 5, 2022
@EgorBo EgorBo deleted the intrinsify-interlocked-null branch December 5, 2022 15:46
@EgorBo
Copy link
Member Author

EgorBo commented Dec 8, 2022

Wow, quite a few improvements: dotnet/perf-autofiling-issues#10542 (e.g. Regex IsMatch as was expected by @GrabYourPitchforks in #69000 (comment))

@drieseng
Copy link
Contributor

drieseng commented Dec 8, 2022

@EgorBo, is it expected that IsMatch_Multithreading went from 56.73 ms to 0 ns?

@stephentoub
Copy link
Member

is it expected that IsMatch_Multithreading went from 56.73 ms to 0 ns?

No. That's got to be a test harness / reporting issue, or else some kind of catastrophic failure :)

Wow, quite a few improvements

Interlocked.Exchange(ref location, null) is reasonably common in multi-threaded caches / object pools, used to reserve some pooled object from a shared cache. I'd expect to see wins in such uses (which many of the cited improvements seem to be).

@dotnet dotnet locked as resolved and limited conversation to collaborators Jan 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-CodeGen-coreclr CLR 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.

Perf: Interlocked.Exchange should special-case nullptr values
5 participants