From 89243dc9d4d12ebe121341832df37caa17c1c4b6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Ferna=CC=81ndez=20Haro?= Date: Tue, 17 Aug 2021 19:19:31 +0200 Subject: [PATCH 1/2] Add debounce to the status observers to reduce unnecessary CPU loops --- src/core/server/status/plugins_status.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/core/server/status/plugins_status.ts b/src/core/server/status/plugins_status.ts index 6a8ef1081e165..60ac135fd8acc 100644 --- a/src/core/server/status/plugins_status.ts +++ b/src/core/server/status/plugins_status.ts @@ -76,6 +76,7 @@ export class PluginsStatusService { public getDerivedStatus$(plugin: PluginName): Observable { return this.update$.pipe( + debounceTime(25), switchMap(() => { // Only go up the dependency tree if any of this plugin's dependencies have a custom status // Helps eliminate memory overhead of creating thousands of Observables unnecessarily. @@ -103,6 +104,7 @@ export class PluginsStatusService { } return this.update$.pipe( + debounceTime(25), switchMap(() => { const pluginStatuses = plugins .map((depName) => { From 1aa2639a21070d56b65d7365ffd783dd1b940df9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Ferna=CC=81ndez=20Haro?= Date: Tue, 17 Aug 2021 20:30:41 +0200 Subject: [PATCH 2/2] Add tests + debounce only when necessary --- src/core/server/status/plugins_status.test.ts | 31 +++++++++++++++++-- src/core/server/status/plugins_status.ts | 3 +- 2 files changed, 30 insertions(+), 4 deletions(-) diff --git a/src/core/server/status/plugins_status.test.ts b/src/core/server/status/plugins_status.test.ts index a6579069acbc0..b7c0733de728e 100644 --- a/src/core/server/status/plugins_status.test.ts +++ b/src/core/server/status/plugins_status.test.ts @@ -247,7 +247,6 @@ describe('PluginStatusService', () => { subscription.unsubscribe(); expect(statusUpdates).toEqual([ - { a: { level: ServiceStatusLevels.available, summary: 'All dependencies are available' } }, { a: { level: ServiceStatusLevels.degraded, summary: 'a degraded' } }, { a: { level: ServiceStatusLevels.unavailable, summary: 'a unavailable' } }, { a: { level: ServiceStatusLevels.available, summary: 'a available' } }, @@ -274,7 +273,6 @@ describe('PluginStatusService', () => { subscription.unsubscribe(); expect(statusUpdates).toEqual([ - { a: { level: ServiceStatusLevels.available, summary: 'All dependencies are available' } }, { a: { level: ServiceStatusLevels.degraded, summary: 'a degraded' } }, { a: { level: ServiceStatusLevels.unavailable, summary: 'a unavailable' } }, { a: { level: ServiceStatusLevels.available, summary: 'a available' } }, @@ -357,6 +355,35 @@ describe('PluginStatusService', () => { }).toThrowError(); }); + it('debounces plugins custom status registration', async () => { + const service = new PluginsStatusService({ + core$: coreAllAvailable$, + pluginDependencies, + }); + const available: ServiceStatus = { + level: ServiceStatusLevels.available, + summary: 'a available', + }; + + const statusUpdates: Array> = []; + const subscription = service + .getDependenciesStatus$('b') + .subscribe((status) => statusUpdates.push(status)); + + const pluginA$ = new BehaviorSubject(available); + service.set('a', pluginA$); + + expect(statusUpdates).toStrictEqual([]); + + const delay = (ms: number) => new Promise((resolve) => setTimeout(resolve, ms)); + + // Waiting for the debounce timeout should cut a new update + await delay(25); + subscription.unsubscribe(); + + expect(statusUpdates).toStrictEqual([{ a: available }]); + }); + it('debounces events in quick succession', async () => { const service = new PluginsStatusService({ core$: coreAllAvailable$, diff --git a/src/core/server/status/plugins_status.ts b/src/core/server/status/plugins_status.ts index 60ac135fd8acc..7ef3ddb31d978 100644 --- a/src/core/server/status/plugins_status.ts +++ b/src/core/server/status/plugins_status.ts @@ -76,7 +76,7 @@ export class PluginsStatusService { public getDerivedStatus$(plugin: PluginName): Observable { return this.update$.pipe( - debounceTime(25), + debounceTime(25), // Avoid calling the plugin's custom status logic for every plugin that depends on it. switchMap(() => { // Only go up the dependency tree if any of this plugin's dependencies have a custom status // Helps eliminate memory overhead of creating thousands of Observables unnecessarily. @@ -104,7 +104,6 @@ export class PluginsStatusService { } return this.update$.pipe( - debounceTime(25), switchMap(() => { const pluginStatuses = plugins .map((depName) => {