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

Efficient way to check if a method has been overridden #111083

Open
huoyaoyuan opened this issue Jan 4, 2025 · 5 comments
Open

Efficient way to check if a method has been overridden #111083

huoyaoyuan opened this issue Jan 4, 2025 · 5 comments
Labels
area-System.Runtime untriaged New issue has not been triaged by the area owner

Comments

@huoyaoyuan
Copy link
Member

Sometimes, it's useful to check overridden status of methods to enable specific optimization. For example, Stream checks override status of APM methods to skip an extra layer:

private Task BeginEndWriteAsync(byte[] buffer, int offset, int count)
{
if (!HasOverriddenBeginEndWrite())
{
// If the Stream does not override Begin/EndWrite, then we can take an optimized path
// that skips an extra layer of tasks / IAsyncResults.
return BeginWriteInternal(buffer, offset, count, null, null, serializeAsynchronously: true, apm: false);
}

A virtual-method based UI framework may also use the pattern to reduce overhead of event handling:

void AddChild(Element child)
{
    if (__has_overridden(child, OnMouseMove))
        handlesMouseMove.Add(child);
}

override void OnMouseMove(MouseMoveEventArgs args)
{
    foreach (var child in handlesMouseMove)
        child.OnMouseMove(MouseMoveEventArgs args);
}

Currently there is no simple way to check if a method is overridden in C#. Using reflection is complicated and not intuitive, with baseMethod.DeclaringType != derivedMethod.DeclaringType && baseMethod.GetBaseDefinition() == derivedMethod.GetBaseDefinition(). It's also very inefficient.
Stream is specially handled by CLR, and the approach is not extensible at all. In IL, it's achievable by comparing method pointers with ldvirtftn:

ILToken beginMethodToken = emitter.NewToken(beginMethod);
codestream.EmitLdArg(0);
codestream.Emit(ILOpcode.ldvirtftn, beginMethodToken);
codestream.Emit(ILOpcode.ldftn, beginMethodToken);
codestream.Emit(ILOpcode.bne_un, lOverridden);

Should we introduce an approach available externally to detect overrides? There are two possible approaches I can see:

Enable ldvirtftn and instance function pointers in C#

Basically it's allowing to write the aforementioned IL in C#. There's nothing more to handle for codegen and will just work. However, there may need more effort to ensure the usage of instance function pointers work fine for broad cases.

Expose a intrinsic helper for checking overrides

This enables more opportunities like constant folding when the type is known, and reduces the risk of inappropriate function pointers. However, currently there's no ideal way to pass a method in C# (lack of methodof). The ability to express the method is questionable.

Is there any interest to provide this functionality at all? Would there be more risks about it?

@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Jan 4, 2025
@dotnet-policy-service dotnet-policy-service bot added the untriaged New issue has not been triaged by the area owner label Jan 4, 2025
@huoyaoyuan huoyaoyuan added area-System.Runtime and removed needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners labels Jan 4, 2025
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-runtime
See info in area-owners.md if you want to be subscribed.

@KalleOlaviNiemitalo
Copy link

Such a feature was discussed in #12760.

@jkotas
Copy link
Member

jkotas commented Jan 4, 2025

There's nothing more to handle for codegen and will just work.

That's not correct. Function pointers are not guaranteed to be stable across all runtime flavors. This only happens to work for runtime flavors where function pointers are stable (e.g. native AOT).

@MichalPetryka
Copy link
Contributor

Would #94975 solve this with VirtualMethodHandle while still being friendly to all runtimes?

@huoyaoyuan
Copy link
Member Author

Would #94975 solve this with VirtualMethodHandle while still being friendly to all runtimes?

It should be functional with stable, comparable handles, but may not be as performant as accessing virtual slots directly. Currently in coreclr, function pointer comparison is used as a fast path, then MethodDesc. Further optimizations should be nice to have.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-System.Runtime untriaged New issue has not been triaged by the area owner
Projects
None yet
Development

No branches or pull requests

4 participants