Call Finalize method directly from the finalization loop.

[VSadov]

Inspired by #118563

With an UnsafeAccessor we can simply call Finalize, no need to dig for the method slot, etc..

Not yet sure if this way is faster. It is certainly simpler.

[VSadov]

A failure in ILC/JitInterface when calling object.Finalize(). @MichalStrehovsky - any idea why?
It should be just a calvirt of a method...
(I'd even hope to remove some specialcasing of the Finalize method slot with this)

Unhandled exception. System.AggregateException: One or more errors occurred. (Code generation failed for method '[S.P.CoreLib]System.Runtime.__Finalizer.DrainQueue()')
   ---> ILCompiler.CodeGenerationFailedException: Code generation failed for method '[S.P.CoreLib]System.Runtime.__Finalizer.DrainQueue()'
   ---> System.InvalidOperationException: object.Finalize()
     at Internal.JitInterface.CorInfoImpl.getMethodVTableOffset(CORINFO_METHOD_STRUCT_*, UInt32&, UInt32&, Boolean&) + 0x248
     at Internal.JitInterface.CorInfoImpl._getMethodVTableOffset(IntPtr, IntPtr*, CORINFO_METHOD_STRUCT_*, UInt32*, UInt32*, Boolean*) + 0x56
     --- End of inner exception stack trace ---
     at Internal.JitInterface.CorInfoImpl.CompileMethodInternal(IMethodNode, MethodIL) + 0x20d
     at Internal.JitInterface.CorInfoImpl.CompileMethod(MethodCodeNode, MethodIL) + 0x5c
     at ILCompiler.RyuJitCompilation.CompileSingleMethod(CorInfoImpl, MethodCodeNode) + 0xa9
     at ILCompiler.RyuJitCompilation.CompileSingleMethod(MethodCodeNode) + 0xd4
     at System.Threading.Tasks.Parallel.<>c__DisplayClass19_0`2.<ForWorker>b__1(RangeWorker&, Int64, Boolean&) + 0x30c
  --- End of stack trace from previous location ---
     at System.Threading.Tasks.Parallel.<>c__DisplayClass19_0`2.<ForWorker>b__1(RangeWorker&, Int64, Boolean&) + 0x4b9
     at System.Threading.Tasks.TaskReplicator.Replica.Execute() + 0x6b
     --- End of inner exception stack trace ---
     at System.Threading.Tasks.TaskReplicator.Run[TState](TaskReplicator.ReplicatableUserAction`1, ParallelOptions, Boolean) + 0x14c
     at System.Threading.Tasks.Parallel.ForWorker[TLocal,TInt](TInt, TInt, ParallelOptions, Action`1, Action`2, Func`4, Func`1, Action`1) + 0x2fe
  --- End of stack trace from previous location ---
     at System.Threading.Tasks.Parallel.ThrowSingleCancellationExceptionOrOtherException(ICollection, CancellationToken, Exception) + 0x30
     at System.Threading.Tasks.Parallel.ForWorker[TLocal,TInt](TInt, TInt, ParallelOptions, Action`1, Action`2, Func`4, Func`1, Action`1) + 0x434
     at ILCompiler.RyuJitCompilation.CompileMultiThreaded(List`1) + 0x23f
     at ILCompiler.RyuJitCompilation.ComputeDependencyNodeDependencies(List`1) + 0x156
     at ILCompiler.DependencyAnalysisFramework.DependencyAnalyzer`2.ComputeMarkedNodes() + 0xaa
     at ILCompiler.RyuJitCompilation.CompileInternal(String, ObjectDumper) + 0x26
     at ILCompiler.Compilation.ILCompiler.ICompilation.Compile(String, ObjectDumper) + 0x3f
     at ILCompiler.Program.Run() + 0x2bd9
     at ILCompiler.ILCompilerRootCommand.<>c__DisplayClass261_0.<.ctor>b__0(ParseResult) + 0x31f
     at System.CommandLine.Invocation.InvocationPipeline.Invoke(ParseResult) + 0xd2
     at ILCompiler.Program.Main(String[] args) + 0xfa

[MichalStrehovsky]

It should be just a calvirt of a method...

We don't assign a vtable slot for Finalize method in native AOT since it can only be called virtually from IL (or UnsafeAccessor now) and nobody outside the finalizer thread should really do it. This saves 8 bytes per MethodTable in terms of working set and size since everything derived from Object would need to have it, finalizable or not.

[VSadov]

It should be just a calvirt of a method...

We don't assign a vtable slot for Finalize method in native AOT since it can only be called virtually from IL (or UnsafeAccessor now) and nobody outside the finalizer thread should really do it. This saves 8 bytes per MethodTable in terms of working set and size since everything derived from Object would need to have it, finalizable or not.

It seems a bit dangerous/observable. It is tempting though to save on an object's v-slot that is rarely overriden.

Perhaps we could put the Finalize at slot -1, but allocate that slot only for object itself and for whoever overrides Finalize. This way Finalize would be directly and virtually callable, at least for objects that override it.

Won't work though, since methodtable has stuff before slots.

Actually might still work, just the offset of the slot would need to be -1 relative to the MT ptr.
Except, even that space is already taken - by the GCDesc stuff ...

[VSadov]

I think I will do the change only for CoreCLR for now and we can think if/how it can be done for NativeAOT as well.

[jkotas]

since Finalize is called from managed code, no need to check for .cctor

I am not following the reasoning here. I think this is trying to make sure that the following won't print Here!. I think it is going to print Here! with your change:

for (;;) try { GC.KeepAlive(new Finalizable()); } catch { }

class Finalizable
{
    static Finalizable()
    {
        throw new Exception();
    }

    ~Finalizable()
    {
        Console.WriteLine("Here!");
    }
}

I would expect that we have a test somewhere for this.

[VSadov]

Yes, I am looking at this right now. I am not sure that can be deleted.
I did not see similar code in NativeAOT, so I assumed that just running Finalizer through managed code would take care of cctor.

But for the finalizer to run we only need to have an object allocated (since GC does not care if either instance or static constructor have run).

Now I am a bit puzzled where the NativeAOT counterpart to this code is.

[VSadov]

NativeAOT prints:

Here!
Here!
Here!
Here!
Here!
Here!
. . .
. . .
Here!
Here!
Here!
Here!
Here!
Here!

Process is terminating due to StackOverflowException.

[VSadov]

I think the StackOverflowException is a separate issue. It happens even if ~Finalizable() is commented out.

[jkotas]

Now I am a bit puzzled where the NativeAOT counterpart to this code is.

NativeAOT has a bug.

[jkotas]

GC does not care if either instance or static constructor have run

Right, GC does not care. This is about compliance with ECMA spec.

[jkotas]

I think the StackOverflowException is a separate issue

Opened #118913. Any chance you can look into that? It looks like a bug we want to fix for .NET 10.

[dotnet-policy-service]

Draft Pull Request was automatically closed for 30 days of inactivity. Please let us know if you'd like to reopen it.