Skip to content

Commit

Permalink
refactor(core): cleanup feature flags for js source inclusion (denola…
Browse files Browse the repository at this point in the history
…nd#19463)

Remove `ExtensionFileSourceCode::LoadedFromFsDuringSnapshot` and feature
`include_js_for_snapshotting` since they leak paths that are only
applicable in this repo to embedders. Replace with feature
`exclude_js_sources`. Additionally the feature
`force_include_js_sources` allows negating it, if both features are set.
We need both of these because features are additive and there must be a
way of force including sources for snapshot creation while still having
the `exclude_js_sources` feature. `force_include_js_sources` is only set
for build deps, so sources are still excluded from the final binary.

You can also specify `force_include_js_sources` on any extension to
override the above features for that extension. Towards denoland#19398.

But there was still the snapshot-from-snapshot situation where code
could be executed twice, I addressed that by making `mod_evaluate()` and
scripts like `core/01_core.js` behave idempotently. This allowed
unifying `ext::init_ops()` and `ext::init_ops_and_esm()` into
`ext::init()`.
  • Loading branch information
nayeemrmn committed Jun 13, 2023
1 parent 5348778 commit ceb03cf
Show file tree
Hide file tree
Showing 33 changed files with 250 additions and 380 deletions.
1 change: 1 addition & 0 deletions Cargo.lock

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

2 changes: 1 addition & 1 deletion bench_util/benches/op_baseline.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ deno_core::extension!(
);

fn setup() -> Vec<Extension> {
vec![bench_setup::init_ops()]
vec![bench_setup::init()]
}

#[op]
Expand Down
5 changes: 1 addition & 4 deletions bench_util/benches/utf8.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,19 @@ use deno_bench_util::bencher::Bencher;
use deno_bench_util::BenchOptions;
use deno_core::Extension;
use deno_core::ExtensionFileSource;
use deno_core::ExtensionFileSourceCode;

fn setup() -> Vec<Extension> {
vec![Extension::builder("bench_setup")
.js(vec![ExtensionFileSource {
specifier: "ext:bench_setup/setup.js",
code: ExtensionFileSourceCode::IncludedInBinary(
r#"
code: r#"
const hello = "hello world\n";
const hello1k = hello.repeat(1e3);
const hello1m = hello.repeat(1e6);
const helloEncoded = Deno.core.encode(hello);
const hello1kEncoded = Deno.core.encode(hello1k);
const hello1mEncoded = Deno.core.encode(hello1m);
"#,
),
}])
.build()]
}
Expand Down
8 changes: 4 additions & 4 deletions cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ harness = false
path = "./bench/lsp_bench_standalone.rs"

[build-dependencies]
deno_runtime = { workspace = true, features = ["snapshot_from_snapshot", "include_js_files_for_snapshotting"] }
deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] }
deno_runtime = { workspace = true, features = ["exclude_js_main_from_snapshot"] }
deno_core.workspace = true
lazy-regex.workspace = true
serde.workspace = true
serde_json.workspace = true
Expand All @@ -41,14 +41,14 @@ winres.workspace = true

[dependencies]
deno_ast = { workspace = true, features = ["bundler", "cjs", "codegen", "dep_graph", "module_specifier", "proposal", "react", "sourcemap", "transforms", "typescript", "view", "visit"] }
deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] }
deno_core = { workspace = true, features = ["exclude_js_sources"] }
deno_doc = "=0.63.1"
deno_emit = "=0.24.0"
deno_graph = "=0.49.0"
deno_lint = { version = "=0.47.0", features = ["docs"] }
deno_lockfile.workspace = true
deno_npm.workspace = true
deno_runtime = { workspace = true, features = ["dont_create_runtime_snapshot", "include_js_files_for_snapshotting"] }
deno_runtime = { workspace = true, features = ["dont_create_runtime_snapshot"] }
deno_semver.workspace = true
deno_task_shell = "=0.12.0"
eszip = "=0.44.0"
Expand Down
64 changes: 25 additions & 39 deletions cli/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@ use std::sync::Arc;
use deno_core::snapshot_util::*;
use deno_core::Extension;
use deno_core::ExtensionFileSource;
use deno_core::ExtensionFileSourceCode;
use deno_runtime::deno_cache::SqliteBackedCache;
use deno_runtime::deno_http::DefaultHttpPropertyExtractor;
use deno_runtime::deno_kv::sqlite::SqliteDbHandler;
Expand Down Expand Up @@ -262,15 +261,11 @@ mod ts {
)
.unwrap();

