From 5da4028ebfba3e60b7f92bc2efee47a7c4a17f64 Mon Sep 17 00:00:00 2001 From: Manav Rathi Date: Sat, 29 Jun 2024 08:31:28 +0530 Subject: [PATCH] [desktop] Speed up reconciliation by doing an upfront directory listing --- desktop/src/main/ipc.ts | 10 ++++---- desktop/src/main/services/fs.ts | 15 ++++++++++++ desktop/src/main/services/watch.ts | 16 ------------- desktop/src/preload.ts | 6 ++--- .../photos/src/components/WatchFolder.tsx | 2 +- web/apps/photos/src/services/export/index.ts | 15 ++++++++++-- web/apps/photos/src/services/watch.ts | 2 +- web/packages/next/types/ipc.ts | 24 +++++++++---------- 8 files changed, 50 insertions(+), 40 deletions(-) diff --git a/desktop/src/main/ipc.ts b/desktop/src/main/ipc.ts index 55f5f8530e..d6672fa3ab 100644 --- a/desktop/src/main/ipc.ts +++ b/desktop/src/main/ipc.ts @@ -31,6 +31,7 @@ import { import { ffmpegExec } from "./services/ffmpeg"; import { fsExists, + fsFindFiles, fsIsDir, fsMkdirIfNeeded, fsReadTextFile, @@ -63,7 +64,6 @@ import { } from "./services/upload"; import { watchAdd, - watchFindFiles, watchGet, watchRemove, watchUpdateIgnoredFiles, @@ -154,6 +154,10 @@ export const attachIPCHandlers = () => { ipcMain.handle("fsIsDir", (_, dirPath: string) => fsIsDir(dirPath)); + ipcMain.handle("fsFindFiles", (_, folderPath: string) => + fsFindFiles(folderPath), + ); + // - Conversion ipcMain.handle("convertToJPEG", (_, imageData: Uint8Array) => @@ -262,10 +266,6 @@ export const attachFSWatchIPCHandlers = (watcher: FSWatcher) => { (_, ignoredFiles: FolderWatch["ignoredFiles"], folderPath: string) => watchUpdateIgnoredFiles(ignoredFiles, folderPath), ); - - ipcMain.handle("watchFindFiles", (_, folderPath: string) => - watchFindFiles(folderPath), - ); }; /** diff --git a/desktop/src/main/services/fs.ts b/desktop/src/main/services/fs.ts index 4570a4a33a..a909ebf493 100644 --- a/desktop/src/main/services/fs.ts +++ b/desktop/src/main/services/fs.ts @@ -4,6 +4,7 @@ import { existsSync } from "node:fs"; import fs from "node:fs/promises"; +import path from "node:path"; export const fsExists = (path: string) => existsSync(path); @@ -28,3 +29,17 @@ export const fsIsDir = async (dirPath: string) => { const stat = await fs.stat(dirPath); return stat.isDirectory(); }; + +export const fsFindFiles = async (dirPath: string) => { + const items = await fs.readdir(dirPath, { withFileTypes: true }); + let paths: string[] = []; + for (const item of items) { + const itemPath = path.posix.join(dirPath, item.name); + if (item.isFile()) { + paths.push(itemPath); + } else if (item.isDirectory()) { + paths = [...paths, ...(await fsFindFiles(itemPath))]; + } + } + return paths; +}; diff --git a/desktop/src/main/services/watch.ts b/desktop/src/main/services/watch.ts index a65fade8f5..6f198c513b 100644 --- a/desktop/src/main/services/watch.ts +++ b/desktop/src/main/services/watch.ts @@ -1,7 +1,5 @@ import chokidar, { type FSWatcher } from "chokidar"; import { BrowserWindow } from "electron/main"; -import fs from "node:fs/promises"; -import path from "node:path"; import { FolderWatch, type CollectionMapping } from "../../types/ipc"; import log from "../log"; import { watchStore } from "../stores/watch"; @@ -143,20 +141,6 @@ export const watchUpdateIgnoredFiles = ( ); }; -export const watchFindFiles = async (dirPath: string) => { - const items = await fs.readdir(dirPath, { withFileTypes: true }); - let paths: string[] = []; - for (const item of items) { - const itemPath = path.posix.join(dirPath, item.name); - if (item.isFile()) { - paths.push(itemPath); - } else if (item.isDirectory()) { - paths = [...paths, ...(await watchFindFiles(itemPath))]; - } - } - return paths; -}; - /** * Stop watching all existing folder watches and remove any callbacks. * diff --git a/desktop/src/preload.ts b/desktop/src/preload.ts index 50fb8b15c7..29bf9c0946 100644 --- a/desktop/src/preload.ts +++ b/desktop/src/preload.ts @@ -216,8 +216,8 @@ const watchOnRemoveDir = (f: (path: string, watch: FolderWatch) => void) => { ); }; -const watchFindFiles = (folderPath: string) => - ipcRenderer.invoke("watchFindFiles", folderPath); +const fsFindFiles = (folderPath: string) => + ipcRenderer.invoke("fsFindFiles", folderPath); const watchRemoveListeners = () => { ipcRenderer.removeAllListeners("watchAddFile"); @@ -340,6 +340,7 @@ contextBridge.exposeInMainWorld("electron", { readTextFile: fsReadTextFile, writeFile: fsWriteFile, isDir: fsIsDir, + findFiles: fsFindFiles, }, // - Conversion @@ -366,7 +367,6 @@ contextBridge.exposeInMainWorld("electron", { onAddFile: watchOnAddFile, onRemoveFile: watchOnRemoveFile, onRemoveDir: watchOnRemoveDir, - findFiles: watchFindFiles, }, // - Upload diff --git a/web/apps/photos/src/components/WatchFolder.tsx b/web/apps/photos/src/components/WatchFolder.tsx index e99fbb0073..b5bd80e2c8 100644 --- a/web/apps/photos/src/components/WatchFolder.tsx +++ b/web/apps/photos/src/components/WatchFolder.tsx @@ -82,7 +82,7 @@ export const WatchFolder: React.FC = ({ open, onClose }) => { }; const selectCollectionMappingAndAddWatch = async (path: string) => { - const filePaths = await ensureElectron().watch.findFiles(path); + const filePaths = await ensureElectron().fs.findFiles(path); if (areAllInSameDirectory(filePaths)) { addWatch(path, "root"); } else { diff --git a/web/apps/photos/src/services/export/index.ts b/web/apps/photos/src/services/export/index.ts index 754c3dc980..f63760464a 100644 --- a/web/apps/photos/src/services/export/index.ts +++ b/web/apps/photos/src/services/export/index.ts @@ -1283,6 +1283,17 @@ const readOnDiskFileExportRecordIDs = async ( const result = new Set(); if (!(await fs.exists(exportDir))) return result; + // Both the paths involved are guaranteed to use POSIX separators and thus + // can directly be compared. + // + // - `exportDir` traces its origin to `electron.selectDirectory()`, which + // returns POSIX paths. Down below we use it as the base directory when + // construction paths for the items to export. + // + // - `findFiles` is also guaranteed to return POSIX paths. + // + const ls = new Set(await ensureElectron().fs.findFiles(exportDir)); + const fileExportNames = exportRecord.fileExportNames ?? {}; for (const file of files) { @@ -1309,11 +1320,11 @@ const readOnDiskFileExportRecordIDs = async ( } const filePath = `${collectionExportPath}/${fileName}`; - if (await fs.exists(filePath)) { + if (ls.has(filePath)) { // Also check that the sibling part exists (if any). if (fileName2) { const filePath2 = `${collectionExportPath}/${fileName2}`; - if (await fs.exists(filePath2)) result.add(recordID); + if (ls.has(filePath2)) result.add(recordID); } else { result.add(recordID); } diff --git a/web/apps/photos/src/services/watch.ts b/web/apps/photos/src/services/watch.ts index e9f7f33005..597bbe29cc 100644 --- a/web/apps/photos/src/services/watch.ts +++ b/web/apps/photos/src/services/watch.ts @@ -561,7 +561,7 @@ const deduceEvents = async (watches: FolderWatch[]): Promise => { for (const watch of watches) { const folderPath = watch.folderPath; - const filePaths = await electron.watch.findFiles(folderPath); + const filePaths = await electron.fs.findFiles(folderPath); // Files that are on disk but not yet synced. for (const filePath of pathsToUpload(filePaths, watch)) diff --git a/web/packages/next/types/ipc.ts b/web/packages/next/types/ipc.ts index 21246ea660..4f68d50d73 100644 --- a/web/packages/next/types/ipc.ts +++ b/web/packages/next/types/ipc.ts @@ -234,6 +234,18 @@ export interface Electron { * directory. */ isDir: (dirPath: string) => Promise; + + /** + * Return the paths of all the files under the given folder. + * + * This function walks the directory tree starting at {@link folderPath} + * and returns a list of the absolute paths of all the files that exist + * therein. It will recursively traverse into nested directories, and + * return the absolute paths of the files there too. + * + * The returned paths are guaranteed to use POSIX separators ('/'). + */ + findFiles: (folderPath: string) => Promise; }; // - Conversion @@ -479,18 +491,6 @@ export interface Electron { * The path is guaranteed to use POSIX separators ('/'). */ onRemoveDir: (f: (path: string, watch: FolderWatch) => void) => void; - - /** - * Return the paths of all the files under the given folder. - * - * This function walks the directory tree starting at {@link folderPath} - * and returns a list of the absolute paths of all the files that exist - * therein. It will recursively traverse into nested directories, and - * return the absolute paths of the files there too. - * - * The returned paths are guaranteed to use POSIX separators ('/'). - */ - findFiles: (folderPath: string) => Promise; }; // - Upload