From 398ce9d44543e6cd06a02a58284b717585f110f3 Mon Sep 17 00:00:00 2001
From: Manav Rathi <manav@mrmr.io>
Date: Thu, 20 Jun 2024 21:04:18 +0530
Subject: [PATCH] [desktop] Add a memory usage high water mark during uploads

---
 .../src/services/upload/uploadManager.ts      | 47 +++++++++++++++++++
 1 file changed, 47 insertions(+)

diff --git a/web/apps/photos/src/services/upload/uploadManager.ts b/web/apps/photos/src/services/upload/uploadManager.ts
index eadb4ffc85..150b85ba69 100644
--- a/web/apps/photos/src/services/upload/uploadManager.ts
+++ b/web/apps/photos/src/services/upload/uploadManager.ts
@@ -520,6 +520,11 @@ class UploadManager {
         while (this.itemsToBeUploaded.length > 0) {
             this.abortIfCancelled();
 
+            if (shouldWaitForMemoryPressureToEase()) {
+                await wait(2000);
+                continue;
+            }
+
             const clusteredItem = this.itemsToBeUploaded.pop();
             const { localID, collectionID } = clusteredItem;
             const collection = this.collections.get(collectionID);
@@ -999,3 +1004,45 @@ const uploadItemSize = async (uploadItem: UploadItem): Promise<number> => {
         return ensureElectron().pathOrZipItemSize(uploadItem);
     return uploadItem.file.size;
 };
+
+/**
+ * [Note: Memory pressure when uploading video files]
+ *
+ * Some users have reported that their app runs out of memory when the app tries
+ * to upload multiple large videos simultaneously. For example, 4 parallel
+ * uploads of 4 700 MB videos.
+ *
+ * I am unable to reproduce this: tested on macOS, with videos up to 3.8 G
+ * uploaded in parallel. The memory usage remains constant (as expected,
+ * hovering around 2 G), since we don't pull the entire videos in memory and
+ * instead do a streaming disk read + encryption + upload.
+ *
+ * The JavaScript heap for the renderer process (when we're running in the
+ * context of our desktop app) is limited to 4 GB. See
+ * https://www.electronjs.org/blog/v8-memory-cage.
+ *
+ * Perhaps there is some distinct memory usage pattern on some systems that
+ * causes this limit to be reached.
+ *
+ * So as a safety check, this function returns true whenever we exceed some
+ * memory usage high water mark. If so, then the uploader should wait for the
+ * memory usage to come down before initiating a new upload.
+ */
+const shouldWaitForMemoryPressureToEase = () => {
+    if (!globalThis.electron) return false;
+    // performance.memory is deprecated in general as a Web standard, and is
+    // also not available in the DOM types provided by TypeScript. However, it
+    // is the method recommended by the Electron team (see the link about the V8
+    // memory cage). The embedded Chromium supports it fine though, we just need
+    // to goad TypeScript to accept the type.
+    const heapSizeInBytes = (performance as any).memory.usedJSHeapSize;
+    const heapSizeInGB = heapSizeInBytes / (1024 * 1024 * 1024);
+    // 4 GB is the hard limit. Let us keep a lot of margin since uploads get
+    // triggered in parallel so if we're unlucky they all might get trigger when
+    // the memory usage is relatively low.
+    if (heapSizeInGB < 2.5) return false;
+    log.info(
+        `Memory usage (${heapSizeInGB} GB) exceeds the high water mark, pausing new uploads`,
+    );
+    return true;
+};