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

Convert pointers in a couple tables to relative #78801

Merged
merged 1 commit into from
Nov 28, 2022

Conversation

MichalStrehovsky
Copy link
Member

EmbeddedPointerIndirection is used in:

  • Array of pointers to dispatch maps
  • Array of pointers to GC static infos
  • Array of pointers to eager constructors

I'm also updating array of module initializers emission since the code to interpret this table at runtime is the same as eager constructors.

I'd guess this saves another ~1% on Linux. I didn't measure Windows this time. There will a couple kilobytes of savings for hello world on Windows for sure.

Cc @dotnet/ilc-contrib

`EmbeddedPointerIndirection` is used in:

* Array of pointers to dispatch maps
* Array of pointers to GC static infos
* Array of pointers to eager constructors

I'm also updating array of module initializers emission since the code to interpret this table at runtime is the same as eager constructors.
@ghost
Copy link

ghost commented Nov 24, 2022

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

Issue Details

EmbeddedPointerIndirection is used in:

  • Array of pointers to dispatch maps
  • Array of pointers to GC static infos
  • Array of pointers to eager constructors

I'm also updating array of module initializers emission since the code to interpret this table at runtime is the same as eager constructors.

I'd guess this saves another ~1% on Linux. I didn't measure Windows this time. There will a couple kilobytes of savings for hello world on Windows for sure.

Cc @dotnet/ilc-contrib

Author: MichalStrehovsky
Assignees: -
Labels:

area-NativeAOT-coreclr

Milestone: -


for (byte* pCurrent = pInitializers;
pCurrent < (pInitializers + length);
pCurrent += MethodTable.SupportsRelativePointers ? sizeof(int) : sizeof(nint))
Copy link
Contributor

Choose a reason for hiding this comment

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

Extracting this logic into a property instead of c&p?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, we still need to switch on the size of the element later in the loop, so while it would be fewer characters, whoever is reading the code would now need to scroll to the definition to match it with the inner logic.

The ultimate improvement would be to split the code that reads the data structures from the code that interprets it with a struct iterator. But this is code that we only touch once every few years and mostly just forget it exists, so I didn't spend time to make it pretty.

Copy link
Member

@jkotas jkotas left a comment

Choose a reason for hiding this comment

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

Thank you!

@jkotas
Copy link
Member

jkotas commented Nov 28, 2022

Opened dotnet/arcade#11723 on the test failures

@jkotas jkotas merged commit 1771d63 into dotnet:main Nov 28, 2022
@MichalStrehovsky MichalStrehovsky deleted the embedptr branch November 28, 2022 04:40
@ghost ghost locked as resolved and limited conversation to collaborators Dec 28, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants