[desktop] Throttle ML status updates (#2450)

web/docs/dependencies.md

@@ -46,7 +46,7 @@ The root `package.json` also has a convenience dev dependency:
 -   [concurrently](https://github.com/open-cli-tools/concurrently) for spawning
     parallel tasks when we invoke various yarn scripts.
 
-## Crypto
+## Cryptography
 
 We use [libsodium](https://libsodium.gitbook.io/doc/) for encryption, key
 generation etc. Specifically, we use its WebAssembly and JS wrappers made using
@@ -72,29 +72,31 @@ bit more exhaustively when changing the crypto layer.
 
 ### Next.js
 
-[Next.js](https://nextjs.org) ("next") provides the meta framework for both the
-Photos and the Auth app, and also for some of the sidecar apps like accounts and
-cast.
+[Next.js](https://nextjs.org) (package:
+[next](https://github.com/vercel/next.js)) provides the meta framework for both
+the photos and the auth app, and also for some of the sidecar apps like accounts
+and cast.
 
-We use a limited subset of Next. The main thing we get out of it is a reasonable
-set of defaults for bundling our app into a static export which we can then
-deploy to our webserver. In addition, the Next.js page router is convenient.
-Apart from this, while we use a few tidbits from Next.js here and there, overall
-our apps are regular React SPAs, and are not particularly tied to Next.
+We use a limited subset of Next.js. The main thing we get out of it is a
+reasonable set of defaults for bundling our app into a static export which we
+can then deploy to our webserver. In addition, the Next.js page router is
+convenient. Overall our apps can be described as regular React SPAs, and are not
+particularly tied to Next.js.
 
 ### Vite
 
 For some of our newer code, we have started to use [Vite](https://vitejs.dev).
-It is more lower level than Next, but the bells and whistles it doesn't have are
-the bells and whistles (and the accompanying complexity) that we don't need in
-some cases.
+It is likely the future (both generally, and for our code) since Next.js is
+becoming less suitable for SSR and SPAs with their push towards RSC/SSG.
 
 ## UI
 
 ### React
 
-[React](https://react.dev) ("react") is our core framework. It also has a
-sibling "react-dom" package that renders JSX to the DOM.
+[React](https://react.dev) (package: [react](https://github.com/facebook/react))
+is our core framework. We also import its a sibling
+[react-dom](https://github.com/facebook/react) package that renders JSX to the
+DOM.
 
 ### MUI and Material Icons
 
@@ -109,18 +111,19 @@ We use [MUI](https://mui.com)'s
 ### Emotion
 
 MUI uses [Emotion](https://emotion.sh/) (a styled-component variant) as its
-preferred CSS-in-JS library, and we use the same in our code too to reduce
-moving parts.
+preferred CSS-in-JS library, so we use the same in our code too to reduce moving
+parts.
 
 Emotion itself comes in many parts, of which we need the following:
 
--   "@emotion/react" - React interface to Emotion. In particular, we set this as
-    the package that handles the transformation of JSX into JS (via the
-    `jsxImportSource` property in `tsconfig.json`).
+-   [@emotion/react](https://github.com/emotion-js/emotion) - React interface to
+    Emotion. In particular, we set this as the package that handles the
+    transformation of JSX into JS (via the `jsxImportSource` property in
+    `tsconfig.json`).
 
--   "@emotion/styled" - Provides the `styled` utility, a la styled-components.
-    We don't use it directly, instead we import it from `@mui/material`.
-    However, MUI docs
+-   [@emotion/styled](https://github.com/emotion-js/emotion) - Provides the
+    `styled` utility, a la styled-components. We don't use it directly, instead
+    we import it from `@mui/material`. However, MUI docs
     [mention](https://mui.com/material-ui/integrations/interoperability/#styled-components)
     that
 
@@ -146,14 +149,15 @@ infelicity for now.
 
 ### Translations
 
-For showing the app's UI in multiple languages, we use the i18next library,
-specifically its three components
+For showing the app's UI in multiple languages, we use the
+[i18next](https://www.i18next.com), specifically its three components
 
--   "i18next": The core `i18next` library.
--   "i18next-http-backend": Adds support for initializing `i18next` with JSON
-    file containing the translation in a particular language, fetched at
-    runtime.
--   "react-i18next": React specific support in `i18next`.
+-   [i18next](https://github.com/i18next/i18next): The core `i18next` library.
+-   [react-i18next](https://github.com/i18next/react-i18next): React specific
+    support in `i18next`.
+-   [i18next-http-backend](https://github.com/i18next/i18next-http-backend):
+    Adds support for initializing `i18next` with JSON file containing the
+    translation in a particular language, fetched at runtime.
 
 Note that inspite of the "next" in the name of the library, it has nothing to do
 with Next.js.
@@ -166,7 +170,7 @@ For more details, see [translations.md](translations.md).
     abstraction for dealing with form state, validation and submission states
     when using React.
 
-## Infrastructure
+## Utilities
 
 -   [comlink](https://github.com/GoogleChromeLabs/comlink) provides a minimal
     layer on top of web workers to make them more easier to use.
@@ -179,6 +183,11 @@ For more details, see [translations.md](translations.md).
 -   [zod](https://github.com/colinhacks/zod) is used for runtime typechecking
     (e.g. verifying that API responses match the expected TypeScript shape).
 
+-   [debounce](https://github.com/sindresorhus/debounce) and its
+    promise-supporting sibling
+    [pDebounce](https://github.com/sindresorhus/p-debounce) are used for
+    debouncing operations (See also: `[Note: Throttle and debounce]`).
+
 ## Media
 
 -   [jszip](https://github.com/Stuk/jszip) is used for reading zip files in
@@ -187,7 +196,7 @@ For more details, see [translations.md](translations.md).
 -   [file-type](https://github.com/sindresorhus/file-type) is used for MIME type
     detection. We are at an old version 16.5.4 because v17 onwards the package
     became ESM only - for our limited use case, the custom Webpack configuration
-    that entails is not worth the upgrade.
+    that it'd entail is not worth the upgrade.
 
 -   [heic-convert](https://github.com/catdad-experiments/heic-convert) is used
     for converting HEIC files (which browsers don't natively support) into JPEG.
@@ -228,5 +237,5 @@ For more details, see [translations.md](translations.md).
     the actual OTP from the user's TOTP/HOTP secret.
 
 -   However, otpauth doesn't support steam OTPs. For these, we need to compute
-    the SHA-1, and we use the same library, `jssha` that `otpauth` uses (since
-    it is already part of our bundle).
+    the SHA-1, and we use the same library, `jssha` that `otpauth` uses since it
+    is already part of our bundle (transitively).

web/packages/new/photos/services/ml/index.ts

@@ -9,6 +9,7 @@ import { blobCache } from "@/next/blob-cache";
 import { ensureElectron } from "@/next/electron";
 import log from "@/next/log";
 import { ComlinkWorker } from "@/next/worker/comlink-worker";
+import { throttled } from "@/utils/promise";
 import { proxy } from "comlink";
 import { isBetaUser, isInternalUser } from "../feature-flags";
 import { getRemoteFlag, updateRemoteFlag } from "../remote-store";
@@ -421,7 +422,7 @@ const setInterimScheduledStatus = () => {
     setMLStatusSnapshot({ phase: "scheduled", nSyncedFiles, nTotalFiles });
 };
 
-const workerDidProcessFile = triggerStatusUpdate;
+const workerDidProcessFile = throttled(updateMLStatusSnapshot, 2000);
 
 /**
  * Return the IDs of all the faces in the given {@link enteFile} that are not

web/packages/utils/promise.ts

@@ -7,6 +7,82 @@
 export const wait = (ms: number) =>
     new Promise((resolve) => setTimeout(resolve, ms));
 
+/**
+ * Throttle invocations of an underlying function, coalescing pending calls.
+ *
+ * Take a function that returns a promise, and return a new function that can be
+ * called an any number of times while still ensuring that the underlying
+ * function is only called a maximum of once per the specified period.
+ *
+ * The underlying function is immediately called if there were no calls to the
+ * throttled function in the last period.
+ *
+ * Otherwise we start waiting. Multiple calls to the throttled function while
+ * we're waiting (either for the original promise to resolve, or after that, for
+ * the specified cooldown period to elapse) will all be coalesced into a single
+ * call to the underlying function when we're done waiting.
+ *
+ * ---
+ *
+ * [Note: Throttle and debounce]
+ *
+ * There are many throttle/debounce libraries, and ideally I'd have liked to
+ * just use one of them instead of reinventing such a basic and finicky wheel.
+ * Then why write a bespoke one?
+ *
+ * -   "debounce" means that the underlying function will only be called when a
+ *     particular wait time has elapsed since the last call to the _debounced_
+ *     function.
+ *
+ * -   This behaviour, while useful sometimes, is not what we want always. If
+ *     the debounced function is continuously being called, then the underlying
+ *     function might never get called (since the wait time does not elapse).
+ *
+ * -   To avoid this starvation, some debounce implementations like lodash
+ *     provide a "maxWait" option, which tells the debounced function to always
+ *     call the underlying function if maxWait has elapsed.
+ *
+ * -   The debounced functions can trigger the underlying in two ways: leading
+ *     (aka immediate) and trailing which control if the underlying should be
+ *     called at the leading or the trailing edge of the time period.
+ *
+ * -   "throttle" can be conceptually thought of as just maxWait + leading. In
+ *     fact, this is how lodash actually implements it too. So we could've used
+ *     lodash, except that is a big dependency to pull for a small function.
+ *
+ * -   Alternatively, pThrottle is a micro-library that provide such a
+ *     "throttle" primitive. However, its implementation enqueues all incoming
+ *     requests to the throttled function: it still calls the underlying once
+ *     per period, but eventually underlying will get called once for each call
+ *     to the throttled function.
+ *
+ * -   There are circumstances where that would be the appropriate behaviour,
+ *     but that's not what we want. We wish to trigger an async action,
+ *     coalescing multiple triggers into a single one, one per period.
+ *
+ * -   Perhaps there are other focused and standard library that'd have what we
+ *     want, but instead of spending more time searching I just wrote it from
+ *     scratch for now. Indeed, I've spent more time writing about the function
+ *     than the function itself.
+ */
+export const throttled = (underlying: () => Promise<void>, period: number) => {
+    let pending = 0;
+
+    const f = () => {
+        pending += 1;
+        if (pending > 1) return;
+        void underlying()
+            .then(() => wait(period))
+            .then(() => {
+                const retrigger = pending > 1;
+                pending = 0;
+                if (retrigger) f();
+            });
+    };
+
+    return f;
+};
+
 /**
  * Await the given {@link promise} for {@link timeoutMS} milliseconds. If it
  * does not resolve within {@link timeoutMS}, then reject with a timeout error.