From 426b752953a08ea758d84ddf3e93c0aab0a4acb6 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Thu, 13 Jul 2023 23:56:48 +0900 Subject: [PATCH 1/2] Reduce overhead in `ManagedNamedPipeClient` during connection loop The implementation of `NamedPipeClientStream` has some egregious use of `SpinOnce`: https://github.com/dotnet/runtime/blob/d59af2cf097acb100ad6a9cba652be34be6f4a2e/src/libraries/System.IO.Pipes/src/System/IO/Pipes/NamedPipeClientStream.cs#L151-L155 This adds perceivable overhead to the connect process, which we really don't want on a background thread. It is especially noticeable when discord is not running and the reconnect process is constantly running in the background. To get around this, we can request a connect timeout of 0 ms from the underlying API. Checking the implementations, this seems like a valid method, as long as there is external retry logic in place (which there is in the local class). Tested on Windows, macOS and linux. --- DiscordRPC/IO/ManagedNamedPipeClient.cs | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/DiscordRPC/IO/ManagedNamedPipeClient.cs b/DiscordRPC/IO/ManagedNamedPipeClient.cs index 6f4e1124..328bfa8d 100644 --- a/DiscordRPC/IO/ManagedNamedPipeClient.cs +++ b/DiscordRPC/IO/ManagedNamedPipeClient.cs @@ -140,6 +140,9 @@ private bool AttemptConnection(int pipe, bool isSandbox = false) { Logger.Info("Attempting to connect to '{0}'", pipename); _stream = new NamedPipeClientStream(".", pipename, PipeDirection.InOut, PipeOptions.Asynchronous); + + // Intentionally use a timeout of 0 here to avoid spinlock overhead. + // We are already performing local retry logic, so this is not required. _stream.Connect(1000); //Spin for a bit while we wait for it to finish connecting @@ -378,7 +381,7 @@ public void Close() return; } - //flush and dispose + //flush and dispose try { //Wait for the stream object to become available. From 2ec401e89953cc7623490bba4ffa199339c240f2 Mon Sep 17 00:00:00 2001 From: Dean Herbert Date: Sun, 16 Jul 2023 02:56:20 +0900 Subject: [PATCH 2/2] Actually update proposed value with change Co-authored-by: Philipp <34287258+Phippe@users.noreply.github.com> --- DiscordRPC/IO/ManagedNamedPipeClient.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/DiscordRPC/IO/ManagedNamedPipeClient.cs b/DiscordRPC/IO/ManagedNamedPipeClient.cs index 328bfa8d..14fec151 100644 --- a/DiscordRPC/IO/ManagedNamedPipeClient.cs +++ b/DiscordRPC/IO/ManagedNamedPipeClient.cs @@ -143,7 +143,7 @@ private bool AttemptConnection(int pipe, bool isSandbox = false) // Intentionally use a timeout of 0 here to avoid spinlock overhead. // We are already performing local retry logic, so this is not required. - _stream.Connect(1000); + _stream.Connect(0); //Spin for a bit while we wait for it to finish connecting Logger.Trace("Waiting for connection...");