Skip to content

Commit

Permalink
Fix some problems with progress edits (#194838)
Browse files Browse the repository at this point in the history
  • Loading branch information
alexdima authored Oct 4, 2023
1 parent 44bd95e commit bf2ab76
Show file tree
Hide file tree
Showing 3 changed files with 102 additions and 12 deletions.
49 changes: 43 additions & 6 deletions src/vs/platform/progress/common/progress.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,9 @@ export class Progress<T> implements IProgress<T> {
private _value?: T;
get value(): T | undefined { return this._value; }

private _lastTask?: Promise<unknown>;
private _asyncQueue?: T[];
private _processingAsyncQueue?: boolean;
private _drainListener: (() => void) | undefined;

constructor(private callback: (data: T) => unknown, opts?: { async?: boolean }) {
this.report = opts?.async
Expand All @@ -130,11 +132,46 @@ export class Progress<T> implements IProgress<T> {
}

private _reportAsync(item: T) {
Promise.resolve(this._lastTask).finally(() => {
this._value = item;
const r = this.callback(this._value);
this._lastTask = Promise.resolve(r).finally(() => this._lastTask = undefined);
});
if (!this._asyncQueue) {
this._asyncQueue = [item];
} else {
this._asyncQueue.push(item);
}
this._processAsyncQueue();
}

private async _processAsyncQueue() {
if (this._processingAsyncQueue) {
return;
}
try {
this._processingAsyncQueue = true;

while (this._asyncQueue && this._asyncQueue.length) {
const item = this._asyncQueue.shift()!;
this._value = item;
await this.callback(this._value);
}

} finally {
this._processingAsyncQueue = false;
const drainListener = this._drainListener;
this._drainListener = undefined;
drainListener?.();
}
}

public drain(): Promise<void> {
if (this._processingAsyncQueue) {
return new Promise<void>(resolve => {
const prevListener = this._drainListener;
this._drainListener = () => {
prevListener?.();
resolve();
};
});
}
return Promise.resolve();
}
}

Expand Down
49 changes: 49 additions & 0 deletions src/vs/platform/progress/test/common/progress.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/

import * as assert from 'assert';
import { runWithFakedTimers } from 'vs/base/test/common/timeTravelScheduler';
import { Progress } from 'vs/platform/progress/common/progress';

suite('Progress', () => {
test('multiple report calls are processed in sequence', async () => {
await runWithFakedTimers({ useFakeTimers: true, maxTaskCount: 100 }, async () => {
const executionOrder: string[] = [];
const timeout = (time: number) => {
return new Promise<void>(resolve => setTimeout(resolve, time));
};
const executor = async (value: number) => {
executionOrder.push(`start ${value}`);
if (value === 1) {
// 1 is slowest
await timeout(100);
} else if (value === 2) {
// 2 is also slow
await timeout(50);
} else {
// 3 is fast
await timeout(10);
}
executionOrder.push(`end ${value}`);
};
const progress = new Progress<number>(executor, { async: true });

progress.report(1);
progress.report(2);
progress.report(3);

await timeout(1000);

assert.deepStrictEqual(executionOrder, [
'start 1',
'end 1',
'start 2',
'end 2',
'start 3',
'end 3',
]);
});
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -547,7 +547,7 @@ export class InlineChatController implements IEditorContribution {
}
if (data.edits) {
progressEdits.push(data.edits);
await this._makeChanges(progressEdits);
await this._makeChanges(progressEdits, false);
}
}, { async: true });
const task = this._activeSession.provider.provideResponse(this._activeSession.session, request, progress, requestCts.token);
Expand All @@ -561,12 +561,15 @@ export class InlineChatController implements IEditorContribution {
this._ctxHasActiveRequest.set(true);
reply = await raceCancellationError(Promise.resolve(task), requestCts.token);

// we must wait for all edits that came in via progress to complete
await progress.drain();

if (reply?.type === InlineChatResponseType.Message) {
response = new MarkdownResponse(this._activeSession.textModelN.uri, reply);
} else if (reply) {
const editResponse = new EditResponse(this._activeSession.textModelN.uri, this._activeSession.textModelN.getAlternativeVersionId(), reply, progressEdits);
if (editResponse.allLocalEdits.length > progressEdits.length) {
await this._makeChanges(editResponse.allLocalEdits);
await this._makeChanges(editResponse.allLocalEdits, true);
}
response = editResponse;
} else {
Expand Down Expand Up @@ -619,7 +622,7 @@ export class InlineChatController implements IEditorContribution {
return State.SHOW_RESPONSE;
}

private async _makeChanges(allEdits: TextEdit[][]) {
private async _makeChanges(allEdits: TextEdit[][], computeMoreMinimalEdits: boolean) {
assertType(this._activeSession);
assertType(this._strategy);

Expand All @@ -628,17 +631,18 @@ export class InlineChatController implements IEditorContribution {
}

// diff-changes from model0 -> modelN+1
for (const edits of allEdits) {
{
const lastEdits = allEdits[allEdits.length - 1];
const textModelNplus1 = this._modelService.createModel(createTextBufferFactoryFromSnapshot(this._activeSession.textModelN.createSnapshot()), null, undefined, true);
textModelNplus1.applyEdits(edits.map(TextEdit.asEditOperation));
textModelNplus1.applyEdits(lastEdits.map(TextEdit.asEditOperation));
const diff = await this._editorWorkerService.computeDiff(this._activeSession.textModel0.uri, textModelNplus1.uri, { ignoreTrimWhitespace: false, maxComputationTimeMs: 5000, computeMoves: false }, 'advanced');
this._activeSession.lastTextModelChanges = diff?.changes ?? [];
textModelNplus1.dispose();
}

// make changes from modelN -> modelN+1
const lastEdits = allEdits[allEdits.length - 1];
const moreMinimalEdits = await this._editorWorkerService.computeHumanReadableDiff(this._activeSession.textModelN.uri, lastEdits);
const moreMinimalEdits = computeMoreMinimalEdits ? await this._editorWorkerService.computeHumanReadableDiff(this._activeSession.textModelN.uri, lastEdits) : undefined;
const editOperations = (moreMinimalEdits ?? lastEdits).map(TextEdit.asEditOperation);
this._log('edits from PROVIDER and after making them MORE MINIMAL', this._activeSession.provider.debugName, lastEdits, moreMinimalEdits);

Expand Down

0 comments on commit bf2ab76

Please sign in to comment.