Proposal: implement limited CHA in RyuJit and take into account 'sealed' keyword

[ciplogic]

Devirtualize more aggressively sealed classes:

Steps to Reproduce:

Most of times calling ToString(). GetHashCode() and co is converted to a virtual call. But by specification sealed methods and classes cannot have overriden methods, so if in context the virtual method cannot be overriden, it should be enforced to be devirtualized safely. There is one difference between calling a virtual method and a non-virtual method (the null pointer check) which should be still checked on this devirtualization.

Expected Behavior:

It should be expected that calling .ToString() to a provable sealed class (like against an Array classes, as byte[], should call a non-virtual method).

This snippet of code is inside a class which is not overriden anywhere:

        sealed class Internal
        {
            private const string Abc = "abc";

            public override string ToString()
            {
                return Abc;
            }

            public string ToStringStatic()
            {
                return Abc;
            }
        }

       [Test]
        public void TestNonVirtualSplit()
        {
            int count = 100000000;
            var inter = new Internal();
            var virtualCall = TimmingAction.TimeInMs(() => inter.ToString(), count);
            var staticCall = TimmingAction.TimeInMs(() => inter.ToStringStatic(), count);
            Console.WriteLine($"Time: virtual: {virtualCall} static: {staticCall}");
            Assert.IsTrue(virtualCall >= staticCall, "Static call should be faster");
        }

The output of this program is:

Time: virtual: 266 static: 188

(the code does a lambda capture,

Not only that static calls could be optimized, but RyuJit could have bigger devirtualization opportunities.

As a phase two of this proposal, it is that RyuJit to identify sealed classes which are in context of assemblies:

• internal classes which are not overriden inside the assembly could be considered sealed

• private classes (classes defined inside another classes) which are not overriden, could be considered sealed

This optimization step could add extra startup cost when loading new assemblies to check the Class Hierarchy Analysis, so maybe the marking of internal classes/priva classes as sealed, could be done on Roslyn side

I added a reverse link to RyuJIT repository

[HaloFour]

Changing the IL opcode emitted by Roslyn from callvirt to call would also omit the CLR-imposed null check, which is a part of the C# specification:

Internal inter = null;
string value = inter.ToStaticString(); // not only succeeds, also returns "abc"

[0xd4d]

internal classes which are not overriden inside the assembly could be considered sealed

It's a breaking change: friend assemblies exist.

[alrz]

An analyzer could suggest sealed keyword on such internal classes.

[jnm2]

So: inline the null check in IL?

[ciplogic]

@0xd4d It looks to me that Friend Assemblies have to be specially marked as InternalsVisibleToAttribute . So in fact it can be done this automatic sealing of classes, just that it has a bit more limited scope.

Instead of: "internal classes which are not overriden inside the assembly could be considered sealed", it could be read as: "internal classes which are not overriden inside the assembly could be considered sealed, excluding that the assembly is exported as a friend assembly, when this optimization would be disabled in the scope of the assembly".

Friend assembly documentation

[gafter]

The Roslyn compilers do not have control over what sequence of machine instructions are executed. The C# language specification binds the invocation of ToString to the declaration of the virtual method in object, and the compiler implements that specification. It has been suggested (#15929) that the compiler should generate code at odds with the requirements of the C# specification in order to produce improved (but observably incorrect in binary compatibility scenarios) code (see also #15644). If that is the suggestion here, this is a dup of #15929. If the suggestion is that the runtime should execute a better native instruction sequence, that does not belong in the Roslyn repository, but on the repository of the runtime you're interested in.