Skip to content

Commit

Permalink
Delete useless delegate virtual methods (#97959)
Browse files Browse the repository at this point in the history
The split between `Delegate` and `MulticastDelegate` is a wart that likely has some history behind it. Types that inherit from `Delegate` directly would not be considered delegates by the runtime. They need to inherit `MulticastDelegate. I can't find a reason why we'd need some useless base implementation of these methods that immediately gets overriden in `MulticastDelegate`. This deletes the useless base implementation and moves the useful implementation from `MulticastDelegate` to `Delegate`.

This along with #97951 saves ~40 bytes per delegate `MethodTable` because the virtual methods can now be devirtualized or placed into the sealed vtable. We might be able to do even more since technically sealed virtuals could be reshuffled after the codegen phase and slots eliminated then.
  • Loading branch information
MichalStrehovsky authored Feb 7, 2024
1 parent 0cbbe7e commit 47f325e
Show file tree
Hide file tree
Showing 10 changed files with 429 additions and 529 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,7 @@
<Compile Include="$(BclSourcesRoot)\System\IO\FileNotFoundException.CoreCLR.cs" />
<Compile Include="$(BclSourcesRoot)\System\Math.CoreCLR.cs" />
<Compile Include="$(BclSourcesRoot)\System\MathF.CoreCLR.cs" />
<Compile Include="$(BclSourcesRoot)\System\MulticastDelegate.cs" />
<Compile Include="$(BclSourcesRoot)\System\MulticastDelegate.CoreCLR.cs" />
<Compile Include="$(BclSourcesRoot)\System\Object.CoreCLR.cs" />
<Compile Include="$(BclSourcesRoot)\System\Reflection\Assembly.CoreCLR.cs" />
<Compile Include="$(BclSourcesRoot)\System\Reflection\AssemblyName.CoreCLR.cs" />
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,29 +14,14 @@ namespace System
{
[ClassInterface(ClassInterfaceType.None)]
[ComVisible(true)]
public abstract class MulticastDelegate : Delegate
public abstract partial class MulticastDelegate : Delegate
{
// This is set under 2 circumstances
// 1. Multicast delegate
// 2. Wrapper delegate
private object? _invocationList; // Initialized by VM as needed
private nint _invocationCount;

// This constructor is called from the class generated by the
// compiler generated code (This must match the constructor
// in Delegate
[RequiresUnreferencedCode("The target method might be removed")]
protected MulticastDelegate(object target, string method) : base(target, method)
{
}

// This constructor is called from a class to generate a
// delegate based upon a static method name and the Type object
// for the class defining the method.
protected MulticastDelegate([DynamicallyAccessedMembers(DynamicallyAccessedMemberTypes.All)] Type target, string method) : base(target, method)
{
}

internal bool IsUnmanagedFunctionPtr()
{
return _invocationCount == -1;
Expand Down Expand Up @@ -448,34 +433,6 @@ public sealed override Delegate[] GetInvocationList()
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static bool operator ==(MulticastDelegate? d1, MulticastDelegate? d2)
{
// Test d2 first to allow branch elimination when inlined for null checks (== null)
// so it can become a simple test
if (d2 is null)
{
return d1 is null;
}

return ReferenceEquals(d2, d1) ? true : d2.Equals((object?)d1);
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static bool operator !=(MulticastDelegate? d1, MulticastDelegate? d2)
{
// Can't call the == operator as it will call object==

// Test d2 first to allow branch elimination when inlined for not null checks (!= null)
// so it can become a simple test
if (d2 is null)
{
return d1 is not null;
}

return ReferenceEquals(d2, d1) ? false : !d2.Equals(d1);
}

public sealed override int GetHashCode()
{
if (IsUnmanagedFunctionPtr())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -183,7 +183,6 @@
<Compile Include="System\GC.NativeAot.cs" />
<Compile Include="System\Math.NativeAot.cs" />
<Compile Include="System\MathF.NativeAot.cs" />
<Compile Include="System\MulticastDelegate.cs" />
<Compile Include="System\Object.NativeAot.cs" />
<Compile Include="System\Resources\ManifestBasedResourceGroveler.NativeAot.cs" />
<Compile Include="System\RuntimeArgumentHandle.cs" />
Expand Down
Loading

0 comments on commit 47f325e

Please sign in to comment.