Skip to content

Commit ff67d5d

Browse files
omarArmomarArm
authored
Fix update component viewer lagging updates issue for single core (#757)
* updating instances only when needed * guarding against error messages that don't throw exceptions in the cdt-adapter * removing guard around update instances as it no longer needed
1 parent 2b0d9ff commit ff67d5d

3 files changed

Lines changed: 64 additions & 78 deletions

File tree

‎src/views/component-viewer/component-viewer-main.ts‎

100644100755
Lines changed: 20 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
*/
1616

1717
import * as vscode from 'vscode';
18-
import { GDBTargetDebugTracker, GDBTargetDebugSession, SessionStackItem } from '../../debug-session';
18+
import { GDBTargetDebugTracker, GDBTargetDebugSession } from '../../debug-session';
1919
import { ComponentViewerInstance } from './component-viewer-instance';
2020
import { URI } from 'vscode-uri';
2121
import { ComponentViewerTreeDataProvider } from './component-viewer-tree-view';
@@ -27,7 +27,6 @@ export class ComponentViewer {
2727
private _componentViewerTreeDataProvider: ComponentViewerTreeDataProvider | undefined;
2828
private _context: vscode.ExtensionContext;
2929
private _instanceUpdateCounter: number = 0;
30-
private _updateSemaphoreFlag: boolean = false;
3130
private _loadingCounter: number = 0;
3231

3332
public constructor(context: vscode.ExtensionContext) {
@@ -68,15 +67,14 @@ export class ComponentViewer {
6867
this._instances = cbuildRunInstances;
6968
}
7069

71-
private async loadCbuildRunInstances(session: GDBTargetDebugSession, tracker: GDBTargetDebugTracker) : Promise<void> {
70+
private async loadCbuildRunInstances(session: GDBTargetDebugSession, tracker: GDBTargetDebugTracker) : Promise<void | undefined> {
7271
this._loadingCounter++;
7372
console.log(`Loading SCVD files from cbuild-run, attempt #${this._loadingCounter}`);
7473
// Try to read SCVD files from cbuild-run file first
7574
await this.readScvdFiles(tracker, session);
7675
// Are there any SCVD files found in cbuild-run?
77-
if (this._instances.length > 0) {
78-
await this.updateInstances();
79-
return;
76+
if (this._instances.length === 0) {
77+
return undefined;
8078
}
8179
}
8280

@@ -87,26 +85,26 @@ export class ComponentViewer {
8785
const onConnectedDisposable = tracker.onConnected(async (session) => {
8886
await this.handleOnConnected(session, tracker);
8987
});
90-
const onDidChangeActiveStackItemDisposable = tracker.onDidChangeActiveStackItem(async (stackTraceItem) => {
91-
await this.handleOnDidChangeActiveStackItem(stackTraceItem);
92-
});
9388
const onDidChangeActiveDebugSessionDisposable = tracker.onDidChangeActiveDebugSession(async (session) => {
9489
await this.handleOnDidChangeActiveDebugSession(session);
9590
});
96-
const onStopped = tracker.onStopped(async (session) => {
97-
await this.handleOnStopped(session.session);
91+
const onStackTraceDisposable = tracker.onStackTrace(async (session) => {
92+
await this.handleOnStackTrace(session.session);
93+
});
94+
const onWillStartSessionDisposable = tracker.onWillStartSession(async (session) => {
95+
await this.handleOnWillStartSession(session);
9896
});
9997
// clear all disposables on extension deactivation
10098
context.subscriptions.push(
10199
onWillStopSessionDisposable,
102100
onConnectedDisposable,
103-
onDidChangeActiveStackItemDisposable,
104101
onDidChangeActiveDebugSessionDisposable,
105-
onStopped
102+
onStackTraceDisposable,
103+
onWillStartSessionDisposable
106104
);
107105
}
108106

109-
private async handleOnStopped(session: GDBTargetDebugSession): Promise<void> {
107+
private async handleOnStackTrace(session: GDBTargetDebugSession): Promise<void> {
110108
// Clear active session if it is NOT the one being stopped
111109
if (this._activeSession?.session.id !== session.session.id) {
112110
this._activeSession = undefined;
@@ -120,8 +118,11 @@ export class ComponentViewer {
120118
if (this._activeSession?.session.id === session.session.id) {
121119
this._activeSession = undefined;
122120
}
123-
// Update component viewer instance(s)
124-
await this.updateInstances();
121+
}
122+
123+
private async handleOnWillStartSession(session: GDBTargetDebugSession): Promise<void> {
124+
// Subscribe to refresh events of the started session
125+
session.refreshTimer.onRefresh(async (refreshSession) => await this.handleRefreshTimerEvent(refreshSession));
125126
}
126127

127128
private async handleOnConnected(session: GDBTargetDebugSession, tracker: GDBTargetDebugTracker): Promise<void> {
@@ -134,18 +135,11 @@ export class ComponentViewer {
134135
this._activeSession = session;
135136
// Load SCVD files from cbuild-run
136137
await this.loadCbuildRunInstances(session, tracker);
137-
// Subscribe to refresh events of the started session
138-
session.refreshTimer.onRefresh(async (refreshSession) => {
139-
if (this._activeSession?.session.id === refreshSession.session.id) {
140-
// Update component viewer instance(s)
141-
await this.updateInstances();
142-
}
143-
});
144138
}
145139

146-
private async handleOnDidChangeActiveStackItem(stackTraceItem: SessionStackItem): Promise<void> {
147-
if ((stackTraceItem.item as vscode.DebugStackFrame).frameId !== undefined) {
148-
// Update instance(s) with new stack frame info
140+
private async handleRefreshTimerEvent(session: GDBTargetDebugSession): Promise<void> {
141+
if (this._activeSession?.session.id === session.session.id) {
142+
// Update component viewer instance(s)
149143
await this.updateInstances();
150144
}
151145
}
@@ -158,18 +152,12 @@ export class ComponentViewer {
158152
}
159153

160154
private async updateInstances(): Promise<void> {
161-
if (this._updateSemaphoreFlag) {
162-
return;
163-
}
164-
this._updateSemaphoreFlag = true;
165155
this._instanceUpdateCounter = 0;
166156
if (!this._activeSession) {
167157
this._componentViewerTreeDataProvider?.deleteModels();
168-
this._updateSemaphoreFlag = false;
169158
return;
170159
}
171160
if (this._instances.length === 0) {
172-
this._updateSemaphoreFlag = false;
173161
return;
174162
}
175163
this._componentViewerTreeDataProvider?.resetModelCache();
@@ -180,6 +168,5 @@ export class ComponentViewer {
180168
this._componentViewerTreeDataProvider?.addGuiOut(instance.getGuiTree());
181169
}
182170
this._componentViewerTreeDataProvider?.showModelData();
183-
this._updateSemaphoreFlag = false;
184171
}
185172
}

‎src/views/component-viewer/component-viewer-target-access.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,10 @@ export class ComponentViewerTargetAccess {
4242
context: context
4343
};
4444
const response = await this._activeSession?.session.customRequest('evaluate', args) as DebugProtocol.EvaluateResponse['body'];
45+
// cdt-adapter may return error messages without throwing exceptions
46+
if (response.result.startsWith('Error')) {
47+
return undefined;
48+
}
4549
return response.result.split(' ')[0]; // Return only the address part
4650
} catch (error: unknown) {
4751
const errorMessage = (error as Error)?.message;

src/views/component-viewer/test/unit/component-viewer-controller.test.ts renamed to src/views/component-viewer/test/unit/component-viewer-main.test.ts

Lines changed: 40 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -55,32 +55,32 @@ import type { ExtensionContext } from 'vscode';
5555
import type { GDBTargetDebugTracker } from '../../../../debug-session';
5656
import { ComponentViewer } from '../../component-viewer-main';
5757

58+
// Local test mocks
59+
60+
type Session = {
61+
session: { id: string };
62+
getCbuildRun: () => Promise<{ getScvdFilePaths: () => string[] } | undefined>;
63+
refreshTimer: { onRefresh: (cb: (session: Session) => void) => void };
64+
};
65+
5866
type TrackerCallbacks = {
5967
onWillStopSession: (cb: (session: Session) => Promise<void>) => { dispose: jest.Mock };
6068
onConnected: (cb: (session: Session) => Promise<void>) => { dispose: jest.Mock };
61-
onDidChangeActiveStackItem: (cb: (item: StackItem) => Promise<void>) => { dispose: jest.Mock };
6269
onDidChangeActiveDebugSession: (cb: (session: Session | undefined) => Promise<void>) => { dispose: jest.Mock };
63-
onStopped: (cb: (session: { session: Session }) => Promise<void>) => { dispose: jest.Mock };
70+
onStackTrace: (cb: (session: { session: Session }) => Promise<void>) => { dispose: jest.Mock };
71+
onWillStartSession: (cb: (session: Session) => Promise<void>) => { dispose: jest.Mock };
6472
callbacks: Partial<{
6573
willStop: (session: Session) => Promise<void>;
6674
connected: (session: Session) => Promise<void>;
67-
stackItem: (item: StackItem) => Promise<void>;
6875
activeSession: (session: Session | undefined) => Promise<void>;
69-
stopped: (session: { session: Session }) => Promise<void>;
76+
stackTrace: (session: { session: Session }) => Promise<void>;
77+
willStart: (session: Session) => Promise<void>;
7078
}>;
7179
};
7280

73-
type Session = {
74-
session: { id: string };
75-
getCbuildRun: () => Promise<{ getScvdFilePaths: () => string[] } | undefined>;
76-
refreshTimer: { onRefresh: (cb: (session: Session) => void) => void };
77-
};
78-
79-
type StackItem = { item: { frameId?: number } };
80-
8181
type Context = { subscriptions: Array<{ dispose: jest.Mock }> };
8282

83-
describe('ComponentViewerController', () => {
83+
describe('ComponentViewer', () => {
8484
beforeEach(() => {
8585
jest.clearAllMocks();
8686
});
@@ -99,16 +99,16 @@ describe('ComponentViewerController', () => {
9999
callbacks.connected = cb;
100100
return { dispose: jest.fn() };
101101
},
102-
onDidChangeActiveStackItem: (cb) => {
103-
callbacks.stackItem = cb;
104-
return { dispose: jest.fn() };
105-
},
106102
onDidChangeActiveDebugSession: (cb) => {
107103
callbacks.activeSession = cb;
108104
return { dispose: jest.fn() };
109105
},
110-
onStopped: (cb) => {
111-
callbacks.stopped = cb;
106+
onStackTrace: (cb) => {
107+
callbacks.stackTrace = cb;
108+
return { dispose: jest.fn() };
109+
},
110+
onWillStartSession: (cb) => {
111+
callbacks.willStart = cb;
112112
return { dispose: jest.fn() };
113113
},
114114
};
@@ -127,7 +127,7 @@ describe('ComponentViewerController', () => {
127127
const tracker = makeTracker();
128128
const controller = new ComponentViewer(context as unknown as ExtensionContext);
129129

130-
await controller.activate(tracker as unknown as GDBTargetDebugTracker);
130+
controller.activate(tracker as unknown as GDBTargetDebugTracker);
131131

132132
expect(registerTreeDataProvider).toHaveBeenCalledWith('cmsis-debugger.componentViewer', expect.any(Object));
133133
expect(context.subscriptions.length).toBe(6);
@@ -147,6 +147,9 @@ describe('ComponentViewerController', () => {
147147
refreshTimer: { onRefresh: jest.fn() },
148148
};
149149
await readScvdFiles(tracker, sessionNoReader);
150+
151+
const instances = (controller as unknown as { _instances: unknown[] })._instances;
152+
expect(instances).toEqual([]);
150153
});
151154

152155
it('skips reading when no scvd files are listed', async () => {
@@ -172,73 +175,63 @@ describe('ComponentViewerController', () => {
172175

173176
const instances = (controller as unknown as { _instances: unknown[] })._instances;
174177
expect(instances.length).toBe(2);
175-
});
176-
177-
it('skips scvd instances when active session is missing', async () => {
178-
const controller = new ComponentViewer(makeContext() as unknown as ExtensionContext);
179-
const tracker = makeTracker();
180-
const session = makeSession('s1', ['a.scvd']);
181-
182-
const readScvdFiles = (controller as unknown as { readScvdFiles: (t: TrackerCallbacks, s?: Session) => Promise<void> }).readScvdFiles.bind(controller);
183-
await readScvdFiles(tracker, session);
184-
185-
const instances = (controller as unknown as { _instances: unknown[] })._instances;
186-
expect(instances.length).toBe(0);
178+
expect(instanceFactory).toHaveBeenCalledTimes(2);
187179
});
188180

189181
it('handles tracker events and updates sessions', async () => {
190182
const context = makeContext();
191183
const tracker = makeTracker();
192184
const controller = new ComponentViewer(context as unknown as ExtensionContext);
193-
await controller.activate(tracker as unknown as GDBTargetDebugTracker);
185+
controller.activate(tracker as unknown as GDBTargetDebugTracker);
186+
187+
const provider = (controller as unknown as { _componentViewerTreeDataProvider?: ReturnType<typeof treeProviderFactory> })._componentViewerTreeDataProvider;
194188

195189
const session = makeSession('s1', ['a.scvd']);
196190
const otherSession = makeSession('s2', []);
197191

198-
await tracker.callbacks.connected?.(session);
192+
await tracker.callbacks.willStart?.(session);
199193
await tracker.callbacks.connected?.(session);
200194

201195
const refreshCallback = (session.refreshTimer.onRefresh as jest.Mock).mock.calls[0]?.[0];
196+
expect(refreshCallback).toBeDefined();
202197
if (refreshCallback) {
203198
await refreshCallback(session);
204199
await refreshCallback(otherSession);
205200
}
206201

207202
await tracker.callbacks.connected?.(otherSession);
203+
expect(provider?.deleteModels).toHaveBeenCalled();
204+
208205
await tracker.callbacks.activeSession?.(session);
209206
await tracker.callbacks.activeSession?.(undefined);
210207

211-
await tracker.callbacks.stackItem?.({ item: { frameId: 1 } });
212-
await tracker.callbacks.stackItem?.({ item: {} });
213-
214208
(controller as unknown as { _activeSession?: Session })._activeSession = session;
215-
await tracker.callbacks.stopped?.({ session });
216-
await tracker.callbacks.stopped?.({ session: otherSession });
209+
await tracker.callbacks.stackTrace?.({ session });
210+
await tracker.callbacks.stackTrace?.({ session: otherSession });
211+
217212
(controller as unknown as { _activeSession?: Session })._activeSession = session;
218213
await tracker.callbacks.willStop?.(session);
219214
(controller as unknown as { _activeSession?: Session })._activeSession = otherSession;
220215
await tracker.callbacks.willStop?.(session);
221216
});
222217

223-
it('updates instances and respects semaphore and empty states', async () => {
218+
it('updates instances when active session and instances are present', async () => {
224219
const context = makeContext();
225220
const controller = new ComponentViewer(context as unknown as ExtensionContext);
226221
const provider = treeProviderFactory();
227222
(controller as unknown as { _componentViewerTreeDataProvider?: typeof provider })._componentViewerTreeDataProvider = provider;
228223

229224
const updateInstances = (controller as unknown as { updateInstances: () => Promise<void> }).updateInstances.bind(controller);
230225

231-
(controller as unknown as { _updateSemaphoreFlag: boolean })._updateSemaphoreFlag = true;
232-
await updateInstances();
233-
234-
(controller as unknown as { _updateSemaphoreFlag: boolean })._updateSemaphoreFlag = false;
235226
(controller as unknown as { _activeSession?: Session | undefined })._activeSession = undefined;
236227
await updateInstances();
237-
expect(provider.deleteModels).toHaveBeenCalled();
228+
expect(provider.deleteModels).toHaveBeenCalledTimes(1);
229+
provider.deleteModels.mockClear();
238230

239231
(controller as unknown as { _activeSession?: Session | undefined })._activeSession = makeSession('s1');
240232
(controller as unknown as { _instances: unknown[] })._instances = [];
241233
await updateInstances();
234+
expect(provider.deleteModels).not.toHaveBeenCalled();
242235

243236
const instanceA = instanceFactory();
244237
const instanceB = instanceFactory();
@@ -247,5 +240,7 @@ describe('ComponentViewerController', () => {
247240
expect(provider.resetModelCache).toHaveBeenCalled();
248241
expect(provider.addGuiOut).toHaveBeenCalledTimes(2);
249242
expect(provider.showModelData).toHaveBeenCalled();
243+
expect(instanceA.update).toHaveBeenCalled();
244+
expect(instanceB.update).toHaveBeenCalled();
250245
});
251246
});

0 commit comments

Comments
 (0)