let output = create_snapshot(CreateSnapshotOptions {
create_snapshot(CreateSnapshotOptions {
cargo_manifest_dir: env!("CARGO_MANIFEST_DIR"),
snapshot_path,
startup_snapshot: None,
extensions: vec![deno_tsc::init_ops_and_esm(
op_crate_libs,
build_libs,
path_dts,
)],
extensions: vec![deno_tsc::init(op_crate_libs, build_libs, path_dts)],

// NOTE(bartlomieju): Compressing the TSC snapshot in debug build took
// ~45s on M1 MacBook Pro; without compression it took ~1s.
Expand All @@ -289,9 +284,6 @@ mod ts {
})),
snapshot_module_load_cb: None,
});
for path in output.files_loaded_during_snapshot {
println!("cargo:rerun-if-changed={}", path.display());
}
}

pub(crate) fn version() -> String {
Expand Down Expand Up @@ -322,55 +314,52 @@ deno_core::extension!(
customizer = |ext: &mut deno_core::ExtensionBuilder| {
ext.esm(vec![ExtensionFileSource {
specifier: "ext:cli/runtime/js/99_main.js",
code: ExtensionFileSourceCode::LoadedFromFsDuringSnapshot(
std::path::PathBuf::from(deno_runtime::js::PATH_FOR_99_MAIN_JS),
),
code: include_str!("../runtime/js/99_main.js"),
}]);
}
);

#[must_use = "The files listed by create_cli_snapshot should be printed as 'cargo:rerun-if-changed' lines"]
fn create_cli_snapshot(snapshot_path: PathBuf) -> CreateSnapshotOutput {
fn create_cli_snapshot(snapshot_path: PathBuf) {
// NOTE(bartlomieju): ordering is important here, keep it in sync with
// `runtime/worker.rs`, `runtime/web_worker.rs` and `runtime/build.rs`!
let fs = Arc::new(deno_fs::RealFs);
let extensions: Vec<Extension> = vec![
deno_webidl::deno_webidl::init_ops(),
deno_console::deno_console::init_ops(),
deno_url::deno_url::init_ops(),
deno_web::deno_web::init_ops::<PermissionsContainer>(
deno_webidl::deno_webidl::init(),
deno_console::deno_console::init(),
deno_url::deno_url::init(),
deno_web::deno_web::init::<PermissionsContainer>(
deno_web::BlobStore::default(),
Default::default(),
),
deno_fetch::deno_fetch::init_ops::<PermissionsContainer>(Default::default()),
deno_cache::deno_cache::init_ops::<SqliteBackedCache>(None),
deno_websocket::deno_websocket::init_ops::<PermissionsContainer>(
deno_fetch::deno_fetch::init::<PermissionsContainer>(Default::default()),
deno_cache::deno_cache::init::<SqliteBackedCache>(None),
deno_websocket::deno_websocket::init::<PermissionsContainer>(
"".to_owned(),
None,
None,
),
deno_webstorage::deno_webstorage::init_ops(None),
deno_crypto::deno_crypto::init_ops(None),
deno_broadcast_channel::deno_broadcast_channel::init_ops(
deno_webstorage::deno_webstorage::init(None),
deno_crypto::deno_crypto::init(None),
deno_broadcast_channel::deno_broadcast_channel::init(
deno_broadcast_channel::InMemoryBroadcastChannel::default(),
false, // No --unstable.
),
deno_ffi::deno_ffi::init_ops::<PermissionsContainer>(false),
deno_net::deno_net::init_ops::<PermissionsContainer>(
deno_ffi::deno_ffi::init::<PermissionsContainer>(false),
deno_net::deno_net::init::<PermissionsContainer>(
None, false, // No --unstable.
None,
),
deno_tls::deno_tls::init_ops(),
deno_kv::deno_kv::init_ops(
deno_tls::deno_tls::init(),
deno_kv::deno_kv::init(
SqliteDbHandler::<PermissionsContainer>::new(None),
false, // No --unstable.
),
deno_napi::deno_napi::init_ops::<PermissionsContainer>(),
deno_http::deno_http::init_ops::<DefaultHttpPropertyExtractor>(),
deno_io::deno_io::init_ops(Default::default()),
deno_fs::deno_fs::init_ops::<PermissionsContainer>(false, fs.clone()),
deno_node::deno_node::init_ops::<PermissionsContainer>(None, fs),
cli::init_ops_and_esm(), // NOTE: This needs to be init_ops_and_esm!
deno_napi::deno_napi::init::<PermissionsContainer>(),
deno_http::deno_http::init::<DefaultHttpPropertyExtractor>(),
deno_io::deno_io::init(Default::default()),
deno_fs::deno_fs::init::<PermissionsContainer>(false, fs.clone()),
deno_node::deno_node::init::<PermissionsContainer>(None, fs),
cli::init(),
];

create_snapshot(CreateSnapshotOptions {
Expand Down Expand Up @@ -485,10 +474,7 @@ fn main() {
ts::create_compiler_snapshot(compiler_snapshot_path, &c);

let cli_snapshot_path = o.join("CLI_SNAPSHOT.bin");
let output = create_cli_snapshot(cli_snapshot_path);
for path in output.files_loaded_during_snapshot {
println!("cargo:rerun-if-changed={}", path.display())
}
create_cli_snapshot(cli_snapshot_path);

#[cfg(target_os = "windows")]
{
Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/tsc.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3236,7 +3236,7 @@ fn op_script_version(
/// server.
fn js_runtime(performance: Arc<Performance>) -> JsRuntime {
JsRuntime::new(RuntimeOptions {
extensions: vec![deno_tsc::init_ops(performance)],
extensions: vec![deno_tsc::init(performance)],
startup_snapshot: Some(tsc::compiler_snapshot()),
..Default::default()
})
Expand Down
2 changes: 1 addition & 1 deletion cli/ops/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ pub mod bench;
pub mod testing;

pub fn cli_exts(npm_resolver: Arc<CliNpmResolver>) -> Vec<Extension> {
vec![deno_cli::init_ops(npm_resolver)]
vec![deno_cli::init(npm_resolver)]
}

deno_core::extension!(deno_cli,
Expand Down
2 changes: 1 addition & 1 deletion cli/tools/bench.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ async fn bench_specifier(
.create_custom_worker(
specifier.clone(),
PermissionsContainer::new(permissions),
vec![ops::bench::deno_bench::init_ops(sender.clone())],
vec![ops::bench::deno_bench::init(sender.clone())],
Default::default(),
)
.await?;
Expand Down
2 changes: 1 addition & 1 deletion cli/tools/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -935,7 +935,7 @@ pub async fn test_specifier(
.create_custom_worker(
specifier.clone(),
PermissionsContainer::new(permissions),
vec![ops::testing::deno_test::init_ops(sender.clone())],
vec![ops::testing::deno_test::init(sender.clone())],
Stdio {
stdin: StdioPipe::Inherit,
stdout,
Expand Down
4 changes: 2 additions & 2 deletions cli/tsc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,7 @@ fn get_asset_texts_from_new_runtime() -> Result<Vec<AssetText>, AnyError> {
// the assets are stored within the typescript isolate, so take them out of there
let mut runtime = JsRuntime::new(RuntimeOptions {
startup_snapshot: Some(compiler_snapshot()),
extensions: vec![deno_cli_tsc::init_ops()],
extensions: vec![deno_cli_tsc::init()],
..Default::default()
});
let global = runtime
Expand Down Expand Up @@ -787,7 +787,7 @@ pub fn exec(request: Request) -> Result<Response, AnyError> {

let mut runtime = JsRuntime::new(RuntimeOptions {
startup_snapshot: Some(compiler_snapshot()),
extensions: vec![deno_cli_tsc::init_ops(
extensions: vec![deno_cli_tsc::init(
request,
root_map,
remapped_specifiers,
Expand Down
3 changes: 3 additions & 0 deletions core/00_primordials.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,9 @@
"use strict";

(() => {
if (globalThis.__bootstrap) {
return;
}
const primordials = {};

const {
Expand Down
3 changes: 3 additions & 0 deletions core/01_core.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
"use strict";

((window) => {
if (globalThis.__bootstrap.core) {
return;
}
const {
Array,
ArrayPrototypeFill,
Expand Down
3 changes: 3 additions & 0 deletions core/02_error.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
"use strict";

((window) => {
if (globalThis.__bootstrap.core.prepareStackTrace) {
return;
}
const core = Deno.core;
const ops = core.ops;
const {
Expand Down
6 changes: 5 additions & 1 deletion core/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,10 @@ path = "lib.rs"
[features]
default = ["v8_use_custom_libcxx"]
v8_use_custom_libcxx = ["v8/use_custom_libcxx"]
include_js_files_for_snapshotting = []
# Enable to exclude JS sources from the binary (e.g. if they are already snapshotted).
exclude_js_sources = []
# Enable to override `exclude_js_sources` (e.g. for snapshot creation in `build.rs`).
force_include_js_sources = ["exclude_js_sources"]

[dependencies]
anyhow.workspace = true
Expand Down Expand Up @@ -47,3 +50,4 @@ path = "examples/http_bench_json_ops/main.rs"
# These dependencies are only used for the 'http_bench_*_ops' examples.
[dev-dependencies]
deno_ast.workspace = true
deno_core = { workspace = true, features = ["force_include_js_sources"] }
Loading

0 comments on commit ceb03cf

Please sign in to comment.