Remove all explicit GC Poll calls in unmanaged code.

[AaronRobinsonMSFT]

Remove explicit GC Poll from GC.SuppressFinalize().
Remove GC Poll from FCall and move it to managed, Buffer.__BulkMoveWithWriteBarrier().
Remove unneccessary explicit FC_GC_POLL() calls.
Remove unneccessary explicit FC_GC_POLL_RET() calls.
Remove unmanaged GCPoll implementation.
Implement JIT intrinsic for GC Poll, Thread.FastGCPoll.

Fixes #113807

[AaronRobinsonMSFT]

@EgorBot -windows_x64 -arm64

using System;
using System.Runtime.CompilerServices;

using BenchmarkDotNet.Attributes;

public unsafe class Bench
{
    private F _f;
    private byte[] _source;
    private byte[] _dest;
    private delegate* <ref byte, ref byte, nuint, void> _method;

    class F
    {
        ~F() { }
    }

    public Bench()
    {
        _f = new F();
        _source = new byte[10 * 1024];
        _dest = new byte[_source.Length];
        Random random = new Random();
        random.NextBytes(array);

        var method = typeof(Buffer).GetMethod("BulkMoveWithWriteBarrier", BindingFlags.NonPublic | BindingFlags.Static);
        _method = (delegate* <ref byte, ref byte, nuint, void>)method.MethodHandle.GetFunctionPointer();
    }

    [Benchmark]
    public void BenchmarkBulkMoveWithWriteBarrier()
    {
        _method(ref _dest[0], ref _source[0], (nuint)dest.Length);
    }

    [Benchmark]
    public void BenchmarkSuppressFinalize()
    {
        GC.SuppressFinalize(_f);
    }
}

[AaronRobinsonMSFT]

@EgorBot -intel -arm

using System;
using System.Reflection;
using System.Runtime.CompilerServices;

using BenchmarkDotNet.Attributes;

public unsafe class Bench
{
    private F _f;
    private byte[] _source;
    private byte[] _dest;
    private delegate* <ref byte, ref byte, nuint, void> _method;

    class F
    {
        ~F() { }
    }

    public Bench()
    {
        _f = new F();
        _source = new byte[10 * 1024];
        _dest = new byte[_source.Length];
        Random random = new Random();
        random.NextBytes(_source);

        var method = typeof(Buffer).GetMethod("BulkMoveWithWriteBarrier", BindingFlags.NonPublic | BindingFlags.Static)!;
        _method = (delegate* <ref byte, ref byte, nuint, void>)method.MethodHandle.GetFunctionPointer();
    }

    [Benchmark]
    public void BenchmarkBulkMoveWithWriteBarrier()
    {
        _method(ref _dest[0], ref _source[0], (nuint)_dest.Length);
    }

    [Benchmark]
    public void BenchmarkSuppressFinalize()
    {
        GC.SuppressFinalize(_f);
    }
}

[jkotas]

@EgorBot -intel -arm

using System;
using System.Reflection;
using System.Runtime.CompilerServices;

using BenchmarkDotNet.Attributes;

public unsafe class Bench
{
    private object[] _source;
    private object[] _dest;

    public Bench()
    {
        _source = new object[10];
        _dest = new object[_source.Length];
    }

    [Benchmark]
    public void BenchmarkBulkMoveWithWriteBarrier()
    {
        _source.AsSpan().CopyTo(_dest.AsSpan());
    }
}

[jkotas]

The kind of benchmark to run for BulkMoveWithWriteBarrier ^

[AaronRobinsonMSFT]

/cc @dotnet/jit-contrib

[Copilot]

Pull Request Overview

This pull request removes unnecessary explicit GC Poll calls in unmanaged code and moves this logic to managed code while also converting certain FCall methods to managed implementations.

• In GC.CoreCLR.cs, the SuppressFinalize method is modified to only call _SuppressFinalize if the object has a finalizer.
• In Thread.CoreCLR.cs, the unmanaged ThreadNative_PollGC call is replaced by a managed PollGCInternal wrapped by an inline PollGCWorker function.
• In Buffer.CoreCLR.cs, an additional using directive for System.Threading is added.

Reviewed Changes

Copilot reviewed 5 out of 22 changed files in this pull request and generated 1 comment.

File	Description
src/coreclr/System.Private.CoreLib/src/System/GC.CoreCLR.cs	Updates SuppressFinalize with a finalizer check to avoid unnecessary calls.
src/coreclr/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs	Renames and wraps the GC poll call to move its implementation to managed code.
src/coreclr/System.Private.CoreLib/src/System/Buffer.CoreCLR.cs	Adds a using directive for System.Threading to support related changes.

Files not reviewed (17)

• src/coreclr/classlibnative/bcltype/objectnative.cpp: Language not supported
• src/coreclr/jit/compiler.h: Language not supported
• src/coreclr/jit/compiler.hpp: Language not supported
• src/coreclr/jit/flowgraph.cpp: Language not supported
• src/coreclr/jit/gentree.cpp: Language not supported
• src/coreclr/jit/gtlist.h: Language not supported
• src/coreclr/jit/ifconversion.cpp: Language not supported
• src/coreclr/jit/importercalls.cpp: Language not supported
• src/coreclr/jit/liveness.cpp: Language not supported
• src/coreclr/jit/namedintrinsiclist.h: Language not supported
• src/coreclr/jit/optcse.cpp: Language not supported
• src/coreclr/jit/rationalize.cpp: Language not supported
• src/coreclr/jit/valuenum.cpp: Language not supported
• src/coreclr/vm/comutilnative.cpp: Language not supported
• src/coreclr/vm/comutilnative.h: Language not supported
• src/coreclr/vm/fcall.cpp: Language not supported
• src/coreclr/vm/fcall.h: Language not supported

Comments suppressed due to low confidence (1)

src/coreclr/System.Private.CoreLib/src/System/Threading/Thread.CoreCLR.cs:509

• [nitpick] The local function 'PollGCWorker' could be renamed to better reflect that it is a simple wrapper for PollGCInternal; consider a more descriptive name if it improves clarity.

static void PollGCWorker() => PollGCInternal();

[AaronRobinsonMSFT]

@jkotas I've confirmed locally that the use of the new CatchAtSafePoint() FCall does make suspension more stable. With the previous implementation I was able to observe the instability you observed as well. I'm considering the approach here as addressing the issue and fixing #113807.

[jkotas]

The fix LGTM assuming that the CI passes.

[AaronRobinsonMSFT]

For some reason, with this patch, it didn't repro in 10+ attempts:

@am11 Thanks. I've applied it and that does indeed "fix" the issue.

[am11]

I've applied it and that does indeed "fix" the issue.

Reported upstream for a proper fix DotNetAnalyzers/StyleCopAnalyzers#3922 :)