Reduce language server latency (#1003)

* avoid recomputing project roots and ReScript versions as much as possible * more precomputation * changelog
rescript-lang · Jun 11, 2024 · aed1e8a · aed1e8a
1 parent 74728bf
commit aed1e8a
Show file tree

Hide file tree

Showing 5 changed files with 117 additions and 63 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,10 @@
 
 ## master
 
+#### :nail_care: Polish
+
+- Reduce latency of language server by caching a few project config related things. https://github.com/rescript-lang/rescript-vscode/pull/1003
+
 #### :bug: Bug Fix
 
 - Fix edge case in switch expr completion. https://github.com/rescript-lang/rescript-vscode/pull/1002

diff --git a/server/src/incrementalCompilation.ts b/server/src/incrementalCompilation.ts
@@ -10,6 +10,7 @@ import config, { send } from "./config";
 import * as c from "./constants";
 import * as chokidar from "chokidar";
 import { fileCodeActions } from "./codeActions";
+import { projectsFiles } from "./projectFiles";
 
 function debug() {
   return (
@@ -75,8 +76,6 @@ type IncrementallyCompiledFileInfo = {
     callArgs: Promise<Array<string> | null>;
     /** The location of the incremental folder for this project. */
     incrementalFolderPath: string;
-    /** The ReScript version. */
-    rescriptVersion: string;
   };
   /** Any code actions for this incremental file. */
   codeActions: Array<fileCodeActions>;
@@ -284,6 +283,8 @@ function getBscArgs(
       });
     } else if (buildSystem === "rewatch") {
       try {
+        const project = projectsFiles.get(entry.project.rootPath);
+        if (project?.rescriptVersion == null) return;
         let rewatchPath = path.resolve(
           entry.project.workspaceRootPath,
           "node_modules/@rolandpeelen/rewatch/rewatch"
@@ -292,7 +293,7 @@ function getBscArgs(
           cp
             .execFileSync(rewatchPath, [
               "--rescript-version",
-              entry.project.rescriptVersion,
+              project.rescriptVersion,
               "--compiler-args",
               entry.file.sourceFilePath,
             ])
@@ -364,21 +365,21 @@ function triggerIncrementalCompilationOfFile(
   if (incrementalFileCacheEntry == null) {
     // New file
     const projectRootPath = utils.findProjectRootOfFile(filePath);
-    const workspaceRootPath = projectRootPath
-      ? utils.findProjectRootOfFile(projectRootPath)
-      : null;
     if (projectRootPath == null) {
       if (debug())
         console.log("Did not find project root path for " + filePath);
       return;
     }
-    const namespaceName = utils.getNamespaceNameFromConfigFile(projectRootPath);
-    if (namespaceName.kind === "error") {
-      if (debug())
-        console.log("Getting namespace config errored for " + filePath);
+    const project = projectsFiles.get(projectRootPath);
+    if (project == null) {
+      if (debug()) console.log("Did not find open project for " + filePath);
       return;
     }
-    const bscBinaryLocation = utils.findBscExeBinary(projectRootPath);
+    const workspaceRootPath = projectRootPath
+      ? utils.findProjectRootOfFile(projectRootPath)
+      : null;
+
+    const bscBinaryLocation = project.bscBinaryLocation;
     if (bscBinaryLocation == null) {
       if (debug())
         console.log("Could not find bsc binary location for " + filePath);
@@ -387,28 +388,15 @@ function triggerIncrementalCompilationOfFile(
     const ext = filePath.endsWith(".resi") ? ".resi" : ".res";
     const moduleName = path.basename(filePath, ext);
     const moduleNameNamespaced =
-      namespaceName.result !== ""
-        ? `${moduleName}-${namespaceName.result}`
+      project.namespaceName != null
+        ? `${moduleName}-${project.namespaceName}`
         : moduleName;
 
     const incrementalFolderPath = path.join(
       projectRootPath,
       INCREMENTAL_FILE_FOLDER_LOCATION
     );
 
-    let rescriptVersion = "";
-    try {
-      rescriptVersion = cp
-        .execFileSync(bscBinaryLocation, ["-version"])
-        .toString()
-        .trim();
-    } catch (e) {
-      console.error(e);
-    }
-    if (rescriptVersion.startsWith("ReScript ")) {
-      rescriptVersion = rescriptVersion.replace("ReScript ", "");
-    }
-
     let originalTypeFileLocation = path.resolve(
       projectRootPath,
       "lib/bs",
@@ -436,7 +424,6 @@ function triggerIncrementalCompilationOfFile(
         callArgs: Promise.resolve([]),
         bscBinaryLocation,
         incrementalFolderPath,
-        rescriptVersion,
       },
       buildRewatch: null,
       buildNinja: null,
@@ -488,6 +475,16 @@ function verifyTriggerToken(filePath: string, triggerToken: number): boolean {
   );
 }
 async function figureOutBscArgs(entry: IncrementallyCompiledFileInfo) {
+  const project = projectsFiles.get(entry.project.rootPath);
+  if (project?.rescriptVersion == null) {
+    if (debug()) {
+      console.log(
+        "Found no project (or ReScript version) for " +
+          entry.file.sourceFilePath
+      );
+    }
+    return null;
+  }
   const res = await getBscArgs(entry);
   if (res == null) return null;
   let astArgs: Array<Array<string>> = [];
@@ -547,7 +544,7 @@ async function figureOutBscArgs(entry: IncrementallyCompiledFileInfo) {
   });
 
   callArgs.push("-color", "never");
-  if (parseInt(entry.project.rescriptVersion.split(".")[0] ?? "10") >= 11) {
+  if (parseInt(project.rescriptVersion.split(".")[0] ?? "10") >= 11) {
     // Only available in v11+
     callArgs.push("-ignore-parse-errors");
   }

diff --git a/server/src/projectFiles.ts b/server/src/projectFiles.ts
@@ -0,0 +1,27 @@
+import * as cp from "node:child_process";
+import * as p from "vscode-languageserver-protocol";
+
+export type filesDiagnostics = {
+  [key: string]: p.Diagnostic[];
+};
+
+interface projectFiles {
+  openFiles: Set<string>;
+  filesWithDiagnostics: Set<string>;
+  filesDiagnostics: filesDiagnostics;
+  rescriptVersion: string | undefined;
+  bscBinaryLocation: string | null;
+  namespaceName: string | null;
+
+  bsbWatcherByEditor: null | cp.ChildProcess;
+
+  // This keeps track of whether we've prompted the user to start a build
+  // automatically, if there's no build currently running for the project. We
+  // only want to prompt the user about this once, or it becomes
+  // annoying.
+  // The type `never` means that we won't show the prompt if the project is inside node_modules
+  hasPromptedToStartBuild: boolean | "never";
+}
+
+export let projectsFiles: Map<string, projectFiles> = // project root path
+  new Map();
diff --git a/server/src/server.ts b/server/src/server.ts
@@ -22,12 +22,11 @@ import * as c from "./constants";
 import * as chokidar from "chokidar";
 import { assert } from "console";
 import { fileURLToPath } from "url";
-import * as cp from "node:child_process";
 import { WorkspaceEdit } from "vscode-languageserver";
-import { filesDiagnostics } from "./utils";
 import { onErrorReported } from "./errorReporter";
 import * as ic from "./incrementalCompilation";
 import config, { extensionConfiguration } from "./config";
+import { projectsFiles } from "./projectFiles";
 
 // This holds client capabilities specific to our extension, and not necessarily
 // related to the LS protocol. It's for enabling/disabling features that might
@@ -49,23 +48,7 @@ let serverSentRequestIdCounter = 0;
 // https://microsoft.github.io/language-server-protocol/specification#exit
 let shutdownRequestAlreadyReceived = false;
 let stupidFileContentCache: Map<string, string> = new Map();
-let projectsFiles: Map<
-  string, // project root path
-  {
-    openFiles: Set<string>;
-    filesWithDiagnostics: Set<string>;
-    filesDiagnostics: filesDiagnostics;
-
-    bsbWatcherByEditor: null | cp.ChildProcess;
-
-    // This keeps track of whether we've prompted the user to start a build
-    // automatically, if there's no build currently running for the project. We
-    // only want to prompt the user about this once, or it becomes
-    // annoying.
-    // The type `never` means that we won't show the prompt if the project is inside node_modules
-    hasPromptedToStartBuild: boolean | "never";
-  }
-> = new Map();
+
 // ^ caching AND states AND distributed system. Why does LSP has to be stupid like this
 
 // This keeps track of code actions extracted from diagnostics.
@@ -275,11 +258,18 @@ let openedFile = (fileUri: string, fileContent: string) => {
       if (config.extensionConfiguration.incrementalTypechecking?.enabled) {
         ic.recreateIncrementalFileFolder(projectRootPath);
       }
+      const namespaceName =
+        utils.getNamespaceNameFromConfigFile(projectRootPath);
+
       projectRootState = {
         openFiles: new Set(),
         filesWithDiagnostics: new Set(),
         filesDiagnostics: {},
+        namespaceName:
+          namespaceName.kind === "success" ? namespaceName.result : null,
+        rescriptVersion: utils.findReScriptVersion(projectRootPath),
         bsbWatcherByEditor: null,
+        bscBinaryLocation: utils.findBscExeBinary(projectRootPath),
         hasPromptedToStartBuild: /(\/|\\)node_modules(\/|\\)/.test(
           projectRootPath
         )
@@ -811,7 +801,9 @@ function format(msg: p.RequestMessage): Array<p.Message> {
     let code = getOpenedFileContent(params.textDocument.uri);
 
     let projectRootPath = utils.findProjectRootOfFile(filePath);
-    let bscExeBinaryPath = utils.findBscExeBinary(projectRootPath);
+    let project =
+      projectRootPath != null ? projectsFiles.get(projectRootPath) : null;
+    let bscExeBinaryPath = project?.bscBinaryLocation ?? null;
 
     let formattedResult = utils.formatCode(
       bscExeBinaryPath,

diff --git a/server/src/utils.ts b/server/src/utils.ts
@@ -14,6 +14,7 @@ import * as c from "./constants";
 import * as lookup from "./lookup";
 import { reportError } from "./errorReporter";
 import config from "./config";
+import { filesDiagnostics, projectsFiles } from "./projectFiles";
 
 let tempFilePrefix = "rescript_format_file_" + process.pid + "_";
 let tempFileId = 0;
@@ -24,9 +25,7 @@ export let createFileInTempDir = (extension = "") => {
   return path.join(os.tmpdir(), tempFileName);
 };
 
-// TODO: races here?
-// TODO: this doesn't handle file:/// scheme
-export let findProjectRootOfFile = (
+let findProjectRootOfFileInDir = (
   source: p.DocumentUri
 ): null | p.DocumentUri => {
   let dir = path.dirname(source);
@@ -40,11 +39,41 @@ export let findProjectRootOfFile = (
       // reached top
       return null;
     } else {
-      return findProjectRootOfFile(dir);
+      return findProjectRootOfFileInDir(dir);
     }
   }
 };
 
+// TODO: races here?
+// TODO: this doesn't handle file:/// scheme
+export let findProjectRootOfFile = (
+  source: p.DocumentUri
+): null | p.DocumentUri => {
+  // First look in project files
+  let foundRootFromProjectFiles: string | null = null;
+
+  for (const rootPath of projectsFiles.keys()) {
+    if (source.startsWith(rootPath)) {
+      // Prefer the longest path (most nested)
+      if (
+        foundRootFromProjectFiles == null ||
+        rootPath.length > foundRootFromProjectFiles.length
+      ) {
+        foundRootFromProjectFiles = rootPath;
+      }
+    }
+  }
+
+  if (foundRootFromProjectFiles != null) {
+    return foundRootFromProjectFiles;
+  } else {
+    const isDir = path.extname(source) === "";
+    return findProjectRootOfFileInDir(
+      isDir ? path.join(source, "dummy.res") : source
+    );
+  }
+};
+
 // Check if binaryName exists inside binaryDirPath and return the joined path.
 export let findBinary = (
   binaryDirPath: p.DocumentUri | null,
@@ -138,7 +167,9 @@ export let formatCode = (
   }
 };
 
-let findReScriptVersion = (filePath: p.DocumentUri): string | undefined => {
+export let findReScriptVersion = (
+  filePath: p.DocumentUri
+): string | undefined => {
   let projectRoot = findProjectRootOfFile(filePath);
   if (projectRoot == null) {
     return undefined;
@@ -161,25 +192,31 @@ let findReScriptVersion = (filePath: p.DocumentUri): string | undefined => {
   }
 };
 
+let binaryPath: string | null = null;
+if (fs.existsSync(c.analysisDevPath)) {
+  binaryPath = c.analysisDevPath;
+} else if (fs.existsSync(c.analysisProdPath)) {
+  binaryPath = c.analysisProdPath;
+} else {
+}
+
 export let runAnalysisAfterSanityCheck = (
   filePath: p.DocumentUri,
   args: Array<any>,
   projectRequired = false
 ) => {
-  let binaryPath;
-  if (fs.existsSync(c.analysisDevPath)) {
-    binaryPath = c.analysisDevPath;
-  } else if (fs.existsSync(c.analysisProdPath)) {
-    binaryPath = c.analysisProdPath;
-  } else {
+  if (binaryPath == null) {
     return null;
   }
 
   let projectRootPath = findProjectRootOfFile(filePath);
   if (projectRootPath == null && projectRequired) {
     return null;
   }
-  let rescriptVersion = findReScriptVersion(filePath);
+  let rescriptVersion =
+    projectsFiles.get(projectRootPath ?? "")?.rescriptVersion ??
+    findReScriptVersion(filePath);
+
   let options: childProcess.ExecFileSyncOptions = {
     cwd: projectRootPath || undefined,
     maxBuffer: Infinity,
@@ -449,9 +486,6 @@ let parseFileAndRange = (fileAndRange: string) => {
 };
 
 // main parsing logic
-export type filesDiagnostics = {
-  [key: string]: p.Diagnostic[];
-};
 type parsedCompilerLogResult = {
   done: boolean;
   result: filesDiagnostics;