IOCP Thread Usage Inconsistency in TcpNetworkHandlerBase - Seeking Design Rationale

[Vivian5510]

Question: IOCP Thread Usage Inconsistency in TcpNetworkHandlerBase - Seeking Design Rationale

Hi Garnet team! 👋

I've been studying the Garnet networking code and found an interesting design pattern in
TcpNetworkHandlerBase.cs that I'd like to understand better. I'm hoping the team can provide some insights into
the design rationale.

The Pattern I'm Questioning

File: libs/common/Networking/TcpNetworkHandlerBase.cs
Method: Start(bool useTLS)

void Start(bool useTLS)
{
    // ... setup code ...

    try
    {
        if (!socket.ReceiveAsync(receiveEventArgs))
        {
            // Synchronous completion - uses Task.Run
            if (useTLS)
                Task.Run(() => RecvEventArgCompletedWithTLS(null, receiveEventArgs));
            else
                Task.Run(() => RecvEventArgCompletedWithoutTLS(null, receiveEventArgs));
        }
    }
    // ... error handling ...
}

// But for asynchronous completion:
void RecvEventArgCompletedWithTLS(object sender, SocketAsyncEventArgs e) =>
    _ = HandleReceiveWithTLSAsync(sender, e);

void RecvEventArgCompletedWithoutTLS(object sender, SocketAsyncEventArgs e) =>
    HandleReceiveWithoutTLS(sender, e);

My Understanding and Concerns

The Comment's Promise

// If the client already have packets, avoid handling it here on the handler so we don't block future accepts.

This comment suggests the goal is to avoid blocking future accepts, which makes perfect sense for a
high-performance server.

The Inconsistency I See

For synchronous completion: Uses Task.Run() → moves to ThreadPool → IOCP thread returns immediately ✅

For asynchronous completion: Direct execution → stays on IOCP thread → IOCP thread is occupied ❌

The Core Question

If we have a limited IOCP thread pool (say 4 threads), and multiple connections are processing data
simultaneously:

IOCP Thread 1: Accept → sync completion → Task.Run → available for more I/O ✅
IOCP Thread 2: Accept → async completion → processing data → occupied ❌
IOCP Thread 3: Accept → async completion → processing data → occupied ❌
IOCP Thread 4: Accept → async completion → processing data → occupied ❌

Result: Only 1 IOCP thread available for new accepts and other I/O operations.

Doesn't this still risk blocking future accepts, just using a different IOCP thread?

Alternative Approach

Would it be more consistent to always use ThreadPool for data processing?

void RecvEventArgCompletedWithTLS(object sender, SocketAsyncEventArgs e) =>
    Task.Run(() => HandleReceiveWithTLSAsync(sender, e));

void RecvEventArgCompletedWithoutTLS(object sender, SocketAsyncEventArgs e) =>
    Task.Run(() => HandleReceiveWithoutTLS(sender, e));

This would ensure:

• All IOCP threads remain available for I/O operations
• Consistent threading model
• Better alignment with the stated goal of not blocking accepts

Questions for the Team

1. Is there a specific performance reason for handling async completion directly on IOCP threads?
2. Are there scenarios where RESP protocol processing is expected to be so fast that IOCP thread occupation isn't
   a concern?
3. Would the suggested unified approach introduce other issues I'm not seeing?
4. Is this perhaps legacy code that could benefit from modernization?

Context

I'm asking this from a learning perspective - I'm genuinely curious about the design decisions and whether there
might be room for improvement. The Garnet codebase is impressive, and I want to understand the nuances better.

Thanks for any insights you can provide! 🙏

---

Environment:

• Garnet version: latest main branch
• Focus area: High-concurrency networking performance