Include more details in the busy status tracker (#1338)

* rebuildPathFilterer now retains project-specific patterns on rebuild * Add test to verify removed project includeLists get unregistered * Include more information in the busy tracker * Fix lint issues
rokucommunity · Oct 28, 2024 · 2aeae9b · 2aeae9b
1 parent 9ec6f72
commit 2aeae9b
Show file tree

Hide file tree

Showing 7 changed files with 125 additions and 60 deletions.
diff --git a/src/BusyStatusTracker.spec.ts b/src/BusyStatusTracker.spec.ts
@@ -190,5 +190,35 @@ describe('BusyStatusTracker', () => {
                 await onChange
             ).to.eql(BusyStatus.idle);
         });
+
+        it('emits an active-runs-change event when any run changes', async () => {
+            let count = 0;
+            tracker.on('active-runs-change', () => {
+                count++;
+            });
+            tracker.run(() => { }, 'run1');
+            tracker.run(() => { }, 'run2');
+            await tracker.run(() => Promise.resolve(true), 'run3');
+
+            tracker.beginScopedRun(this, 'run4');
+            tracker.beginScopedRun(this, 'run4');
+            await tracker.endScopedRun(this, 'run4');
+            await tracker.endScopedRun(this, 'run4');
+
+            //we should have 10 total events (5 starts, 5 ends)
+            expect(count).to.eql(10);
+        });
+
+        it('removes the entry for the scope when the last run is cleared', async () => {
+            expect(tracker['activeRuns']).to.be.empty;
+
+            tracker.beginScopedRun(scope1, 'run1');
+
+            expect(tracker['activeRuns'].find(x => x.scope === scope1 && x.label === 'run1')).to.exist;
+
+            await tracker.endScopedRun(scope1, 'run1');
+
+            expect(tracker['activeRuns']).to.be.empty;
+        });
     });
 });
diff --git a/src/BusyStatusTracker.ts b/src/BusyStatusTracker.ts
@@ -1,49 +1,41 @@
 import { EventEmitter } from 'eventemitter3';
 import { Deferred } from './deferred';
 
+interface RunInfo<T> {
+    label: string;
+    startTime: Date;
+    scope?: T;
+    deferred?: Deferred;
+}
+
 /**
  * Tracks the busy/idle status of various sync or async tasks
  * Reports the overall status to the client
  */
