diff --git a/web/apps/photos/src/services/upload/uploadManager.ts b/web/apps/photos/src/services/upload/uploadManager.ts index 0560fe8010..869bbbad27 100644 --- a/web/apps/photos/src/services/upload/uploadManager.ts +++ b/web/apps/photos/src/services/upload/uploadManager.ts @@ -402,6 +402,8 @@ class UploadManager { this.uploadInProgress = true; this.uploaderName = uploaderName; + const logInterval = setInterval(logAboutMemoryPressureIfNeeded, 1000); + try { await this.updateExistingFilesAndCollections(collections); @@ -449,6 +451,7 @@ class UploadManager { this.cryptoWorkers[i]?.terminate(); } this.uploadInProgress = false; + clearInterval(logInterval); } return this.uiService.hasFilesInResultList(); @@ -519,11 +522,7 @@ class UploadManager { while (this.itemsToBeUploaded.length > 0) { this.abortIfCancelled(); - - if (shouldWaitForMemoryPressureToEase()) { - await wait(2000); - continue; - } + logAboutMemoryPressureIfNeeded(); const clusteredItem = this.itemsToBeUploaded.pop(); const { localID, collectionID } = clusteredItem; @@ -1008,41 +1007,36 @@ const uploadItemSize = async (uploadItem: UploadItem): Promise => { /** * [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. + * A user (Fedora 39 VM on Qubes OS with 32 GB RAM, both AppImage and RPM) has + * 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. + * I am unable to reproduce this: tested on macOS and Linux, with videos up to + * 3.8 G x 1 + 3 x 700 M 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. + * For now, add logs if our usage increases some high water mark. This is solely + * so we can better understand the issue if it arises again (and can deal with + * it in an informed manner). */ -const shouldWaitForMemoryPressureToEase = () => { - if (!globalThis.electron) return false; +const logAboutMemoryPressureIfNeeded = () => { + if (!globalThis.electron) return; // 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; + const heapSize = (performance as any).memory.totalJSHeapSize; + const heapLimit = (performance as any).memory.jsHeapSizeLimit; + if (heapSize / heapLimit > 0.7) { + log.info( + `Memory usage (${heapSize} bytes of ${heapLimit} bytes) exceeds the high water mark`, + ); + } };