Skip to content

Improvements for Delegate.GetHashCode() for CsWinRT (and NativeAOT) #96947

@Sergio0694

Description

@Sergio0694

Overview

We're working on improving the binary size of assemblies using CsWinRT and NativeAOT, and talking with @MichalStrehovsky we noticed that the new trimming optimization to stop delegate targets from being considered reflection-visible by default (#96166) is not kicking in at all, as it's blocked by the use of Delegate.Method in our EventRegistrationTokenTable<T> type (see here).

We need to calculate a "good" pseudo-random value for a given delegate, which will be the starting value for an event registration token passed to the WinRT side. Currently we're using Delegate.GetHashCode() only when the delegate is multicast, because unfortunately when that is not the case, the returned hashcode only depends on the target of the delegate, and on the delegate type, but not on the specific method being wrapped by the delegate:

if (_methodPtrAux == IntPtr.Zero)
return (_target != null ? RuntimeHelpers.GetHashCode(_target) * 33 : 0) + GetType().GetHashCode();
else
return GetType().GetHashCode();

The way we're working around that is to check if the delegate is unicast, and in that case do handler.Method.GetHashCode(), which breaks that size optimization entirely. I can't really think of another way of getting some other value from the delegate without touching Method at all. Additionally, Michal also mentioned that the GetHashCode() implementation on NativeAOT could be improved.

Dropping that Method property entirely saves over 200 KB in a minimal WinRT component sample:

image (35)

I don't have an exact ask here, so opening this more to try to figure out what the best approach to this could be. For instance, would it be possible to change the Delegate.GetHashCode() implementation for unicast delegates to also account for the method being invoked? Or, if that was a nonstarter due to the overhead it would add to all uses (even when callers would be fine with the current behavior), could perhaps a new API make sense to compute a "better" hashcode for a delegate? Strawman idea:

namespace System.Runtime.CompilerServices;

public static class RuntimeHelpers
{
    public static int GetDelegateHashCode(Delegate? d);
}

This could compute a "better", slightly more expensive hashcode, that would combine all target methods as well.
Alternatively, could there be some other way of retrieving a "hashcode of the target method" that's trim friendly?

To also try things from another angle — @AaronRobinsonMSFT @jkoritzinsky I know you have experience in this area specifically (I mean WinRT event handling in general). Would you know whether it might be possible to actually just stop this approach entirely and not just, for instance, use a completely random value for the lower 32 bits instead? Reading all the comments in CsWinRT it's not really clear why that's not already being done. The comments say the current approach "will also give us a shot at preventing unregistration rom becoming an O(N) operation", but given the unregistration is just performed with a token that's coming from outside this class (ie. the one that callers would've stored when registering), it's not clear to me at all how this would relate to how that initial token value was calculated, and especially to whether it was a pseudo-random value depending on the exact delegate instance or not.

To clarify: the goal here is to drop that Method use in CsWinRT to allow the optimization in #96166 to work. I'd be happy with any approach that would allow that, be it some new API, some other equivalent workaround, or perhaps just a different logic to compute that token entirely from CsWinRT's side, assuming that didn't cause other problems or performance regressions.

Also cc. @manodasanW

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    Status

    No status

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions