Skip to content

Zstd patch application degrades app performance on macOS #639

@frenzibyte

Description

@frenzibyte

OS

macOS Sequoia 15.4.1

Programming Language

dotnet 8.0

VPK Version

vpk 0.0.1053

Library Version

Nuget v0.0.1053

What happened?

When downloading and installing a new update via UpdateManager.DownloadUpdatesAsync, if delta patching method was chosen and files were patched using Zstd patching method, a separate Process is spawned for each file, causing a lot of interruption for the app's main loop.

There seems to be a performance issue on macOS specifically with spawning a Process, I'm not quite familiar with the details but from what I understand, .NET uses a fork()/exec() pattern for starting a process, and macOS appears to not handle this pattern efficiently, causing interruptions to the app's main loop (i.e. frame drops).

One seemingly more efficient and quick solution to this issue is replacing Process.Start with posix_spawn/waitpid P/Invokes. From local testing, this eliminates all kind of interruptions while performing all operations in order. Came up with this diff as well since it was simple to integrate:

diff --git a/src/lib-csharp/Compression/DeltaUpdateExe.cs b/src/lib-csharp/Compression/DeltaUpdateExe.cs
index 1d198367..d0210241 100644
--- a/src/lib-csharp/Compression/DeltaUpdateExe.cs
+++ b/src/lib-csharp/Compression/DeltaUpdateExe.cs
@@ -1,5 +1,6 @@
 using System;
 using System.Diagnostics;
+using System.Threading;
 using Velopack.Logging;
 using Velopack.Util;
 
@@ -9,24 +10,53 @@ internal class DeltaUpdateExe : DeltaPackage
     {
         private readonly string _updateExePath;
 
-        public DeltaUpdateExe(IVelopackLogger logger, string baseTmpDir, string updateExePath) : base(logger, baseTmpDir)
+        public DeltaUpdateExe(IVelopackLogger logger, string baseTmpDir, string updateExePath)
+            : base(logger, baseTmpDir)
         {
             _updateExePath = updateExePath;
         }
 
         protected override void ApplyZstdPatch(string baseFile, string patchFile, string outputFile)
         {
-            var psi = new ProcessStartInfo(_updateExePath);
-            psi.AppendArgumentListSafe(new string[] { "patch", "--old", baseFile, "--patch", patchFile, "--output", outputFile }, out var _);
-            psi.CreateNoWindow = true;
-            var p = psi.StartRedirectOutputToILogger(Log, VelopackLogLevel.Debug);
-            if (!p.WaitForExit(30_000)) {
-                p.Kill();
-                throw new TimeoutException("zstd patch process timed out (30s).");
+            int spawnResult = PosixUtil.posix_spawn(
+                out int pid,
+                _updateExePath,
+                IntPtr.Zero,
+                IntPtr.Zero,
+                new[] { _updateExePath, "patch", "--old", baseFile, "--patch", patchFile, "--output", outputFile, null },
+                IntPtr.Zero);
+
+            // todo: redirect output to logger via posix_spawn_file_actions_t
+            // var p = psi.StartRedirectOutputToILogger(Log, VelopackLogLevel.Debug);
+
+            if (spawnResult != 0) {
+                throw new Exception($"zstd patch process failed to start: {spawnResult}.");
+            }
+
+            var time = DateTimeOffset.Now;
+            int status;
+
+            while (true) {
+                int waitResult = PosixUtil.waitpid(pid, out status, PosixUtil.WNOHANG);
+
+                if (waitResult == -1) {
+                    throw new Exception($"zstd patch process could not be awaited: {waitResult}");
+                }
+
+                if (waitResult > 0) {
+                    break;
+                }
+
+                if (DateTimeOffset.Now - time > TimeSpan.FromSeconds(30)) {
+                    PosixUtil.kill(pid, PosixUtil.SIGKILL);
+                    throw new TimeoutException("zstd patch process timed out (30s).");
+                }
+
+                Thread.Sleep(100);
             }
 
-            if (p.ExitCode != 0) {
-                throw new Exception($"zstd patch process failed with exit code {p.ExitCode}.");
+            if (status != 0) {
+                throw new Exception($"zstd patch process failed with exit code {status}.");
             }
         }
     }
diff --git a/src/lib-csharp/Util/PosixUtil.cs b/src/lib-csharp/Util/PosixUtil.cs
new file mode 100644
index 00000000..a077bca0
--- /dev/null
+++ b/src/lib-csharp/Util/PosixUtil.cs
@@ -0,0 +1,23 @@
+// Copyright (c) ppy Pty Ltd <contact@ppy.sh>. Licensed under the MIT Licence.
+// See the LICENCE file in the repository root for full licence text.
+
+using System;
+using System.Runtime.InteropServices;
+
+namespace Velopack.Util
+{
+    public static class PosixUtil
+    {
+        public const int WNOHANG = 1;
+        public const int SIGKILL = 9;
+
+        [DllImport("libc", SetLastError = true)]
+        public static extern int posix_spawn(out int pid, string file, IntPtr fileActions, IntPtr attrp, string?[] argv, IntPtr envp);
+
+        [DllImport("libc", SetLastError = true)]
+        public static extern int waitpid(int pid, out int status, int options);
+
+        [DllImport("libc", SetLastError = true)]
+        public static extern int kill(int pid, int sig);
+    }
+}
\ No newline at end of file

Relevant log output

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't working

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions