reduce time spent holding http.sys transaction lock (#437)

* changed where calls to IssueNewRequestMaybe are made to reduce per-transaction lock contention
* minor: reduce stack frame for HttpSysTransaction::IoCompletionCallback

1 files changed, 25 insertions, 14 deletions

diff --git a/src/zenhttp/httpsys.cpp b/src/zenhttp/httpsys.cpp
index 60358d0b0..c7ed0bb2f 100644
--- a/src/zenhttp/httpsys.cpp
+++ b/src/zenhttp/httpsys.cpp
@@ -1111,12 +1111,13 @@ HttpSysServer::Run(bool IsInteractive)
 }
 
 void
-HttpSysServer::OnHandlingRequest()
+HttpSysServer::OnHandlingNewRequest()
 {
 	if (--m_PendingRequests > m_MinPendingRequests)
 	{
 		// We have more than the minimum number of requests pending, just let someone else
-		// enqueue new requests
+		// enqueue new requests. This should be the common case as we check if we need to
+		// enqueue more requests before exiting the completion handler.
 		return;
 	}
 
@@ -1242,6 +1243,16 @@ HttpSysTransaction::IssueInitialRequest(std::error_code& ErrorCode)
 thread_local bool		t_IsHttpSysThreadNamed = false;
 static std::atomic<int> HttpSysThreadIndex	   = 0;
 
+static void
+NameCurrentHttpSysThread()
+{
+	t_IsHttpSysThreadNamed						 = true;
+	const int						 ThreadIndex = ++HttpSysThreadIndex;
+	zen::ExtendableStringBuilder<16> ThreadName;
+	ThreadName << "httpio_" << ThreadIndex;
+	SetCurrentThreadName(ThreadName);
+}
+
 void
 HttpSysTransaction::IoCompletionCallback(PTP_CALLBACK_INSTANCE Instance,
 										 PVOID				   pContext /* HttpSysServer */,
@@ -1250,19 +1261,13 @@ HttpSysTransaction::IoCompletionCallback(PTP_CALLBACK_INSTANCE Instance,
 										 ULONG_PTR			   NumberOfBytesTransferred,
 										 PTP_IO				   Io)
 {
-	UNREFERENCED_PARAMETER(Io);
-	UNREFERENCED_PARAMETER(Instance);
-	UNREFERENCED_PARAMETER(pContext);
+	ZEN_UNUSED(Io, Instance);
 
 	// Assign names to threads for context
 
 	if (!t_IsHttpSysThreadNamed)
 	{
-		t_IsHttpSysThreadNamed						  = true;
-		const int						  ThreadIndex = ++HttpSysThreadIndex;
-		zen::ExtendableStringBuilder<128> ThreadName;
-		ThreadName << "httpio_" << ThreadIndex;
-		SetCurrentThreadName(ThreadName);
+		NameCurrentHttpSysThread();
 	}
 
 	// Note that for a given transaction we may be in this completion function on more
@@ -1275,6 +1280,15 @@ HttpSysTransaction::IoCompletionCallback(PTP_CALLBACK_INSTANCE Instance,
 	{
 		delete Transaction;
 	}
+
+	// Ensure new requests are enqueued as necessary. We do this here instead
+	// of inside the transaction completion handler now to avoid spending time
+	// in unrelated API calls while holding the transaction lock
+
+	if (HttpSysServer* HttpServer = reinterpret_cast<HttpSysServer*>(pContext))
+	{
+		HttpServer->IssueNewRequestMaybe();
+	}
 }
 
 bool
@@ -1336,7 +1350,7 @@ HttpSysTransaction::HandleCompletion(ULONG IoResult, ULONG_PTR NumberOfBytesTran
 		if ((CurrentHandler == &m_InitialHttpHandler) && m_InitialHttpHandler.IsInitialRequest())
 		{
 			// Ensure we have a sufficient number of pending requests outstanding
-			m_HttpServer.OnHandlingRequest();
+			m_HttpServer.OnHandlingNewRequest();
 		}
 
 		auto NewCompletionHandler = CurrentHandler->HandleCompletion(IoResult, NumberOfBytesTransferred);
@@ -1344,9 +1358,6 @@ HttpSysTransaction::HandleCompletion(ULONG IoResult, ULONG_PTR NumberOfBytesTran
 		IsRequestPending = IssueNextRequest(NewCompletionHandler);
 	}
 
-	// Ensure new requests are enqueued as necessary
-	m_HttpServer.IssueNewRequestMaybe();
-
 	if (IsRequestPending)
 	{
 		// There is another request pending on this transaction, so it needs to remain valid