Skip to content

Commit

Permalink
Eagerly compute SHA's for watched files
Browse files Browse the repository at this point in the history
  • Loading branch information
@vinistock
vinistock committed Nov 19, 2024
1 parent a3ed380 commit c5de676
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 43 deletions.
44 changes: 40 additions & 4 deletions vscode/src/test/suite/workspace.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -59,15 +59,15 @@ suite("Workspace", () => {

await workspace.activate();

for (let i = 0; i < 5; i++) {
await new Promise((resolve) => setTimeout(resolve, 200));
for (let i = 0; i < 4; i++) {
await new Promise((resolve) => setTimeout(resolve, 50));
fs.writeFileSync(path.join(gitDir, "rebase-apply"), "1");
await new Promise((resolve) => setTimeout(resolve, 200));
await new Promise((resolve) => setTimeout(resolve, 50));
fs.rmSync(path.join(gitDir, "rebase-apply"));
}

// Give enough time for all watchers to fire and all debounces to run off
await new Promise((resolve) => setTimeout(resolve, 10000));
await new Promise((resolve) => setTimeout(resolve, 6000));

startStub.restore();
restartSpy.restore();
Expand All @@ -77,4 +77,40 @@ suite("Workspace", () => {
// The restart call only happens once because of the debounce
assert.strictEqual(restartSpy.callCount, 1);
}).timeout(60000);

test("does not restart when watched files are touched without modifying contents", async () => {
const lockfileUri = vscode.Uri.file(
path.join(workspacePath, "Gemfile.lock"),
);
const contents = Buffer.from("hello");
await vscode.workspace.fs.writeFile(lockfileUri, contents);

const workspace = new Workspace(
context,
workspaceFolder,
FAKE_TELEMETRY,
() => {},
new Map(),
);

await workspace.activate();

const startStub = sinon.stub(workspace, "start");
const restartSpy = sinon.spy(workspace, "restart");

for (let i = 0; i < 4; i++) {
const updateDate = new Date();
fs.utimesSync(lockfileUri.fsPath, updateDate, updateDate);
await new Promise((resolve) => setTimeout(resolve, 50));
}

// Give enough time for all watchers to fire and all debounces to run off
await new Promise((resolve) => setTimeout(resolve, 6000));

startStub.restore();
restartSpy.restore();

assert.strictEqual(startStub.callCount, 0);
assert.strictEqual(restartSpy.callCount, 0);
}).timeout(10000);
});
93 changes: 54 additions & 39 deletions vscode/src/workspace.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,13 @@ import {
} from "./common";
import { WorkspaceChannel } from "./workspaceChannel";

const WATCHED_FILES = [
"Gemfile.lock",
"gems.locked",
".rubocop.yml",
".rubocop",
];

export class Workspace implements WorkspaceInterface {
public lspClient?: Client;
public readonly ruby: Ruby;
Expand Down Expand Up @@ -58,8 +65,6 @@ export class Workspace implements WorkspaceInterface {
this.createTestItems = createTestItems;
this.isMainWorkspace = isMainWorkspace;
this.virtualDocuments = virtualDocuments;

this.registerRestarts(context);
}

// Activate this workspace. This method is intended to be invoked only once, unlikely `start` which may be invoked
Expand All @@ -82,6 +87,20 @@ export class Workspace implements WorkspaceInterface {
rootGitUri,
".git/{rebase-merge,rebase-apply,BISECT_START,CHERRY_PICK_HEAD}",
);

this.registerRestarts();

// Eagerly calculate SHAs for the watched files to avoid unnecessary restarts
for (const file of WATCHED_FILES) {
const uri = vscode.Uri.joinPath(this.workspaceFolder.uri, file);
const currentSha = await this.fileContentsSha(uri);

if (!currentSha) {
continue;
}

this.restartDocumentShas.set(uri.fsPath, currentSha);
}
}

async start(debugMode?: boolean) {
Expand Down Expand Up @@ -164,7 +183,9 @@ export class Workspace implements WorkspaceInterface {
// server can now handle shutdown requests
if (this.needsRestart) {
this.needsRestart = false;
await this.restart();
await this.debouncedRestart(
"a restart was requested while the server was still booting",
);
}
} catch (error: any) {
this.error = true;
Expand Down Expand Up @@ -342,67 +363,49 @@ export class Workspace implements WorkspaceInterface {
return result;
}

private registerRestarts(context: vscode.ExtensionContext) {
this.createRestartWatcher(context, "Gemfile.lock");
this.createRestartWatcher(context, "gems.locked");
this.createRestartWatcher(context, "**/.rubocop.yml");
this.createRestartWatcher(context, ".rubocop");
private registerRestarts() {
this.createRestartWatcher(`{${WATCHED_FILES.join(",")}}`);

// If a configuration that affects the Ruby LSP has changed, update the client options using the latest
// configuration and restart the server
context.subscriptions.push(
this.context.subscriptions.push(
vscode.workspace.onDidChangeConfiguration(async (event) => {
if (event.affectsConfiguration("rubyLsp")) {
// Re-activate Ruby if the version manager changed
if (
event.affectsConfiguration("rubyLsp.rubyVersionManager") ||
event.affectsConfiguration("rubyLsp.bundleGemfile") ||
event.affectsConfiguration("rubyLsp.customRubyCommand")
) {
await this.ruby.activateRuby();
}

this.outputChannel.info(
"Restarting the Ruby LSP because configuration changed",
);
await this.restart();
await this.debouncedRestart("configuration changed");
}
}),
);
}

private createRestartWatcher(
context: vscode.ExtensionContext,
pattern: string,
) {
private createRestartWatcher(pattern: string) {
const watcher = vscode.workspace.createFileSystemWatcher(
new vscode.RelativePattern(this.workspaceFolder, pattern),
);

// Handler for only triggering restart if the contents of the file have been modified. If the file was just touched,
// but the contents are the same, we don't want to restart
const debouncedRestartWithHashCheck = async (uri: vscode.Uri) => {
const fileContents = await vscode.workspace.fs.readFile(uri);
const fsPath = uri.fsPath;
const currentSha = await this.fileContentsSha(uri);

const hash = createHash("sha256");
hash.update(fileContents.toString());
const currentSha = hash.digest("hex");

if (this.restartDocumentShas.get(fsPath) !== currentSha) {
if (currentSha && this.restartDocumentShas.get(fsPath) !== currentSha) {
this.restartDocumentShas.set(fsPath, currentSha);
await this.debouncedRestart(`${pattern} changed`);
await this.debouncedRestart(`${fsPath} changed, matching ${pattern}`);
}
};

const debouncedRestart = async () => {
await this.debouncedRestart(`${pattern} changed`);
const debouncedRestart = async (uri: vscode.Uri) => {
this.restartDocumentShas.delete(uri.fsPath);
await this.debouncedRestart(`${uri.fsPath} changed, matching ${pattern}`);
};

context.subscriptions.push(
this.context.subscriptions.push(
watcher,
watcher.onDidChange(debouncedRestartWithHashCheck),
watcher.onDidCreate(debouncedRestart),
// Interestingly, we are seeing create events being fired even when the file already exists. If a create event is
// fired for an update to an existing file, then we need to check if the contents still match to prevent unwanted
// restarts
watcher.onDidCreate(debouncedRestartWithHashCheck),
watcher.onDidDelete(debouncedRestart),
);
}
Expand All @@ -416,9 +419,9 @@ export class Workspace implements WorkspaceInterface {
this.#inhibitRestart = true;
};

const stop = async () => {
const stop = async (uri: vscode.Uri) => {
this.#inhibitRestart = false;
await this.debouncedRestart(`${glob} changed`);
await this.debouncedRestart(`${uri.fsPath} changed, matching ${glob}`);
};

this.context.subscriptions.push(
Expand All @@ -429,4 +432,16 @@ export class Workspace implements WorkspaceInterface {
workspaceWatcher.onDidDelete(stop),
);
}

private async fileContentsSha(uri: vscode.Uri): Promise<string | undefined> {
try {
const fileContents = await vscode.workspace.fs.readFile(uri);

const hash = createHash("sha256");
hash.update(fileContents.toString());
return hash.digest("hex");
} catch (error: any) {
return undefined;
}
}
}

0 comments on commit c5de676

Please sign in to comment.