Skip to content

Introduce RuntimeHelpers.GetObjectHeader and Optimize Object.GetHashCode() with it#100999

Closed
EgorBo wants to merge 4 commits intodotnet:mainfrom
EgorBo:GetObjectHeader-jit
Closed

Introduce RuntimeHelpers.GetObjectHeader and Optimize Object.GetHashCode() with it#100999
EgorBo wants to merge 4 commits intodotnet:mainfrom
EgorBo:GetObjectHeader-jit

Conversation

@EgorBo
Copy link
Member

@EgorBo EgorBo commented Apr 13, 2024 •
edited
Loading

A GC-hole free version of #55273, should slightly speed up object.GetHashCode, e.g.:

Example:

[MethodImpl(MethodImplOptions.NoInlining)]
int Test(Prog o) => o.GetHashCode();

Current codegen:

; Method Prog:Test(Prog):int:this (FullOpts)
       sub      rsp, 40
       cmp      byte  ptr [rdx], dl
       mov      rcx, rdx
       call     System.Runtime.CompilerServices.RuntimeHelpers:GetHashCode(System.Object):int
       nop      
       add      rsp, 40
       ret      
; Total bytes of code: 20

New codegen:

; Method Prog:Test(Prog):int:this (FullOpts)
       sub      rsp, 40
       cmp      byte  ptr [rdx], dl
       mov      eax, dword ptr [rcx-0x04] ;; <---
       mov      edx, eax
       and      edx, 0xC000000
       cmp      edx, 0xC000000
       jne      SHORT G_M29166_IG04
       and      eax, 0x3FFFFFF
       jmp      SHORT G_M8578_IG05
G_M8578_IG04:  ;; offset=0x0020
       mov      rcx, rdx
       call     System.Runtime.CompilerServices.RuntimeHelpers:InternalGetHashCode(System.Object):int
G_M8578_IG05:  ;; offset=0x0028
       nop      
       add      rsp, 40
       ret      
; Total bytes of code: 46

This should allow us to convert Monitor.Enter/Exit to C# as well (thin lock, etc).

@ghost ghost added the area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI label Apr 13, 2024
@dotnet-policy-service
Copy link
Contributor

dotnet-policy-service bot commented Apr 13, 2024

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

@EgorBo EgorBo mentioned this pull request Apr 13, 2024
Closed
This was referenced Apr 13, 2024
Open
Closed
@huoyaoyuan
Copy link
Member

huoyaoyuan commented Apr 13, 2024

I tried non-intrinsic version ealier and measured a significant regression.

huoyaoyuan
huoyaoyuan reviewed Apr 13, 2024
View reviewed changes
@EgorBo
Copy link
Member Author

EgorBo commented Apr 13, 2024

I tried non-intrinsic version ealier and measured a significant regression.

with pinning? no wonder then 🙂

@huoyaoyuan
Copy link
Member

huoyaoyuan commented Apr 13, 2024

with pinning? no wonder then 🙂

No pinning. I was not sure about how GC deals with exterior pointer. I reverted it then in #97641

@EgorBo
Copy link
Member Author

EgorBo commented Apr 13, 2024 •
edited
Loading

with pinning? no wonder then 🙂

No pinning. I was not sure about how GC deals with exterior pointer. I reverted it then in #97641

I don't see why my version could be slower (and it's not according to benchmarks) - the codegen has a small CQ problem (#101000) but I am not even sure it can change the result.

@huoyaoyuan
Copy link
Member

huoyaoyuan commented Apr 13, 2024

I don't see why my version could be slower

Yeah, I realized it may need intrinsic handling like retrieving method table.

@EgorBo
Copy link
Member Author

EgorBo commented Apr 13, 2024

Looks like this PR is a regression when object has a syncblock and I don't think I can surface PassiveGetSyncBlock into C#

@EgorBo EgorBo closed this Apr 13, 2024
@jkotas
Copy link
Member

jkotas commented Apr 13, 2024

This should allow us to convert Monitor.Enter/Exit to C# as well (thin lock, etc).

We would be need both reads and interlocked writes for this.

To enable rewriting methods like there in C#, I think it would be better to look into general pinning optimizations (#63397) rather than specialized intrinsics.

@jkotas
Copy link
Member

jkotas commented Apr 13, 2024

surface PassiveGetSyncBlock into C#

It can be done once we have a good reason to.

@github-actions github-actions bot locked and limited conversation to collaborators May 14, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

Copilot code review Copilot Awaiting requested review from Copilot Copilot will automatically review once the pull request is marked ready for review

2 more reviewers

@AustinWise AustinWise AustinWise left review comments

@huoyaoyuan huoyaoyuan huoyaoyuan left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

@EgorBo EgorBo

Labels

area-CodeGen-coreclr CLR JIT compiler in src/coreclr/src/jit and related components such as SuperPMI

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@EgorBo @huoyaoyuan @jkotas @AustinWise