Skip to content

Commit

Permalink
refactor(lsp): make TS host use CLI snapshot (#28062)
Browse files Browse the repository at this point in the history
This commit changes the TS host implementation
in the LSP to use the same snapshot as the runtime worker
and web worker use.

This is due to upcoming V8 upgrade that might require
that all isolates in the same process use the exact same
snapshot.
  • Loading branch information
bartlomieju authored Feb 11, 2025
1 parent c1a0d63 commit 795ecfd
Show file tree
Hide file tree
Showing 11 changed files with 349 additions and 314 deletions.
2 changes: 1 addition & 1 deletion cli/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1194,7 +1194,7 @@ impl CliFactory {
serve_port: cli_options.serve_port(),
serve_host: cli_options.serve_host(),
otel_config: self.cli_options()?.otel_config(),
startup_snapshot: crate::js::deno_isolate_init(),
startup_snapshot: deno_snapshots::CLI_SNAPSHOT,
})
}

Expand Down
8 changes: 0 additions & 8 deletions cli/js.rs

This file was deleted.

18 changes: 9 additions & 9 deletions cli/lsp/tsc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4912,17 +4912,17 @@ fn run_tsc_thread(
maybe_inspector_server: Option<Arc<InspectorServer>>,
) {
let has_inspector_server = maybe_inspector_server.is_some();
// Create and setup a JsRuntime based on a snapshot. It is expected that the
// supplied snapshot is an isolate that contains the TypeScript language
// server.
let mut extensions =
deno_runtime::snapshot_info::get_extensions_in_snapshot();
extensions.push(deno_tsc::init_ops_and_esm(
performance,
specifier_map,
request_rx,
));
let mut tsc_runtime = JsRuntime::new(RuntimeOptions {
extensions: vec![deno_tsc::init_ops_and_esm(
performance,
specifier_map,
request_rx,
)],
extensions,
create_params: create_isolate_create_params(),
startup_snapshot: None,
startup_snapshot: deno_snapshots::CLI_SNAPSHOT,
inspector: has_inspector_server,
..Default::default()
});
Expand Down
1 change: 0 additions & 1 deletion cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ mod file_fetcher;
mod graph_container;
mod graph_util;
mod http_util;
mod js;
mod jsr;
mod lsp;
mod module_loader;
Expand Down
55 changes: 9 additions & 46 deletions cli/tsc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1410,13 +1410,17 @@ pub fn exec(request: Request) -> Result<Response, ExecError> {
});
let exec_source = format!("globalThis.exec({request_value})");

let mut extensions =
deno_runtime::snapshot_info::get_extensions_in_snapshot();
extensions.push(deno_cli_tsc::init_ops_and_esm(
request,
root_map,
remapped_specifiers,
));
let mut runtime = JsRuntime::new(RuntimeOptions {
extensions: vec![deno_cli_tsc::init_ops_and_esm(
request,
root_map,
remapped_specifiers,
)],
extensions,
create_params: create_isolate_create_params(),
startup_snapshot: deno_snapshots::CLI_SNAPSHOT,
..Default::default()
});

Expand Down Expand Up @@ -1562,47 +1566,6 @@ mod tests {
exec(request)
}

// TODO(bartlomieju): this test is segfaulting in V8, saying that there are too
// few external references registered. It seems to be a bug in our snapshotting
// logic. Because when we create TSC snapshot we register a few ops that
// are called during snapshotting time, V8 expects at least as many references
// when it starts up. The thing is that these ops are one-off - ie. they will never
// be used again after the snapshot is taken. We should figure out a mechanism
// to allow removing some of the ops before taking a snapshot.
#[ignore]
#[tokio::test]
async fn test_compiler_snapshot() {
let mut js_runtime = JsRuntime::new(RuntimeOptions {
startup_snapshot: None,
extensions: vec![super::deno_cli_tsc::init_ops_and_esm(
Request {
check_mode: TypeCheckMode::All,
config: Arc::new(TsConfig(json!({}))),
debug: false,
graph: Arc::new(ModuleGraph::new(GraphKind::TypesOnly)),
hash_data: 0,
maybe_npm: None,
maybe_tsbuildinfo: None,
root_names: vec![],
},
HashMap::new(),
HashMap::new(),
)],
..Default::default()
});
js_runtime
.execute_script(
"<anon>",
r#"
if (!(globalThis.exec)) {
throw Error("bad");
}
console.log(`ts version: ${ts.version}`);
"#,
)
.unwrap();
}

#[tokio::test]
async fn test_create_hash() {
let mut state = setup(None, Some(123), None).await;
Expand Down
2 changes: 1 addition & 1 deletion cli/worker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -486,7 +486,7 @@ mod tests {
RuntimePermissionDescriptorParser::new(crate::sys::CliSys::default()),
);
let options = WorkerOptions {
startup_snapshot: crate::js::deno_isolate_init(),
startup_snapshot: deno_snapshots::CLI_SNAPSHOT,
..Default::default()
};

Expand Down
1 change: 1 addition & 0 deletions runtime/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ pub mod ops;
pub mod permissions;
#[cfg(feature = "snapshot")]
pub mod snapshot;
pub mod snapshot_info;
pub mod tokio_util;
#[cfg(feature = "transpile")]
pub mod transpile;
Expand Down
Loading

0 comments on commit 795ecfd

Please sign in to comment.