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.

Sign up for GitHub

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

Jump to bottom

Investigate EETypePtr overhead #232

Closed
jkotas opened this issue Oct 15, 2020 · 1 comment · Fixed by dotnet/runtime#97207
Closed

Investigate EETypePtr overhead #232

jkotas opened this issue Oct 15, 2020 · 1 comment · Fixed by dotnet/runtime#97207
Labels
area-NativeAOT-coreclr .NET runtime optimized for ahead of time compilation

Comments

@jkotas
Copy link
Member

jkotas commented Oct 15, 2020

Context: #225 (comment)

RuJIT is not able to optimize out the EETypePtr wrapper completely. In the example above the EETypePtr wrapper:

  • Disables tailcall optimization. RyuJIT is not able to prove that the byrefs taken by EETypePtr are not passed into the worker method
  • Results into less efficient register allocation and instruction selection (e.g. cmp [rcx], rdx vs. mov rax, [rcx] cmp rax, rdx)
@jkotas jkotas added the area-NativeAOT-coreclr .NET runtime optimized for ahead of time compilation label Oct 15, 2020
@jkotas
Copy link
Member Author

jkotas commented Oct 15, 2020 •
edited
Loading

Another factor to consider is code sharing with full CoreCLR. MethodTable and EEType are 80% the same between. We need a plan how to share the code that uses these. For example, the Array.Copy impl could have been shared except this difference.

EEType is a better name. Unfortunately, MethodTable is too entrenched in CoreCLR in all code, documentation and user facing surface (e.g. DumpMT command in SOS).

@jkotas jkotas mentioned this issue Oct 15, 2020
Merged
MichalStrehovsky added a commit to MichalStrehovsky/runtimelab that referenced this issue Sep 3, 2021
@MichalStrehovsky
Speed up StElemRef if there's no covariance
8ba192c 
Makes storing into arrays 40% faster if there's no covariance.

I added `Object.MethodTableOf<T>` because `EETypePtrOf<T>` was causing a spill into a temporary and was messing up codegen and we'll want this for dotnet#232 anyway. I wanted to add `MethodTable.Of<T>`, but `Object` has a property named `MethodTable` and that was breaking the pattern. So it's on `Object`. Oh well.
@MichalStrehovsky MichalStrehovsky mentioned this issue Sep 3, 2021
Merged
jkotas pushed a commit that referenced this issue Sep 3, 2021
@MichalStrehovsky
Speed up StElemRef if there's no covariance (#1496)
54e6f84 
Makes storing into arrays 40% faster if there's no covariance.

I added `Object.MethodTableOf<T>` because `EETypePtrOf<T>` was causing a spill into a temporary and was messing up codegen and we'll want this for #232 anyway. I wanted to add `MethodTable.Of<T>`, but `Object` has a property named `MethodTable` and that was breaking the pattern. So it's on `Object`. Oh well.
@jkotas jkotas mentioned this issue Apr 8, 2022
Merged
MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this issue Mar 13, 2023
@MichalStrehovsky
Use MethodTable.Of in generated valuetype methods
e1d604f 
I thought it might improve codegen, but the improvements are marginal. Still, contributes to dotnet/runtimelab#232.
@MichalStrehovsky MichalStrehovsky mentioned this issue Mar 13, 2023
Merged
MichalStrehovsky added a commit to dotnet/runtime that referenced this issue Mar 13, 2023
@MichalStrehovsky
Use MethodTable.Of in generated valuetype methods (#83325)
1c8d37a 
I thought it might improve codegen, but the improvements are marginal. Still, contributes to dotnet/runtimelab#232.
@VSadov VSadov mentioned this issue Mar 14, 2023
Merged
MichalStrehovsky added a commit to MichalStrehovsky/runtime that referenced this issue Jan 19, 2024
@MichalStrehovsky
Get rid of EETypePtr
180aaf8 
Resolves dotnet/runtimelab#232.

This wrapper served two purposes over `MethodTable*`:

* Make sure equality semantics were enforced at the time where we could have multiple `MethodTable*` representing the same type. This is no longer the case and we don't need it for this purpose.
* Save us from typing `unsafe`

Apart from this, it also pessimized codegen (see the linked issue). Bye `EETypePtr`.

I didn't fully delete it because the `CorElementType` conversion is still there. We can deal with that later.
@MichalStrehovsky MichalStrehovsky mentioned this issue Jan 19, 2024
Merged
MichalStrehovsky added a commit to dotnet/runtime that referenced this issue Jan 21, 2024
@MichalStrehovsky
Get rid of EETypePtr (#97207)
6fafbad 
Resolves dotnet/runtimelab#232.

This wrapper served two purposes over `MethodTable*`:

* Make sure equality semantics were enforced at the time where we could have multiple `MethodTable*` representing the same type. This is no longer the case and we don't need it for this purpose.
* Save us from typing `unsafe`

Apart from this, it also pessimized codegen (see the linked issue). Bye `EETypePtr`.

I didn't fully delete it because the `CorElementType` conversion is still there. We can deal with that later.
tmds pushed a commit to tmds/runtime that referenced this issue Jan 23, 2024
@MichalStrehovsky @tmds
Get rid of EETypePtr (dotnet#97207)
7a6c786 
Resolves dotnet/runtimelab#232.

This wrapper served two purposes over `MethodTable*`:

* Make sure equality semantics were enforced at the time where we could have multiple `MethodTable*` representing the same type. This is no longer the case and we don't need it for this purpose.
* Save us from typing `unsafe`

Apart from this, it also pessimized codegen (see the linked issue). Bye `EETypePtr`.

I didn't fully delete it because the `CorElementType` conversion is still there. We can deal with that later.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
area-NativeAOT-coreclr .NET runtime optimized for ahead of time compilation
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Get rid of EETypePtr MichalStrehovsky/runtime
1 participant
@jkotas