Skip to content

Commit

Permalink
[debug] Lazily update frames of all threads in all-stop mode
Browse files Browse the repository at this point in the history
A re-make of d1678ad.

The changed bits:

```diff
@@ -181,7 +196,17 @@ export class DebugThread extends DebugThreadData implements TreeElement {
         return [...result.values()];
     }
     protected clearFrames(): void {
+        // Clear all frames
         this._frames.clear();
+
+        // Cancel all request promises
+        this.pendingFetchCancel.cancel();
+        this.pendingFetchCancel = new CancellationTokenSource();
+
+        // Empty all current requests
+        this.pendingFetch = Promise.resolve([]);
+        this._pendingFetchCount = 0;
+
         this.updateCurrentFrame();
     }
     protected updateCurrentFrame(): void {
```

So what's the difference? Well, if you got multiple stops fast enough,
pending frames would be blocking the refresh from happening. Since
`updateFrames` was being called from one more place (when switching
thread), it forced me to avoid pointless lookups as to not error (GDB
complains when you're trying to fetch something that has already been
fetched). This new behavior that avoids pointless refreshes didn't
take speed into account, and could potentially block an entirely new
fetch.

To fix this, I cancel all frame fetches when clearing frames (which is
done when updating a thread in all-stop mode).

Signed-off-by: Samuel Frylmark <samuel.frylmark@ericsson.com>
  • Loading branch information
sfrylmark authored and marcdumais-work committed Mar 16, 2020
1 parent b553a22 commit f015148
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 2 deletions.
8 changes: 7 additions & 1 deletion packages/debug/src/browser/debug-session.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,10 @@ export class DebugSession implements CompositeTreeElement {
}
}),
this.on('stopped', async ({ body }) => {
// Update thread list
await this.updateThreads(body);

// Update current thread's frames immediately
await this.updateFrames();
}),
this.on('thread', ({ body: { reason, threadId } }) => {
Expand Down Expand Up @@ -221,6 +224,9 @@ export class DebugSession implements CompositeTreeElement {
this.fireDidChange();
if (thread) {
this.toDisposeOnCurrentThread.push(thread.onDidChanged(() => this.fireDidChange()));

// If this thread is missing stack frame information, then load that.
this.updateFrames();
}
}

Expand Down Expand Up @@ -465,7 +471,7 @@ export class DebugSession implements CompositeTreeElement {

protected async updateFrames(): Promise<void> {
const thread = this._currentThread;
if (!thread || thread.frameCount) {
if (!thread || thread.pendingFrameCount || thread.frameCount) {
return;
}
if (this.capabilities.supportsDelayedStackTraceLoading) {
Expand Down
27 changes: 26 additions & 1 deletion packages/debug/src/browser/model/debug-thread.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
********************************************************************************/

import * as React from 'react';
import { Event, Emitter } from '@theia/core';
import { CancellationTokenSource, Emitter, Event } from '@theia/core';
import { DebugProtocol } from 'vscode-debugprotocol/lib/debugProtocol';
import { TreeElement } from '@theia/core/lib/browser/source-tree';
import { DebugStackFrame } from './debug-stack-frame';
Expand Down Expand Up @@ -140,18 +140,33 @@ export class DebugThread extends DebugThreadData implements TreeElement {
}

protected pendingFetch = Promise.resolve<DebugStackFrame[]>([]);
protected _pendingFetchCount: number = 0;
protected pendingFetchCancel = new CancellationTokenSource();
async fetchFrames(levels: number = 20): Promise<DebugStackFrame[]> {
const cancel = this.pendingFetchCancel.token;
this._pendingFetchCount += 1;

return this.pendingFetch = this.pendingFetch.then(async () => {
try {
const start = this.frameCount;
const frames = await this.doFetchFrames(start, levels);
if (cancel.isCancellationRequested) {
return [];
}
return this.doUpdateFrames(frames);
} catch (e) {
console.error(e);
return [];
} finally {
if (!cancel.isCancellationRequested) {
this._pendingFetchCount -= 1;
}
}
});
}
get pendingFrameCount(): number {
return this._pendingFetchCount;
}
protected async doFetchFrames(startFrame: number, levels: number): Promise<DebugProtocol.StackFrame[]> {
try {
const response = await this.session.sendRequest('stackTrace',
Expand Down Expand Up @@ -181,7 +196,17 @@ export class DebugThread extends DebugThreadData implements TreeElement {
return [...result.values()];
}
protected clearFrames(): void {
// Clear all frames
this._frames.clear();

// Cancel all request promises
this.pendingFetchCancel.cancel();
this.pendingFetchCancel = new CancellationTokenSource();

// Empty all current requests
this.pendingFetch = Promise.resolve([]);
this._pendingFetchCount = 0;

this.updateCurrentFrame();
}
protected updateCurrentFrame(): void {
Expand Down

0 comments on commit f015148

Please sign in to comment.