Skip to content

Commit

Permalink
throttle save participants in certain cases (#102542)
Browse files Browse the repository at this point in the history
  • Loading branch information
bpasero committed Jul 16, 2020
1 parent a05390d commit 69fd7af
Showing 1 changed file with 37 additions and 3 deletions.
40 changes: 37 additions & 3 deletions src/vs/workbench/services/textfile/common/textFileEditorModel.ts
Expand Up @@ -75,6 +75,9 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
private bufferSavedVersionId: number | undefined;
private ignoreDirtyOnModelContentChange = false;

private static readonly UNDO_REDO_SAVE_PARTICIPANTS_AUTO_SAVE_THROTTLE_THRESHOLD = 500;
private lastModelContentChangeFromUndoRedo: number | undefined = undefined;

private lastResolvedFileStat: IFileStatWithMetadata | undefined;

private readonly saveSequentializer = new TaskSequentializer();
Expand Down Expand Up @@ -450,16 +453,23 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
// where `value` was captured in the content change listener closure scope.

// Content Change
this._register(model.onDidChangeContent(() => this.onModelContentChanged(model)));
this._register(model.onDidChangeContent(e => this.onModelContentChanged(model, e.isUndoing || e.isRedoing)));
}

private onModelContentChanged(model: ITextModel): void {
private onModelContentChanged(model: ITextModel, isUndoingOrRedoing: boolean): void {
this.logService.trace(`[text file model] onModelContentChanged() - enter`, this.resource.toString(true));

// In any case increment the version id because it tracks the textual content state of the model at all times
this.versionId++;
this.logService.trace(`[text file model] onModelContentChanged() - new versionId ${this.versionId}`, this.resource.toString(true));

// Remember when the user changed the model through a undo/redo operation.
// We need this information to throttle save participants to fix
// https://github.com/microsoft/vscode/issues/102542
if (isUndoingOrRedoing) {
this.lastModelContentChangeFromUndoRedo = Date.now();
}

// We mark check for a dirty-state change upon model content change, unless:
// - explicitly instructed to ignore it (e.g. from model.load())
// - the model is readonly (in that case we never assume the change was done by the user)
Expand Down Expand Up @@ -639,7 +649,31 @@ export class TextFileEditorModel extends BaseTextEditorModel implements ITextFil
// Save participants can also be skipped through API.
if (this.isResolved() && !options.skipSaveParticipants) {
try {
await this.textFileService.files.runSaveParticipants(this, { reason: options.reason ?? SaveReason.EXPLICIT }, saveCancellation.token);

// Measure the time it took from the last undo/redo operation to this save. If this
// time is below `UNDO_REDO_SAVE_PARTICIPANTS_THROTTLE_THRESHOLD`, we make sure to
// delay the save participant for the remaining time if the reason is auto save.
//
// This fixes the following issue:
// - the user has configured auto save with delay of 100ms or shorter
// - the user has a save participant enabled that modifies the file on each save
// - the user types into the file and the file gets saved
// - the user triggers undo operation
// - this will undo the save participant change but trigger the save participant right after
// - the user has no chance to undo over the save participant
//
// Reported as: https://github.com/microsoft/vscode/issues/102542
if (options.reason === SaveReason.AUTO && typeof this.lastModelContentChangeFromUndoRedo === 'number') {
const timeFromUndoRedoToSave = Date.now() - this.lastModelContentChangeFromUndoRedo;
if (timeFromUndoRedoToSave < TextFileEditorModel.UNDO_REDO_SAVE_PARTICIPANTS_AUTO_SAVE_THROTTLE_THRESHOLD) {
await timeout(TextFileEditorModel.UNDO_REDO_SAVE_PARTICIPANTS_AUTO_SAVE_THROTTLE_THRESHOLD - timeFromUndoRedoToSave);
}
}

// Run save participants unless save was cancelled meanwhile
if (!saveCancellation.token.isCancellationRequested) {
await this.textFileService.files.runSaveParticipants(this, { reason: options.reason ?? SaveReason.EXPLICIT }, saveCancellation.token);
}
} catch (error) {
this.logService.error(`[text file model] runSaveParticipants(${versionId}) - resulted in an error: ${error.toString()}`, this.resource.toString(true));
}
Expand Down

0 comments on commit 69fd7af

Please sign in to comment.