Skip to content

perf(language-server): memoize possibly heavy IO utils #1001

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions server/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
},
"dependencies": {
"chokidar": "^3.5.1",
"memize": "^2.1.0",
"vscode-jsonrpc": "^8.0.1",
"vscode-languageserver": "^8.0.1",
"vscode-languageserver-protocol": "^3.17.1"
Expand Down
15 changes: 2 additions & 13 deletions server/src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,17 +74,6 @@ let codeActionsFromDiagnostics: codeActions.filesCodeActions = {};
// will be properly defined later depending on the mode (stdio/node-rpc)
let send: (msg: p.Message) => void = (_) => {};

let findRescriptBinary = (projectRootPath: p.DocumentUri | null) =>
config.extensionConfiguration.binaryPath == null
? lookup.findFilePathFromProjectRoot(
projectRootPath,
path.join(c.nodeModulesBinDir, c.rescriptBinName)
)
: utils.findBinary(
config.extensionConfiguration.binaryPath,
c.rescriptBinName
);

let createInterfaceRequest = new v.RequestType<
p.TextDocumentIdentifier,
p.TextDocumentIdentifier,
Expand Down Expand Up @@ -271,7 +260,7 @@ let openedFile = (fileUri: string, fileContent: string) => {
// TODO: sometime stale .bsb.lock dangling. bsb -w knows .bsb.lock is
// stale. Use that logic
// TODO: close watcher when lang-server shuts down
if (findRescriptBinary(projectRootPath) != null) {
if (utils.findRescriptBinary(projectRootPath) != null) {
let payload: clientSentBuildAction = {
title: c.startBuildAction,
projectRootPath: projectRootPath,
Expand Down Expand Up @@ -1295,7 +1284,7 @@ function onMessage(msg: p.Message) {
// TODO: close watcher when lang-server shuts down. However, by Node's
// default, these subprocesses are automatically killed when this
// language-server process exits
let rescriptBinaryPath = findRescriptBinary(projectRootPath);
let rescriptBinaryPath = utils.findRescriptBinary(projectRootPath);
if (rescriptBinaryPath != null) {
let bsbProcess = utils.runBuildWatcherUsingValidBuildPath(
rescriptBinaryPath,
Expand Down
55 changes: 28 additions & 27 deletions server/src/utils.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,14 @@
import * as childProcess from "child_process";
import * as path from "node:path";
import * as fs from "node:fs";
import * as os from "node:os";
import * as childProcess from "node:child_process";
import * as p from "vscode-languageserver-protocol";
import * as path from "path";
import * as t from "vscode-languageserver-types";
import {
RequestMessage,
ResponseMessage,
} from "vscode-languageserver-protocol";
import fs from "fs";
import * as os from "os";
import memo from "memize";

import * as codeActions from "./codeActions";
import * as c from "./constants";
Expand All @@ -26,7 +27,7 @@ export let createFileInTempDir = (extension = "") => {

// TODO: races here?
// TODO: this doesn't handle file:/// scheme
export let findProjectRootOfFile = (
export let findProjectRootOfFile = memo((
source: p.DocumentUri
): null | p.DocumentUri => {
let dir = path.dirname(source);
Expand All @@ -43,10 +44,10 @@ export let findProjectRootOfFile = (
return findProjectRootOfFile(dir);
}
}
};
});

// Check if binaryName exists inside binaryDirPath and return the joined path.
export let findBinary = (
export let findBinary = memo((
binaryDirPath: p.DocumentUri | null,
binaryName: string
): p.DocumentUri | null => {
Expand All @@ -59,7 +60,21 @@ export let findBinary = (
} else {
return null;
}
};
});

export let findRescriptBinary = memo((
projectRootPath: p.DocumentUri | null
) =>
config.extensionConfiguration.binaryPath == null
? lookup.findFilePathFromProjectRoot(
projectRootPath,
path.join(c.nodeModulesBinDir, c.rescriptBinName)
)
: findBinary(
config.extensionConfiguration.binaryPath,
c.rescriptBinName
)
);

type execResult =
| {
Expand Down Expand Up @@ -138,28 +153,14 @@ export let formatCode = (
}
};

let findReScriptVersion = (filePath: p.DocumentUri): string | undefined => {
let projectRoot = findProjectRootOfFile(filePath);
if (projectRoot == null) {
return undefined;
}

let rescriptBinary = lookup.findFilePathFromProjectRoot(
projectRoot,
path.join(c.nodeModulesBinDir, c.rescriptBinName)
);

if (rescriptBinary == null) {
return undefined;
}

let findReScriptVersion = memo((): string | undefined => {
try {
let version = childProcess.execSync(`${rescriptBinary} -v`);
return version.toString().trim();
let { version } = require('rescript/package.json');
return version;
} catch (e) {
return undefined;
}
};
});

export let runAnalysisAfterSanityCheck = (
filePath: p.DocumentUri,
Expand All @@ -179,7 +180,7 @@ export let runAnalysisAfterSanityCheck = (
if (projectRootPath == null && projectRequired) {
return null;
}
let rescriptVersion = findReScriptVersion(filePath);
let rescriptVersion = findReScriptVersion();
let options: childProcess.ExecFileSyncOptions = {
cwd: projectRootPath || undefined,
maxBuffer: Infinity,
Expand Down
Loading