-
Notifications
You must be signed in to change notification settings - Fork 57
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
Reduce language server latency #1003
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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(); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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()) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If I understand correctly here, the most common operation is to find the project info associated with the current open file. I see you make a I think we don't need this loop if we're using an index for it in the same way export const projectRoots = new Map<p.DocumentUri, string>();
export const projects = new Map<string, projectFiles>(); There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We could do that, but this feels like a good trade off to me. That loop is cheap and it'll also be resilient to if roots change for some reason while the file is open (unlikely, but still). |
||
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; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess these 3 are the added cached values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Exactly, and
rescriptVersion
was already cached for incremental compilation, just moved that cache to a more central place.