Skip to content

Stub Precode variant for DynamicHelpers #113402

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

Merged
merged 9 commits into from
Mar 19, 2025

Conversation

davidwrighton
Copy link
Member

@davidwrighton davidwrighton commented Mar 12, 2025

Run DynamicHelpers logic with StubPrecodes instead of dynamically generating the stubs.

Performance measurements indicate that this path is about 1% faster than the current approach for startup of my powershell benchmark, and results seem similar between X64 and Arm64 which was a surprise to me. Throughput results look like roughly a wash. I'd like to merge this PR in with its current state of changing all Arm64 and X64 behavior to use the new path to get a full set of performance data, as it appears this might just be a better approach than generating stubs dynamically.

  • Does it basically work?
  • Port assembly to Arm64, Unix and Windows
  • Port assembly to X64 Unix
  • Perf test startup ... probably on Linux arm64
  • Perf test throughput impact (ASP.NET Benchmarks appear mostly unaffected)

Copy link
Contributor

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

@steveisok
Copy link
Member

/cc @dotnet/area-type-system-and-startup

@davidwrighton davidwrighton marked this pull request as ready for review March 14, 2025 20:40
@Copilot Copilot AI review requested due to automatic review settings March 14, 2025 20:40
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

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

Copilot reviewed 17 out of 17 changed files in this pull request and generated no comments.

@davidwrighton davidwrighton marked this pull request as draft March 14, 2025 20:41
@davidwrighton davidwrighton changed the title [DRAFT] Stub Precode variant for DynamicHelpers Stub Precode variant for DynamicHelpers Mar 17, 2025
@davidwrighton davidwrighton marked this pull request as ready for review March 17, 2025 21:43
Copy link
Member

@janvorli janvorli left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

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.

This will work for iOS and the startup improvement is nice side-effect. I don't see how it is going to work for wasm and potentially other architectures where low-level tricks like this are not possible.

As I have mentioned offline, we may want to use fixups that run before the method executes as the most portable solution. I expect that it would come with similar startup improvement as side-effect, and it would have a lot less arch-specific code.

@davidwrighton
Copy link
Member Author

@jkotas I agree, although I also see ways to make this work in wasm (with some unfortunate bloat in file size). I suspect that just dealing with fixups will be fundamentally better for most paths in wasm, and probably even for iOS, but this path will work well in the short term, and didn't require any significant refactorings (and thus was surprisingly cheap to implement).

@davidwrighton davidwrighton merged commit 0666ad4 into dotnet:main Mar 19, 2025
95 checks passed
@github-actions github-actions bot locked and limited conversation to collaborators Apr 19, 2025
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.

4 participants