-export class BusyStatusTracker {
+export class BusyStatusTracker<T = any> {
     /**
      * @readonly
      */
-    public activeRuns = new Set<{
-        label?: string;
-        startTime?: Date;
-    }>();
-
-    /**
-     * Collection to track scoped runs. All runs represented here also have a corresponding entry in `activeRuns`
-     */
-    private scopedRuns = new Map<any, Array<{
-        label: string;
-        deferred: Deferred;
-    }>>();
+    public activeRuns: Array<RunInfo<T>> = [];
 
     /**
      * Begin a busy task. It's expected you will call `endScopeRun` when the task is complete.
      * @param scope an object used for reference as to what is doing the work. Can be used to bulk-cancel all runs for a given scope.
      * @param label  label for the run. This is required for the `endScopedRun` method to know what to end.
      */
-    public beginScopedRun(scope: any, label: string) {
-        let runsForScope = this.scopedRuns.get(scope);
-        if (!runsForScope) {
-            runsForScope = [];
-            this.scopedRuns.set(scope, runsForScope);
-        }
-
+    public beginScopedRun(scope: T, label: string) {
         const deferred = new Deferred();
 
-        void this.run(() => {
+        const runInfo = {
+            label: label,
+            startTime: new Date(),
+            deferred: deferred,
+            scope: scope
+        };
+
+        void this._run(runInfo, () => {
             //don't mark the busy run as completed until the deferred is resolved
             return deferred.promise;
-        }, label);
-
-        runsForScope.push({
-            label: label,
-            deferred: deferred
         });
     }
 
@@ -52,17 +44,13 @@ export class BusyStatusTracker {
      * @param scope an object used for reference as to what is doing the work. Can be used to bulk-cancel all runs for a given scope.
      * @param label label for the run
      */
-    public endScopedRun(scope: any, label: string) {
-        const runsForScope = this.scopedRuns.get(scope);
-        if (!runsForScope) {
-            return;
-        }
-        const earliestRunIndex = runsForScope.findIndex(x => x.label === label);
+    public endScopedRun(scope: T, label: string) {
+        const earliestRunIndex = this.activeRuns.findIndex(x => x.scope === scope && x.label === label);
         if (earliestRunIndex === -1) {
             return;
         }
-        const earliestRun = runsForScope[earliestRunIndex];
-        runsForScope.splice(earliestRunIndex, 1);
+        const earliestRun = this.activeRuns[earliestRunIndex];
+        this.activeRuns.splice(earliestRunIndex, 1);
         earliestRun.deferred.resolve();
         return earliestRun.deferred.promise;
     }
@@ -71,37 +59,49 @@ export class BusyStatusTracker {
      * End all runs for a given scope. This is typically used when the scope is destroyed, and we want to make sure all runs are cleaned up.
      * @param scope an object used for reference as to what is doing the work.
      */
-    public endAllRunsForScope(scope: any) {
-        const runsForScope = this.scopedRuns.get(scope);
-        if (!runsForScope) {
-            return;
-        }
-        for (const run of runsForScope) {
-            run.deferred.resolve();
+    public endAllRunsForScope(scope: T) {
+        for (let i = this.activeRuns.length - 1; i >= 0; i--) {
+            const activeRun = this.activeRuns[i];
+            if (activeRun.scope === scope) {
+                //delete the active run
+                this.activeRuns.splice(i, 1);
+                //mark the run as resolved
+                activeRun.deferred.resolve();
+            }
         }
-        this.scopedRuns.delete(scope);
     }
 
     /**
      * Start a new piece of work
      */
-    public run<T, R = T | Promise<T>>(callback: (finalize?: FinalizeBuildStatusRun) => R, label?: string): R {
+    public run<A, R = A | Promise<A>>(callback: (finalize?: FinalizeBuildStatusRun) => R, label?: string): R {
         const run = {
             label: label,
             startTime: new Date()
         };
-        this.activeRuns.add(run);
+        return this._run<A, R>(run, callback);
+    }
+
+    private _run<A, R = A | Promise<A>>(runInfo: RunInfo<T>, callback: (finalize?: FinalizeBuildStatusRun) => R, label?: string): R {
+        this.activeRuns.push(runInfo);
 
-        if (this.activeRuns.size === 1) {
+        if (this.activeRuns.length === 1) {
             this.emit('change', BusyStatus.busy);
         }
+        this.emit('active-runs-change', { activeRuns: [...this.activeRuns] });
 
         let isFinalized = false;
         const finalizeRun = () => {
             if (isFinalized === false) {
                 isFinalized = true;
-                this.activeRuns.delete(run);
-                if (this.activeRuns.size <= 0) {
+
+                this.emit('active-runs-change', { activeRuns: [...this.activeRuns] });
+
+                let idx = this.activeRuns.indexOf(runInfo);
+                if (idx > -1) {
+                    this.activeRuns.splice(idx, 1);
+                }
+                if (this.activeRuns.length <= 0) {
                     this.emit('change', BusyStatus.idle);
                 }
             }
@@ -126,6 +126,7 @@ export class BusyStatusTracker {
 
     private emitter = new EventEmitter<string, BusyStatus>();
 
+    public once(eventName: 'active-runs-change'): Promise<{ activeRuns: Array<RunInfo<T>> }>;
     public once(eventName: 'change'): Promise<BusyStatus>;
     public once<T>(eventName: string): Promise<T> {
         return new Promise<T>((resolve) => {
@@ -136,15 +137,19 @@ export class BusyStatusTracker {
         });
     }
 
-    public on(eventName: 'change', handler: (status: BusyStatus) => void) {
+    public on(eventName: 'active-runs-change', handler: (event: { activeRuns: Array<RunInfo<T>> }) => void);
+    public on(eventName: 'change', handler: (status: BusyStatus) => void);
+    public on(eventName: string, handler: (event: any) => void) {
         this.emitter.on(eventName, handler);
         return () => {
             this.emitter.off(eventName, handler);
         };
     }
 
-    private emit(eventName: 'change', value: BusyStatus) {
-        this.emitter.emit(eventName, value);
+    private emit(eventName: 'active-runs-change', event: { activeRuns: Array<RunInfo<T>> });
+    private emit(eventName: 'change', status: BusyStatus);
+    private emit(eventName: string, event: any) {
+        this.emitter.emit(eventName, event);
     }
 
     public destroy() {
@@ -156,7 +161,7 @@ export class BusyStatusTracker {
      * @readonly
      */
     public get status() {
-        return this.activeRuns.size === 0 ? BusyStatus.idle : BusyStatus.busy;
+        return this.activeRuns.length === 0 ? BusyStatus.idle : BusyStatus.busy;
     }
 }
 

diff --git a/src/LanguageServer.ts b/src/LanguageServer.ts
@@ -125,7 +125,7 @@ export class LanguageServer {
             //resend all open document changes
             const documents = [...this.documents.all()];
             if (documents.length > 0) {
-                this.logger.log(`Project ${event.project?.projectNumber} loaded or changed. Resending all open document changes.`, documents.map(x => x.uri));
+                this.logger.log(`[${event.project?.projectIdentifier}] loaded or changed. Resending all open document changes.`, documents.map(x => x.uri));
                 for (const document of this.documents.all()) {
                     this.onTextDocumentDidChangeContent({
                         document: document
@@ -134,7 +134,8 @@ export class LanguageServer {
             }
         });
 
-        this.projectManager.busyStatusTracker.on('change', (event) => {
+        this.projectManager.busyStatusTracker.on('active-runs-change', (event) => {
+            console.log(event);
             this.sendBusyStatus();
         });
     }
@@ -583,7 +584,14 @@ export class LanguageServer {
             status: this.projectManager.busyStatusTracker.status,
             timestamp: Date.now(),
             index: this.busyStatusIndex,
-            activeRuns: [...this.projectManager.busyStatusTracker.activeRuns]
+            activeRuns: [
+                //extract only specific information from the active run so we know what's going on
+                ...this.projectManager.busyStatusTracker.activeRuns.map(x => ({
+                    scope: x.scope?.projectIdentifier,
+                    label: x.label,
+                    startTime: x.startTime.getTime()
+                }))
+            ]
         })?.catch(logAndIgnoreError);
     }
     private busyStatusIndex = -1;

diff --git a/src/lsp/LspProject.ts b/src/lsp/LspProject.ts
@@ -34,6 +34,11 @@ export interface LspProject {
      */
     projectNumber: number;
 
+    /**
+     * A unique name for this project used in logs to help keep track of everything
+     */
+    projectIdentifier: string;
+
     /**
      * The root directory of the project.
      * Only available after `.activate()` has completed

diff --git a/src/lsp/Project.ts b/src/lsp/Project.ts
@@ -22,12 +22,14 @@ export class Project implements LspProject {
     public constructor(
         options?: {
             logger?: Logger;
+            projectIdentifier?: string;
         }
     ) {
         this.logger = options?.logger ?? createLogger({
             //when running inside a worker thread, we don't want to use colors
             enableColor: false
         });
+        this.projectIdentifier = options?.projectIdentifier ?? '';
     }
 
     /**
@@ -47,7 +49,7 @@ export class Project implements LspProject {
             logger: this.logger
         });
 
-        this.builder.logger.prefix = `[prj${this.projectNumber}]`;
+        this.builder.logger.prefix = `[${this.projectIdentifier}]`;
         this.disposables.push(this.builder);
 
         let cwd: string;
@@ -452,6 +454,11 @@ export class Project implements LspProject {
      */
     public projectNumber: number;
 
+    /**
+     * A unique name for this project used in logs to help keep track of everything
+     */
+    public projectIdentifier: string;
+
     /**
      * The path to the workspace where this project resides. A workspace can have multiple projects (by adding a bsconfig.json to each folder).
      * Defaults to `.projectPath` if not set

diff --git a/src/lsp/ProjectManager.ts b/src/lsp/ProjectManager.ts
@@ -37,10 +37,10 @@ export class ProjectManager {
         });
 
         this.on('validate-begin', (event) => {
-            this.busyStatusTracker.beginScopedRun(event.project, `validate-project-${event.project.projectNumber}`);
+            this.busyStatusTracker.beginScopedRun(event.project, `validate-project`);
         });
         this.on('validate-end', (event) => {
-            void this.busyStatusTracker.endScopedRun(event.project, `validate-project-${event.project.projectNumber}`);
+            void this.busyStatusTracker.endScopedRun(event.project, `validate-project`);
         });
     }
 
@@ -62,7 +62,7 @@ export class ProjectManager {
     private documentManager: DocumentManager;
     public static documentManagerDelay = 150;
 
-    public busyStatusTracker = new BusyStatusTracker();
+    public busyStatusTracker = new BusyStatusTracker<LspProject>();
 
     /**
      * Apply all of the queued document changes. This should only be called as a result of the documentManager flushing changes, and never called manually
@@ -718,13 +718,16 @@ export class ProjectManager {
         }
 
         config.projectNumber = this.getProjectNumber(config);
+        const projectIdentifier = `prj${config.projectNumber}`;
 
         let project: LspProject = config.enableThreading
             ? new WorkerThreadProject({
-                logger: this.logger.createLogger()
+                logger: this.logger.createLogger(),
+                projectIdentifier: projectIdentifier
             })
             : new Project({
-                logger: this.logger.createLogger()
+                logger: this.logger.createLogger(),
+                projectIdentifier: projectIdentifier
             });
 
         this.logger.log(`Created project #${config.projectNumber} for: "${config.projectPath}" (${config.enableThreading ? 'worker thread' : 'main thread'})`);

diff --git a/src/lsp/worker/WorkerThreadProject.ts b/src/lsp/worker/WorkerThreadProject.ts
@@ -40,9 +40,11 @@ export class WorkerThreadProject implements LspProject {
     public constructor(
         options?: {
             logger?: Logger;
+            projectIdentifier?: string;
         }
     ) {
         this.logger = options?.logger ?? createLogger();
+        this.projectIdentifier = options?.projectIdentifier ?? '';
     }
 
     public async activate(options: ProjectConfig) {
@@ -113,6 +115,11 @@ export class WorkerThreadProject implements LspProject {
      */
     public projectNumber: number;
 
+    /**
+     * A unique name for this project used in logs to help keep track of everything
+     */
+    public projectIdentifier: string;
+
     /**
      * The path to the workspace where this project resides. A workspace can have multiple projects (by adding a bsconfig.json to each folder).
      * Defaults to `.projectPath` if not set