Skip to content

Optimize access to the default FuncPtrStub#129918

Draft
davidwrighton wants to merge 1 commit into
dotnet:mainfrom
davidwrighton:extract-default-funcptrstub-cache
Draft

Optimize access to the default FuncPtrStub#129918
davidwrighton wants to merge 1 commit into
dotnet:mainfrom
davidwrighton:extract-default-funcptrstub-cache

Conversation

@davidwrighton

@davidwrighton davidwrighton commented Jun 27, 2026

Copy link
Copy Markdown
Member

Caches the default-type FuncPtrStub precode directly on the MethodDesc (in MethodDescCodeData) so the common FuncPtrStubs::GetFuncPtrStub path avoids taking the FuncPtrStubs hash-table critical section.

A new DefaultFuncPtrStub field plus SetDefaultFuncPtrStub/GetDefaultFuncPtrStub accessors store and retrieve the cached precode using an interlocked publish. For the default precode type, lookup and insertion now go through the cached field instead of the hash table + m_hashTableCrst.

This was extracted from #129019 to keep the optimization independent of the preemptive-mode changes in that PR.

(It's not clear to me this is worth it, but it's certainly not a good part of that original PR).

Note

This PR was generated with the assistance of GitHub Copilot.

Cache the default-type FuncPtrStub precode directly on the MethodDesc
(in MethodDescCodeData) so the common GetFuncPtrStub path avoids taking
the FuncPtrStubs hash-table critical section. A new field
DefaultFuncPtrStub plus Set/GetDefaultFuncPtrStub accessors store and
retrieve the cached precode using an interlocked publish.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings June 27, 2026 00:34
@davidwrighton

Copy link
Copy Markdown
Member Author

@EgorBot -arm64

using System;
using System.Reflection;
using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;

public interface IFoo
{
    void Bar();
}

public class Foo : IFoo
{
    public void Bar() { }
}

public class DelegateBindBenchmarks
{
    private static readonly MethodInfo s_method = typeof(IFoo).GetMethod(nameof(IFoo.Bar))!;
    private static readonly Type s_delegateType = typeof(Action);
    private readonly Foo _target = new Foo();

    [Benchmark(Baseline = true)]
    public Delegate BindToMethodInfo_InterfaceInstanceMethod()
{
        return Delegate.CreateDelegate(s_delegateType, _target, s_method);
}
}

Note

This comment was generated with the assistance of GitHub Copilot.

@dotnet-policy-service

Copy link
Copy Markdown
Contributor

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

@davidwrighton davidwrighton marked this pull request as draft June 27, 2026 00:36

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

This PR adds a per-MethodDesc cache for the default FuncPtrStub (stored in MethodDescCodeData) so the common FuncPtrStubs::GetFuncPtrStub(pMD) path can avoid taking the FuncPtrStubs hash-table lock.

Changes:

  • Adds a DefaultFuncPtrStub field to MethodDescCodeData plus interlocked publish/accessors on MethodDesc.
  • Updates FuncPtrStubs::GetFuncPtrStub to use the cached stub for the default PrecodeType instead of locking and querying the hash table.
  • Uses an interlocked “first-wins” publish so concurrent stub creation races converge on a single cached stub.

Reviewed changes

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

File Description
src/coreclr/vm/method.hpp Adds MethodDescCodeData::DefaultFuncPtrStub and declares MethodDesc accessors.
src/coreclr/vm/method.cpp Implements interlocked set/get helpers for the cached default funcptr stub.
src/coreclr/vm/fptrstubs.cpp Uses the per-MethodDesc cached stub for default PrecodeType and avoids the hash-table lock on that path.

Comment on lines +73 to +81
if (type != GetDefaultType(pMD))
{
CrstHolder ch(&m_hashTableCrst);
pPrecode = m_hashTable.Lookup(PrecodeKey(pMD, type));
}
else
{
pPrecode = pMD->GetDefaultFuncPtrStub();
}
Comment on lines +137 to 155
if (type == GetDefaultType(pMD))
{
// Set the default funcptr stub in the MethodDesc if it is not already set. If another thread beat us to it, then
// we will use the one that was set by the other thread.
if (pMD->SetDefaultFuncPtrStub(pNewPrecode))
{
pPrecode = pNewPrecode;
amt.SuppressRelease();
}
else
{
pPrecode = pMD->GetDefaultFuncPtrStub();
_ASSERTE(pPrecode != NULL);
setTargetAfterAddingToHashTable = false;
}
}
else
{
CrstHolder ch(&m_hashTableCrst);
Comment thread src/coreclr/vm/method.hpp
Comment on lines 260 to 266
struct MethodDescCodeData final
{
#ifdef FEATURE_CODE_VERSIONING
PTR_MethodDescVersioningState VersioningState;
NativeCodeVersion::OptimizationTier OptimizationTier;
Precode* DefaultFuncPtrStub;
#endif // FEATURE_CODE_VERSIONING
Comment thread src/coreclr/vm/method.hpp
#ifdef FEATURE_CODE_VERSIONING
PTR_MethodDescVersioningState VersioningState;
NativeCodeVersion::OptimizationTier OptimizationTier;
Precode* DefaultFuncPtrStub;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we reuse TemporaryEntryPoint as DefaultFuncPtrStub instead of tracking is separately?

This was not possible originally when we had compact temporary entrypoints that were not back patchable, but it should be possible now.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

No, the temporary entrypoint is back-patchable, but the detail that it ISN'T backpatched is a key part of how we establish vtables to point to the latest tiered code. See PR #125359 for a PR which adjusts how that works and actually takes advantage of the backpatchability of the temporary entrypoint to reduce the number of times we need to do a full back-patch operation, but it still requires that the temporary entrypoint end up not pointing to fully tiered code.

@davidwrighton

Copy link
Copy Markdown
Member Author

I don't like the cost benefit tradeoff of this change. Currently it increases the size of the MethodDescCodeData by the size of a pointer, which probably isn't worth it.

I could fix that, by keeping all the hash table stuff around, reducing the size of the OptimizationTier field to 1 byte, and using the remaining 7 bytes to represent the pointer to the funcptrstub, if the funcptrstub address fits in 7 bytes (and some sentinel value for indicating to look in the hash table, in case we are running on a platform which uses the top byte for something.). (32 bit platforms would just always use the hashtable). However, such tweaks are potentially risky as they might run afoul of MTE like features in the processor, and force us to to fairly weird mappings (For instance with MTE the guaranteed 0 bits of the address on Arm64 hardware are bits 60-63 and 52-55. The tag is stored in bits 56-59.) But this seems a bit overkill for the problem.

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.

3 participants