The tests are failing because ArgumentException has different parameter name. I can re-wrap the exception or change the CoreFX test. None of the names are actually correct name of the parameter. CoreFX test expects null, which CoreCLR used to return. In new code GCHandle throws the exception and it uses value as parameter name. The actual name of the parameter in the public function is userData.

The last remaining native function is CheckVMForIOPacket. As far as I can see it is an optimization that tries to improve performance of multiple packets being processed in succession. It seems it could be avoided completely if the check is moved just behind this line and kept directly in the thread pool:

(or in BindIoCompletionCallBack_Worker)

Not sure what condition is handled by the if (overlapped->m_callback == NULL) branch:

/cc @jkotas

CheckVMForIOPacket As far as I can see it is an optimization

Yes, it is an perf optimization. As with any perf optimization, any changes to it should be measured.

change the CoreFX test.

It is fine to update the CoreFX tests to expect the correct argument name. (Disable them in https://github.com/dotnet/coreclr/blob/master/tests/CoreFX/CoreFX.issues.json to make the CoreCLR PR green.)

cc @kouvel

My benchmark machine is broken, so I cannot re-run the benchmark to test for regression. Code is linked in the PR description if anyone wants to give it a go.

The benchmark is definitely triggering some code that results in quite unreliable performance. Here are some results I managed to get:

Before:

After:

The problem is that during the benchmark runs there are some quite visible stalls like this:

WorkloadActual  27: 65536 op, 791229500.00 ns, 12.0732 us/op
WorkloadActual  28: 65536 op, 807996900.00 ns, 12.3291 us/op
WorkloadActual  29: 65536 op, 3896247700.00 ns, 59.4520 us/op
WorkloadActual  30: 65536 op, 788609100.00 ns, 12.0332 us/op
WorkloadActual  31: 65536 op, 972309200.00 ns, 14.8363 us/op

While most of the results are within 11-15 us/op there's significant number of outliers which are 49-72 us/op. It could be the thread pool code, GC, or something else, but it happens both before and after my changes.

While most of the results are within 11-15 us/op there's significant number of outliers which are 49-72 us/op

The benchmark seems to be dominated by the thread pool overhead that can vary for the reasons you have mentioned. How much of this number is the threadpool overhead vs. actual cost of overlapped - what would the numbers be for raw alloc/free (e.g. #18360 (comment))?

The fixed cost of Overlapped did show up during socket performance investigations, e.g. #10302 or #21320.

I'll benchmark the pure Pack/Unpack, but it's not fully representative since it will not show the costs incurred during garbage collection for the old code.

Reduced/fixed benchmark code:

using BenchmarkDotNet.Attributes;
using BenchmarkDotNet.Running;
using System;
using System.Threading;

namespace OverlappedBench
{
    [InProcess]
    public class Program
    {
        [Benchmark]
        public unsafe void AllocOverlappedNull() => AllocOverlapped(null);
        [Benchmark]
        public unsafe void AllocOverlappedObject() => AllocOverlapped(IntPtr.Zero);
        [Benchmark]
        public unsafe void AllocOverlappedArray() => AllocOverlapped(new[] { IntPtr.Zero, IntPtr.Zero, IntPtr.Zero });

        private unsafe void AllocOverlapped(object userObject)
        {
            Overlapped ov = new Overlapped();
            NativeOverlapped* nativeOverlapped = ov.Pack(null, userObject);
            Overlapped.Unpack(nativeOverlapped);
            Overlapped.Free(nativeOverlapped);
        }

        static void Main(string[] args)
        {
            BenchmarkRunner.Run<Program>();
        }
    }
}

Before:

After:

Note that even with the updated benchmark I still get at least one time spike within the run of three benchmarks, so there could still be something wrong. Happens both before/after the changes, it is filtered out by BDN. Here's one example from warmup phase, but it happened during actual benchmark run too:

WorkloadWarmup   1: 4194304 op, 703022900.00 ns, 167.6137 ns/op
WorkloadWarmup   2: 4194304 op, 691389500.00 ns, 164.8401 ns/op
WorkloadWarmup   3: 4194304 op, 2717302000.00 ns, 647.8553 ns/op
WorkloadWarmup   4: 4194304 op, 2700309200.00 ns, 643.8039 ns/op
WorkloadWarmup   5: 4194304 op, 2711021400.00 ns, 646.3579 ns/op
WorkloadWarmup   6: 4194304 op, 683177200.00 ns, 162.8821 ns/op

Does the AllocOverlappedArray actually hit the object[] array path? I think it needs to be

byte[] buffer = new byte[1];
AllocOverlapped(new object[] { buffer, buffer, buffer });

Also, I am not able to replicate these results with low-tech microbenchmarks. For example:

int start = Environment.TickCount;
for (int i = 0; i < 10000000; i++)
{
    object userObject = new IntPtr();
    Overlapped ov = new Overlapped();
    NativeOverlapped* nativeOverlapped = ov.Pack(null, userObject);
    Overlapped.Unpack(nativeOverlapped);
    Overlapped.Free(nativeOverlapped);
}
int end = Environment.TickCount;
Console.WriteLine((end-start).ToString());

Before: 1.9s
After: 12.2s

I'd expect the performance to be different, so I think there's something wrong with my benchmarking. BDN 0.14 dropped support for direct configurations with local CoreCLR, so I had to switch to using the dotnet run -c Release -- --coreRun <path>. I wasn't sure it was working, so I started to print using reflection the layout of the Overlapped class to ensure that it was really different before and after the changes.

I'll run the low-tech benchmark locally, but the different on your run seems to be too big to be explained by tiered compilation.

Low-tech benchmark confirms your numbers. I get the same numbers as you on my machine.

I'll just close this as an unsuccessful attempt and perhaps revisit it some other day. I'll also try to get my BDN infrastructure working again because it obviously broke with updates to latest .NET Core / BDN versions and my attempts to get it working didn't succeed (the --coreRun trick from https://benchmarkdotnet.org/articles/configs/toolchains.html no longer works on my machine).

Thanks for all the help and sorry for wasting so much time on this.

No problem. It was an interesting experiment.