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

Remove HMFs from JIT helpers #111088

Merged
merged 3 commits into from
Jan 4, 2025

Conversation

AaronRobinsonMSFT
Copy link
Member

@AaronRobinsonMSFT AaronRobinsonMSFT commented Jan 4, 2025

Remove unused JIT_StressGC.
Convert JIT_GetRuntimeFieldStub and JIT_GetRuntimeMethodStub to C#.

Contributes to #95695

Choose a reason for hiding this comment

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

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

Files not reviewed (8)
  • src/coreclr/inc/jithelpers.h: Language not supported
  • src/coreclr/vm/corelib.h: Language not supported
  • src/coreclr/vm/field.cpp: Language not supported
  • src/coreclr/vm/field.h: Language not supported
  • src/coreclr/vm/frames.h: Language not supported
  • src/coreclr/vm/interpreter.cpp: Language not supported
  • src/coreclr/vm/jithelpers.cpp: Language not supported
  • src/coreclr/vm/runtimehandles.cpp: Language not supported
Comments suppressed due to low confidence (2)

src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs:929

  • Ensure that the behavior of the new method FromPtr in RuntimeMethodInfoStub is covered by tests.
internal static object FromPtr(IntPtr pMD)

src/coreclr/System.Private.CoreLib/src/System/RuntimeHandles.cs:1397

  • Ensure that the behavior of the new method FromPtr in RuntimeFieldInfoStub is covered by tests.
internal static object FromPtr(IntPtr pFD)
Copy link
Contributor

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

@AaronRobinsonMSFT
Copy link
Member Author

@jkotas I couldn't find any obvious way to emit a ldtoken for a method or field via C#, so I didn't measure any potential performance impact here. Suggestions are welcome.

@jkotas
Copy link
Member

jkotas commented Jan 4, 2025

@EgorBot -windows_intel

using System;
using System.Reflection;
using System.Reflection.Emit;
using BenchmarkDotNet.Attributes;

public class Bench
{
    Func<RuntimeMethodHandle> d = Init();

    static Func<RuntimeMethodHandle> Init()
    {
        DynamicMethod dm = new DynamicMethod("My", typeof(RuntimeMethodHandle), null, typeof(string).Module);

        MethodInfo randomMethod = typeof(object).GetMethod("ReferenceEquals");

        ILGenerator il = dm.GetILGenerator();
        il.Emit(OpCodes.Ldtoken, randomMethod);
        il.Emit(OpCodes.Ret);

        return dm.CreateDelegate<Func<RuntimeMethodHandle>>();
    }

    [Benchmark]
    public RuntimeMethodHandle LdToken() => d();
}

@jkotas
Copy link
Member

jkotas commented Jan 4, 2025

I couldn't find any obvious way to emit a ldtoken for a method or field via C#

C# emits ldtoken for methods/fields as part of expression trees, but it is wrapped by a bunch of code that would obfuscate the perf measurement.

I have a posted a microbenchmark using DynamicMethods. I am not worried about the perf impact of this change. I expect this change to be a perf improvement.

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.

Thanks

@AaronRobinsonMSFT AaronRobinsonMSFT merged commit dae8909 into dotnet:main Jan 4, 2025
90 checks passed
@AaronRobinsonMSFT AaronRobinsonMSFT deleted the hmf_jithelpers03 branch January 4, 2025 19:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants