Skip to content

Commit

Permalink
perf: don't store duplicate info for ops in the snapshot (#27430)
Browse files Browse the repository at this point in the history
Mostly for changes from denoland/deno_core#1010

---------

Co-authored-by: David Sherret <dsherret@gmail.com>
  • Loading branch information
bartlomieju and dsherret authored Dec 20, 2024
1 parent 65b6479 commit c30f345
Show file tree
Hide file tree
Showing 8 changed files with 29 additions and 31 deletions.
12 changes: 6 additions & 6 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 Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ repository = "https://github.com/denoland/deno"

[workspace.dependencies]
deno_ast = { version = "=0.44.0", features = ["transpiling"] }
deno_core = { version = "0.326.0" }
deno_core = { version = "0.327.0" }

deno_bench_util = { version = "0.178.0", path = "./bench_util" }
deno_config = { version = "=0.40.0", features = ["workspace", "sync"] }
Expand Down
4 changes: 2 additions & 2 deletions cli/module_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -996,7 +996,7 @@ impl<TGraphContainer: ModuleGraphContainer> ModuleLoader
std::future::ready(()).boxed_local()
}

fn get_source_map(&self, file_name: &str) -> Option<Vec<u8>> {
fn get_source_map(&self, file_name: &str) -> Option<Cow<[u8]>> {
let specifier = resolve_url(file_name).ok()?;
match specifier.scheme() {
// we should only be looking for emits for schemes that denote external
Expand All @@ -1008,7 +1008,7 @@ impl<TGraphContainer: ModuleGraphContainer> ModuleLoader
.0
.load_prepared_module_for_source_map_sync(&specifier)
.ok()??;
source_map_from_code(source.code.as_bytes())
source_map_from_code(source.code.as_bytes()).map(Cow::Owned)
}

fn get_source_mapped_source_line(
Expand Down
2 changes: 1 addition & 1 deletion cli/standalone/binary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -746,7 +746,7 @@ impl<'a> DenoCompileBinaryWriter<'a> {
for (specifier, source_map) in source_maps {
source_map_store.add(
Cow::Owned(root_dir_url.specifier_key(specifier).into_owned()),
Cow::Owned(source_map),
Cow::Owned(source_map.into_bytes()),
);
}

Expand Down
5 changes: 2 additions & 3 deletions cli/standalone/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -502,7 +502,7 @@ impl ModuleLoader for EmbeddedModuleLoader {
std::future::ready(()).boxed_local()
}

fn get_source_map(&self, file_name: &str) -> Option<Vec<u8>> {
fn get_source_map(&self, file_name: &str) -> Option<Cow<[u8]>> {
if file_name.starts_with("file:///") {
let url =
deno_path_util::url_from_directory_path(self.shared.vfs.root()).ok()?;
Expand All @@ -512,8 +512,7 @@ impl ModuleLoader for EmbeddedModuleLoader {
} else {
self.shared.source_maps.get(file_name)
}
// todo(https://github.com/denoland/deno_core/pull/1007): don't clone
.map(|s| s.as_bytes().to_vec())
.map(Cow::Borrowed)
}

fn get_source_mapped_source_line(
Expand Down
19 changes: 9 additions & 10 deletions cli/standalone/serialization.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ pub fn serialize_binary_data_section(
builder.append_le(specifier.len() as u32);
builder.append(specifier);
builder.append_le(source_map.len() as u32);
builder.append(source_map);
builder.append(source_map.as_ref());
}
}

Expand Down Expand Up @@ -124,9 +124,9 @@ pub fn deserialize_binary_data_section(
#[allow(clippy::type_complexity)]
fn read_source_map_entry(
input: &[u8],
) -> Result<(&[u8], (Cow<str>, Cow<str>)), AnyError> {
) -> Result<(&[u8], (Cow<str>, &[u8])), AnyError> {
let (input, specifier) = read_string_lossy(input)?;
let (input, source_map) = read_string_lossy(input)?;
let (input, source_map) = read_bytes_with_u32_len(input)?;
Ok((input, (specifier, source_map)))
}

Expand Down Expand Up @@ -164,7 +164,7 @@ pub fn deserialize_binary_data_section(
let (current_input, (specifier, source_map)) =
read_source_map_entry(input)?;
input = current_input;
source_maps.add(specifier, source_map);
source_maps.add(specifier, Cow::Borrowed(source_map));
}

// finally ensure we read the magic bytes at the end
Expand Down Expand Up @@ -293,7 +293,7 @@ impl DenoCompileModuleSource {
}

pub struct SourceMapStore {
data: IndexMap<Cow<'static, str>, Cow<'static, str>>,
data: IndexMap<Cow<'static, str>, Cow<'static, [u8]>>,
}

impl SourceMapStore {
Expand All @@ -306,13 +306,13 @@ impl SourceMapStore {
pub fn add(
&mut self,
specifier: Cow<'static, str>,
source_map: Cow<'static, str>,
source_map: Cow<'static, [u8]>,
) {
self.data.insert(specifier, source_map);
}

pub fn get(&self, specifier: &str) -> Option<&Cow<'static, str>> {
self.data.get(specifier)
pub fn get(&self, specifier: &str) -> Option<&[u8]> {
self.data.get(specifier).map(|v| v.as_ref())
}
}

Expand Down Expand Up @@ -763,8 +763,7 @@ fn check_has_len(input: &[u8], len: usize) -> Result<(), AnyError> {
}

fn read_string_lossy(input: &[u8]) -> Result<(&[u8], Cow<str>), AnyError> {
let (input, str_len) = read_u32_as_usize(input)?;
let (input, data_bytes) = read_bytes(input, str_len)?;
let (input, data_bytes) = read_bytes_with_u32_len(input)?;
Ok((input, String::from_utf8_lossy(data_bytes)))
}

Expand Down
4 changes: 2 additions & 2 deletions cli/tools/coverage/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ pub struct CoverageReport {
fn generate_coverage_report(
script_coverage: &cdp::ScriptCoverage,
script_source: String,
maybe_source_map: &Option<Vec<u8>>,
maybe_source_map: Option<&[u8]>,
output: &Option<PathBuf>,
) -> CoverageReport {
let maybe_source_map = maybe_source_map
Expand Down Expand Up @@ -625,7 +625,7 @@ pub fn cover_files(
let coverage_report = generate_coverage_report(
&script_coverage,
runtime_code.as_str().to_owned(),
&source_map,
source_map.as_deref(),
&out_mode,
);

Expand Down
12 changes: 6 additions & 6 deletions cli/util/text_encoding.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,31 +140,31 @@ mod tests {
#[test]
fn test_source_map_from_code() {
let to_string =
|bytes: Vec<u8>| -> String { String::from_utf8(bytes).unwrap() };
|bytes: Vec<u8>| -> String { String::from_utf8(bytes.to_vec()).unwrap() };
assert_eq!(
source_map_from_code(
b"test\n//# sourceMappingURL=data:application/json;base64,dGVzdGluZ3Rlc3Rpbmc=",
b"test\n//# sourceMappingURL=data:application/json;base64,dGVzdGluZ3Rlc3Rpbmc="
).map(to_string),
Some("testingtesting".to_string())
);
assert_eq!(
source_map_from_code(
b"test\n//# sourceMappingURL=data:application/json;base64,dGVzdGluZ3Rlc3Rpbmc=\n \n",
b"test\n//# sourceMappingURL=data:application/json;base64,dGVzdGluZ3Rlc3Rpbmc=\n \n"
).map(to_string),
Some("testingtesting".to_string())
);
assert_eq!(
source_map_from_code(
b"test\n//# sourceMappingURL=data:application/json;base64,dGVzdGluZ3Rlc3Rpbmc=\n test\n",
),
b"test\n//# sourceMappingURL=data:application/json;base64,dGVzdGluZ3Rlc3Rpbmc=\n test\n"
).map(to_string),
None
);
assert_eq!(
source_map_from_code(
b"\"use strict\";
throw new Error(\"Hello world!\");
//# sourceMappingURL=data:application/json;base64,{",
//# sourceMappingURL=data:application/json;base64,{"
),
None
);
Expand Down

0 comments on commit c30f345

Please sign in to comment.