[Wip] Improve Threadpool QUWI throughput

[benaadams]

11% Improvement for regular queuing (1.6sec over 10M QUWI)
19.4% improvement for high thread count queuing (MinWorkerThreads=500) (7.2sec over 10M QUWI)

Test project: https://gist.github.com/benaadams/b022934e62a3ac1c4f261be3216b1111

10M threadpool queues and executes. Changed items in red, ExecutionContext.Run highlighted and list past ExecutionContext.Restore for relative comparison.

Threadpool QUWI before

Threadpool QUWI after 2nd Update

[stephentoub]

Why are you 64-byte padding the items in the queue? The queue is owned by a single thread, and all other threads need to take a lock to access it. The owning thread also needs to take a lock when there are fewer than two items in the queue. The only case where you'd have contention this would help with is if there was a thread stealing concurrently with the owning thread pushing/popping on a list with at least two elements. At that point, they're already some distance apart, though not necessarily a full cache line. Have you shown that this change makes a notable improvement? It does so at the expense of effectively increasing the size of every work item by 56 bytes on 64-bit, since every work item reference to be stored now consumes 64 bytes instead of 8 bytes (plus the size of the work item object itself).

[benaadams]

@stephentoub as you pointed out don't think padding the items is helpful

Still working on it - hot spots are Dequeue and TrySteal

[benaadams]

Looking closer the main effect may just be looping the queues (many threads) with mainly empty queues.

[omariom]

@benaadams I think you should start from finding false sharings. In this article shown how to use VS for that. Not sure if it possible on Windows 10,but on Win 7 it was.

update: Another good tool Intel VTune
Not sure if it works with Windows 10.

[benaadams]

@omariom there is false sharing in stealing; however the current implementation even with the false sharing is pretty hard to improve on. Still iterating, though looking at something quite different than the PR in this current state.

[omariom]

Interesting optimization would be to find all the places (on hot pathes) where volatile reads/writes are unnecessary, replace them with plain reads and use Volatile class in the rest.
It may help on ARM where volatile reads/writes are implemented as fairly expensive full barriers.

[benaadams]

Interesting optimization would be to find all the places (on hot pathes) where volatile reads/writes are unnecessary, replace them with plain reads and use Volatile class in the rest.

There are some areas where this might be possible. Will try it and measure the impact.

Although such a change does make me a little uncomfortable 😟

[benaadams]

Bit better, getting a 6% improvement for QueueUserWorkItem throughput for 10M work items (4 core 1 socket)
With 9% improvement for set COMPlus_ThreadPool_ForceMinWorkerThreads=500

Still investigating.

[benaadams]

10% Improvement for regular (13.1s vs 14.7s)
14% improvement for set COMPlus_ThreadPool_ForceMinWorkerThreads=500 (31.2s vs 36.6s)

[benaadams]

10M threadpool queues and executes. Changed items in red, ExecutionContext.Run highlighted and list to System.Random.Sample for relative comparison.

Threadpool QUWI before

Threadpool QUWI after

[benaadams]

Test code https://gist.github.com/benaadams/b022934e62a3ac1c4f261be3216b1111

It also allocates 2112 bytes per 255 queued items in discarded QueueSegments (85MB per 10M) - which give a process equilibrium at 300MB-400MB memory use vs 50MB equilibrium without these allocations.

Caching a QueueSegment as it gets dropped off the tail and the reusing it as it for a new head avoids these allocations but its also not entirely straightforward with the concurrency flows; so not perusing that at this stage.

I believe the changes in this PR should not alter any of the concurrency behaviour.

[prajwal-aithal]

@dotnet-bot test Linux ARM Emulator Cross Debug Build

[benaadams]

@dotnet-bot test Linux ARM Emulator Cross Release Build

[benaadams]

@dotnet-bot test this please

[benaadams]

Added QueueSegment reuse as commit, needs tests rerunning

[danmoseley]

@benaadams what remains here to call this PR good to go?

[benaadams]

@danmosemsft it needs to be freshened and rebased. I'll open as another PR with new results as there is a lot of noise now in this one.