From 63ecad674c7a7e7aa6c746eb0ecbcd2f66baf7cc Mon Sep 17 00:00:00 2001 From: Florent Benoit Date: Mon, 14 Aug 2023 15:12:27 +0200 Subject: [PATCH] feat(store): add debounce/throttle option allow to use debounce/throttle when updating stores if there are 15 events in a row to update a store, it can be updated only once after all events completed in like 1.5s related to https://github.com/containers/podman-desktop/issues/3376 Signed-off-by: Florent Benoit --- packages/renderer/src/stores/containers.ts | 2 +- .../renderer/src/stores/event-store.spec.ts | 132 ++++++++++++++++- packages/renderer/src/stores/event-store.ts | 133 +++++++++++++++--- packages/renderer/src/stores/images.ts | 2 +- packages/renderer/src/stores/pods.ts | 2 +- packages/renderer/src/stores/volumes.spec.ts | 3 + packages/renderer/src/stores/volumes.ts | 2 +- 7 files changed, 248 insertions(+), 28 deletions(-) diff --git a/packages/renderer/src/stores/containers.ts b/packages/renderer/src/stores/containers.ts index 44c618f4eb691..94451df55deb7 100644 --- a/packages/renderer/src/stores/containers.ts +++ b/packages/renderer/src/stores/containers.ts @@ -62,7 +62,7 @@ const containersEventStore = new EventStore( listContainers, ContainerIcon, ); -containersEventStore.setup(); +containersEventStore.setupWithDebounce(); export const searchPattern = writable(''); diff --git a/packages/renderer/src/stores/event-store.spec.ts b/packages/renderer/src/stores/event-store.spec.ts index 07b174c9038bb..d1ae721a6e7a7 100644 --- a/packages/renderer/src/stores/event-store.spec.ts +++ b/packages/renderer/src/stores/event-store.spec.ts @@ -17,7 +17,7 @@ ***********************************************************************/ import { beforeEach, expect, test, vi } from 'vitest'; -import { EventStore } from './event-store'; +import { EventStore, type EventStoreInfo } from './event-store'; import { get, writable, type Writable } from 'svelte/store'; // first, path window object @@ -47,6 +47,17 @@ interface MyCustomTypeInfo { name: string; } +class TestEventStore extends EventStore { + public async performUpdate( + needUpdate: boolean, + eventStoreInfo: EventStoreInfo, + eventName: string, + args?: unknown[], + ): Promise { + return super.performUpdate(needUpdate, eventStoreInfo, eventName, args); + } +} + test('should call fetch method using window event', async () => { const myStoreInfo: Writable = writable([]); const checkForUpdateMock = vi.fn(); @@ -57,7 +68,7 @@ test('should call fetch method using window event', async () => { // return true to trigger the update checkForUpdateMock.mockResolvedValue(true); - const eventStore = new EventStore('my-test', myStoreInfo, checkForUpdateMock, [windowEventName], [], updater); + const eventStore = new TestEventStore('my-test', myStoreInfo, checkForUpdateMock, [windowEventName], [], updater); // callbacks are empty expect(callbacks.size).toBe(0); @@ -106,7 +117,7 @@ test('should call fetch method using listener event', async () => { // return true to trigger the update checkForUpdateMock.mockResolvedValue(true); - const eventStore = new EventStore( + const eventStore = new TestEventStore( 'my-listener-test', myStoreInfo, checkForUpdateMock, @@ -155,3 +166,118 @@ test('should call fetch method using listener event', async () => { expect(eventStoreInfo.bufferEvents[0]).toHaveProperty('args', undefined); expect(eventStoreInfo.bufferEvents[0]).toHaveProperty('length', 1); }); + +test('Check debounce', async () => { + const myStoreInfo: Writable = writable([]); + const checkForUpdateMock = vi.fn(); + + const windowEventName = 'my-custom-event'; + const updater = vi.fn(); + + // return true to trigger the update + checkForUpdateMock.mockResolvedValue(true); + + const eventStore = new TestEventStore('my-test', myStoreInfo, checkForUpdateMock, [windowEventName], [], updater); + + // callbacks are empty + expect(callbacks.size).toBe(0); + + // now call the setup with a debounce value of 200ms and no throttle + const eventStoreInfo = eventStore.setupWithDebounce(200, 0); + + // spy performUpdate method + const performUpdateSpy = vi.spyOn(eventStore, 'performUpdate'); + + // check we have callbacks + expect(callbacks.size).toBe(1); + + // now we call the listener + const callback = callbacks.get(windowEventName); + expect(callback).toBeDefined(); + + const myCustomTypeInfo: MyCustomTypeInfo = { + name: 'my-custom-type', + }; + updater.mockResolvedValue([myCustomTypeInfo]); + + // now, perform 20 calls every 50ms + for (let i = 0; i < 20; i++) { + await callback(); + await new Promise(resolve => setTimeout(resolve, 50)); + } + + // wait debounce being called for 2 seconds + await new Promise(resolve => setTimeout(resolve, 2000)); + + // We did a lot of calls but with debounce, it should only be called once + expect(updater).toHaveBeenCalledOnce(); + + // check the store is updated + expect(get(myStoreInfo)).toStrictEqual([myCustomTypeInfo]); + + // check the store is updated + expect(get(myStoreInfo)).toStrictEqual([myCustomTypeInfo]); + + // check we have called performUpdate only once + expect(performUpdateSpy).toHaveBeenCalledOnce(); + + // check buffer events + expect(eventStoreInfo.bufferEvents.length).toBeGreaterThan(1); +}); + +// check that we're still calling the update method +// every throttle even if we have lot of calls and are postponing with debounce +test('Check debounce+delay', async () => { + const myStoreInfo: Writable = writable([]); + const checkForUpdateMock = vi.fn(); + + const windowEventName = 'my-custom-event'; + const updater = vi.fn(); + + // return true to trigger the update + checkForUpdateMock.mockResolvedValue(true); + + const eventStore = new EventStore('my-test', myStoreInfo, checkForUpdateMock, [windowEventName], [], updater); + + // callbacks are empty + expect(callbacks.size).toBe(0); + + // now call the setup with a debounce value of 200ms and a throttle of 1s + const eventStoreInfo = eventStore.setupWithDebounce(200, 1000); + + // check we have callbacks + expect(callbacks.size).toBe(1); + + // now we call the listener + const callback = callbacks.get(windowEventName); + expect(callback).toBeDefined(); + + const myCustomTypeInfo: MyCustomTypeInfo = { + name: 'my-custom-type', + }; + updater.mockResolvedValue([myCustomTypeInfo]); + + // now, perform 40 calls every 50ms + for (let i = 0; i < 20; i++) { + await callback(); + await new Promise(resolve => setTimeout(resolve, 50)); + } + + // wait debounce being called for 3 seconds + await new Promise(resolve => setTimeout(resolve, 3000)); + + // We did a lot of calls but with debounce and throttle it should be only like 2 calls + // get number of calls + const calls = updater.mock.calls.length; + expect(calls).toBeGreaterThan(1); + expect(calls).toBeLessThanOrEqual(4); + + // check the store is updated + expect(get(myStoreInfo)).toStrictEqual([myCustomTypeInfo]); + + // check the store is updated + expect(get(myStoreInfo)).toStrictEqual([myCustomTypeInfo]); + + // check buffer events + expect(eventStoreInfo.bufferEvents.length).toBeGreaterThan(1); +}); diff --git a/packages/renderer/src/stores/event-store.ts b/packages/renderer/src/stores/event-store.ts index daad23d40100b..826e3ec1ecd8d 100644 --- a/packages/renderer/src/stores/event-store.ts +++ b/packages/renderer/src/stores/event-store.ts @@ -25,26 +25,33 @@ import { addStore, updateStore } from './event-store-manager'; import humanizeDuration from 'humanize-duration'; import DesktopIcon from '../lib/images/DesktopIcon.svelte'; -export interface EventStoreInfo { +// 1.5 SECOND for DEBOUNCE and 5s for THROTTLE +const SECOND = 1000; +const DEFAULT_DEBOUNCE_TIMEOUT = 1.5 * SECOND; +const DEFAULT_THROTTLE_TIMEOUT = 5 * SECOND; + +interface EventStoreInfoEvent { name: string; - iconComponent?: ComponentType; + // args of the event + args: unknown[]; - // list last 100 events - bufferEvents: { - name: string; + date: number; + // if the event was skipped + skipped: boolean; + length?: number; - // args of the event - args: unknown[]; + // update time in ms + humanDuration?: number; +} + +export interface EventStoreInfo { + name: string; - date: number; - // if the event was skipped - skipped: boolean; - length?: number; + iconComponent?: ComponentType; - // update time in ms - humanDuration?: number; - }[]; + // list last 100 events + bufferEvents: EventStoreInfoEvent[]; // number of elements in the store size: number; @@ -59,6 +66,14 @@ export interface EventStoreInfo { // Helper to manage store updated from events export class EventStore { + // debounce delay in ms. If set to > 0 , timeout before updating store value + // if there are always new requests, it will never update the store + // as we postpone the update until there is no new request + private debounceTimeoutDelay = 0; + + // debounce always delay in ms. If set to > 0 , update after this delay even if some requests are pending. + private debounceThrottleTimeoutDelay = 0; + constructor( private name: string, private store: Writable, @@ -73,6 +88,15 @@ export class EventStore { } } + protected updateEvent(eventStoreInfo: EventStoreInfo, event: EventStoreInfoEvent) { + // update the info object + eventStoreInfo.bufferEvents.push(event); + if (eventStoreInfo.bufferEvents.length > 100) { + eventStoreInfo.bufferEvents.shift(); + } + updateStore(eventStoreInfo); + } + protected async performUpdate( needUpdate: boolean, eventStoreInfo: EventStoreInfo, @@ -100,8 +124,7 @@ export class EventStore { this.store.set(result); } } finally { - // update the info object - eventStoreInfo.bufferEvents.push({ + this.updateEvent(eventStoreInfo, { name: eventName, args: args, date: Date.now(), @@ -109,13 +132,18 @@ export class EventStore { length: numberOfResults, humanDuration: updateDuration, }); - if (eventStoreInfo.bufferEvents.length > 100) { - eventStoreInfo.bufferEvents.shift(); - } - updateStore(eventStoreInfo); } } + setupWithDebounce( + debounceTimeoutDelay = DEFAULT_DEBOUNCE_TIMEOUT, + debounceThrottleTimeoutDelay = DEFAULT_THROTTLE_TIMEOUT, + ): EventStoreInfo { + this.debounceTimeoutDelay = debounceTimeoutDelay; + this.debounceThrottleTimeoutDelay = debounceThrottleTimeoutDelay; + return this.setup(); + } + setup(): EventStoreInfo { const bufferEvents = []; @@ -136,10 +164,73 @@ export class EventStore { }; addStore(eventStoreInfo); + // for debounce + let timeout: NodeJS.Timeout | undefined; + + // for throttling every 5s if not already done + let timeoutThrottle: NodeJS.Timeout | undefined; + const update = async (eventName: string, args?: unknown[]) => { const needUpdate = await this.checkForUpdate(eventName, args); - await this.performUpdate(needUpdate, eventStoreInfo, eventName, args); + + // method that do the update + const doUpdate = async () => { + await this.performUpdate(needUpdate, eventStoreInfo, eventName, args); + }; + + // no debounce, just do it + if (this.debounceTimeoutDelay <= 0) { + await doUpdate(); + return; + } + + // debounce timeout. If there is a pending action, cancel it and wait longer + if (timeout) { + clearTimeout(timeout); + + this.updateEvent(eventStoreInfo, { + name: `debounce-${eventName}`, + args: args, + date: Date.now(), + skipped: true, + length: 0, + humanDuration: 0, + }); + + timeout = undefined; + } + timeout = setTimeout(() => { + // cancel the throttleTimeout if any + if (timeoutThrottle) { + clearTimeout(timeoutThrottle); + timeoutThrottle = undefined; + } + + doUpdate() + .catch((error: unknown) => { + console.error(`Failed to update ${this.name}`, error); + }) + .finally(() => { + timeout = undefined; + }); + }, this.debounceTimeoutDelay); + + // throttle timeout, ask after 5s to update anyway to have at least UI being refreshed every 5s if there is a lot of events + // because debounce will defer all the events until the end so it's not so nice from UI side. + if (!timeoutThrottle && this.debounceThrottleTimeoutDelay > 0) { + timeoutThrottle = setTimeout(() => { + doUpdate() + .catch((error: unknown) => { + console.error(`Failed to update ${this.name}`, error); + }) + .finally(() => { + clearTimeout(timeoutThrottle); + timeoutThrottle = undefined; + }); + }, this.debounceThrottleTimeoutDelay); + } }; + this.windowEvents.forEach(eventName => { window.events?.receive(eventName, async (args?: unknown[]) => { await update(eventName, args); diff --git a/packages/renderer/src/stores/images.ts b/packages/renderer/src/stores/images.ts index 36b524f4ea522..246da1ce78151 100644 --- a/packages/renderer/src/stores/images.ts +++ b/packages/renderer/src/stores/images.ts @@ -65,7 +65,7 @@ const imagesEventStore = new EventStore( listImages, ImageIcon, ); -imagesEventStore.setup(); +imagesEventStore.setupWithDebounce(); export const searchPattern = writable(''); diff --git a/packages/renderer/src/stores/pods.ts b/packages/renderer/src/stores/pods.ts index 5637a2fb080dc..ebee29f6e69d2 100644 --- a/packages/renderer/src/stores/pods.ts +++ b/packages/renderer/src/stores/pods.ts @@ -64,7 +64,7 @@ const eventStore = new EventStore( grabAllPods, PodIcon, ); -eventStore.setup(); +eventStore.setupWithDebounce(); export async function grabAllPods(): Promise { let result = await window.listPods(); diff --git a/packages/renderer/src/stores/volumes.spec.ts b/packages/renderer/src/stores/volumes.spec.ts index 52cbbadc59430..ba940929e7f12 100644 --- a/packages/renderer/src/stores/volumes.spec.ts +++ b/packages/renderer/src/stores/volumes.spec.ts @@ -85,6 +85,9 @@ test('volumes should be updated in case of a container is removed', async () => expect(containerRemovedCallback).toBeDefined(); await containerRemovedCallback(); + // wait debounce + await new Promise(resolve => setTimeout(resolve, 2000)); + // check if the volumes are updated const volumes2 = get(volumeListInfos); expect(volumes2.length).toBe(0); diff --git a/packages/renderer/src/stores/volumes.ts b/packages/renderer/src/stores/volumes.ts index 445fa6b03b29a..6f63120efbfc4 100644 --- a/packages/renderer/src/stores/volumes.ts +++ b/packages/renderer/src/stores/volumes.ts @@ -65,7 +65,7 @@ export const volumesEventStore = new EventStore( listVolumes, VolumeIcon, ); -const volumesEventStoreInfo = volumesEventStore.setup(); +const volumesEventStoreInfo = volumesEventStore.setupWithDebounce(); export const searchPattern = writable('');