avoid allocating in Pipelines ThreadPoolScheduler

[JanEggers]

Im seeing some allocations I cannot get rid of:

can this avoid allocating QueueUserWorkItemCallbackDefaultContext?

like:

public class NonAllocThreadpoolScheduler : PipeScheduler, IThreadPoolWorkItem
    {
        private Action<object> _action;
        private object _state;

        public override void Schedule(Action<object> action, object state)
        {
            _action = action;
            _state = state;
            System.Threading.ThreadPool.UnsafeQueueUserWorkItem(this, false);
        }

        public void Execute()
        {
            _action(_state);
        }
    }

this scheduler would be created twice per pipe one for read and one for write, so there is always only one callback that needs to be scheduled.

[davidfowl]

It shouldn’t allocate on .NET core 3.0.

[JanEggers]

@davidfowl then there is something wrong im using net core 3.0

https://github.com/JanEggers/MQTTnet/blob/249d0d918b4536caa15bdcd51d2a90f145aaff22/Tests/MQTTnet.Benchmarks/MQTTnet.Benchmarks.csproj#L6

[davidfowl]

Maybe tiered compilation? I’m not sure we fixed that @benaadams @stephentoub , did we?

[benaadams]

If its the callback from an await on a ValueTask it won't allocate https://github.com/dotnet/coreclr/pull/21159

[stephentoub]

And this isn't, as it's calling Schedule rather than UnsafeSchedule.

[benaadams]

And in .NET 5.0; currently, if you set a flag the ValueTasks won't allocate Tasks when they suspend. Though feedback is being sought on that behaviour: https://github.com/dotnet/coreclr/issues/27403

[davidfowl]

Maybe a ns2.0 dependency or something pulling in the non NETCore version?

[benaadams]

And this isn't, as it's calling Schedule rather than UnsafeSchedule.

Ah, its the callback from IOCompletion. This is where ASP.NET Core uses the IOQueue as the scheduler

Which is the scheduler parameter in this bit of code (which is chosen from 1 of min(16, ProcCount) IOQueues in a static readonly array) https://github.com/aspnet/AspNetCore/blob/c1cc1684415f2aab86471230346856e8e09dcd43/src/Servers/Kestrel/Transport.Sockets/src/Internal/SocketConnection.cs#L57-L69

[benaadams]

Could make IOQueue the default in corefx and use Thread.GetCurrentProcessorId() % length to determine which from the array to use?

[benaadams]

Hmmm, though it has the caveat that it would suffer from head-of-line blocking if too much was done on a work item (preventing the other queued items in the IOQueue from being picked up by the ThreadPool)

[JanEggers]

here some output of the benchmark so you know jit and stuffs

BenchmarkDotNet=v0.12.0, OS=Windows 10.0.18362 Intel Core i7-4770K CPU 3.50GHz (Haswell), 1 CPU, 8 logical and 4 physical cores .NET Core SDK=3.0.100 [Host] : .NET Core 3.0.0 (CoreCLR 4.700.19.46205, CoreFX 4.700.19.46214), X64 RyuJIT

Toolchain=InProcessEmitToolchain

[davidfowl]

@JanEggers can you move this to Kestrel.

[JanEggers]

@davidfowl i dont understand im using kesterl in the benchmark:

https://github.com/JanEggers/MQTTnet/blob/71b5706988e5c0d9b1bc2726c3c1dff80a4ac24a/Tests/MQTTnet.Benchmarks/MessageProcessingMqttConnectionContextBenchmark.cs#L41

or do you mean move the issue? I think one of the team has to do that?

[davidfowl]

I think I know what this is now, I’m closing this issue. It’s nothing to do with pipelines specifically, just how it’s being used in the bedrock transports repo

[JanEggers]

@davidfowl how do I need to modify the code to avoid these allocations?

[davidfowl]

It's the code in here https://github.com/davidfowl/BedrockFramework/blob/master/src/Bedrock.Framework/Transports/Sockets/ that needs some tweaking.