[desktop] Add logging to help trace memory pressure issues
The previous fix did not help the user https://discord.com/channels/948937918347608085/1253299055472410645 But I've also been unable to reproduce this on Linux. Modifying this code to just log (so that we can better understand before adding a workaround).
This commit is contained in:
@@ -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<number> => {
|
||||
/**
|
||||
* [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`,
|
||||
);
|
||||
}
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user