Skip to content

refactor(core): use Box<u8> for ModuleSource.code instead of a String #14487

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

Merged
merged 4 commits into from
May 5, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions cli/proc_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,7 @@ impl ProcState {
}
};
Ok(ModuleSource {
code,
code: code.as_bytes().to_vec().into_boxed_slice(),
module_url_specified: specifier.to_string(),
module_url_found: found.to_string(),
module_type: match media_type {
Expand Down Expand Up @@ -646,7 +646,8 @@ impl SourceMapGetter for ProcState {
let code = String::from_utf8(code).unwrap();
source_map_from_code(code).or(maybe_map)
} else if let Ok(source) = self.load(specifier, None, false) {
source_map_from_code(source.code)
let code = String::from_utf8(source.code.to_vec()).unwrap();
source_map_from_code(code)
} else {
None
}
Expand Down
4 changes: 2 additions & 2 deletions cli/standalone.rs
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ impl ModuleLoader for EmbeddedModuleLoader {
async move {
if let Some((ref source, _)) = is_data_uri {
return Ok(deno_core::ModuleSource {
code: source.to_owned(),
code: source.as_bytes().to_vec().into_boxed_slice(),
module_type: deno_core::ModuleType::JavaScript,
module_url_specified: module_specifier.to_string(),
module_url_found: module_specifier.to_string(),
Expand All @@ -184,7 +184,7 @@ impl ModuleLoader for EmbeddedModuleLoader {
.to_owned();

Ok(deno_core::ModuleSource {
code,
code: code.as_bytes().to_vec().into_boxed_slice(),
module_type: match module.kind {
eszip::ModuleKind::JavaScript => deno_core::ModuleType::JavaScript,
eszip::ModuleKind::Json => deno_core::ModuleType::Json,
Expand Down
2 changes: 1 addition & 1 deletion core/examples/ts_module_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl ModuleLoader for TypescriptModuleLoader {
code
};
let module = ModuleSource {
code,
code: code.as_bytes().to_vec().into_boxed_slice(),
module_type,
module_url_specified: module_specifier.to_string(),
module_url_found: module_specifier.to_string(),
Expand Down
55 changes: 34 additions & 21 deletions core/modules.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@ use std::task::Poll;
pub type ModuleId = i32;
pub(crate) type ModuleLoadId = i32;

pub const BOM_CHAR: char = '\u{FEFF}';
pub const BOM_CHAR: &[u8] = &[0xef, 0xbb, 0xbf];

/// Strips the byte order mark from the provided text if it exists.
fn strip_bom(text: &str) -> &str {
if text.starts_with(BOM_CHAR) {
&text[BOM_CHAR.len_utf8()..]
fn strip_bom(source_code: &[u8]) -> &[u8] {
if source_code.starts_with(BOM_CHAR) {
&source_code[BOM_CHAR.len()..]
} else {
text
source_code
}
}

Expand Down Expand Up @@ -190,7 +190,7 @@ impl std::fmt::Display for ModuleType {
// intermediate redirects from file loader.
#[derive(Debug, Clone, Eq, PartialEq)]
pub struct ModuleSource {
pub code: String,
pub code: Box<[u8]>,
pub module_type: ModuleType,
pub module_url_specified: String,
pub module_url_found: String,
Expand Down Expand Up @@ -315,9 +315,9 @@ impl ModuleLoader for FsModuleLoader {
ModuleType::JavaScript
};

let code = std::fs::read_to_string(path)?;
let code = std::fs::read(path)?;
let module = ModuleSource {
code,
code: code.into_boxed_slice(),
module_type,
module_url_specified: module_specifier.to_string(),
module_url_found: module_specifier.to_string(),
Expand Down Expand Up @@ -781,10 +781,15 @@ impl ModuleMap {
&mut self,
scope: &mut v8::HandleScope,
name: &str,
source: &str,
source: &[u8],
) -> Result<ModuleId, ModuleError> {
let name_str = v8::String::new(scope, name).unwrap();
let source_str = v8::String::new(scope, strip_bom(source)).unwrap();
let source_str = v8::String::new_from_utf8(
scope,
strip_bom(source),
v8::NewStringType::Normal,
)
.unwrap();

let tc_scope = &mut v8::TryCatch::new(scope);

Expand Down Expand Up @@ -822,10 +827,12 @@ impl ModuleMap {
scope: &mut v8::HandleScope,
main: bool,
name: &str,
source: &str,
source: &[u8],
) -> Result<ModuleId, ModuleError> {
let name_str = v8::String::new(scope, name).unwrap();
let source_str = v8::String::new(scope, source).unwrap();
let source_str =
v8::String::new_from_utf8(scope, source, v8::NewStringType::Normal)
.unwrap();

let origin = bindings::module_origin(scope, name_str);
let source = v8::script_compiler::Source::new(source_str, Some(&origin));
Expand Down Expand Up @@ -1258,7 +1265,7 @@ import "/a.js";
}
match mock_source_code(&inner.url) {
Some(src) => Poll::Ready(Ok(ModuleSource {
code: src.0.to_owned(),
code: src.0.as_bytes().to_vec().into_boxed_slice(),
module_type: ModuleType::JavaScript,
module_url_specified: inner.url.clone(),
module_url_found: src.1.to_owned(),
Expand Down Expand Up @@ -1454,7 +1461,7 @@ import "/a.js";
scope,
true,
&specifier_a,
r#"
br#"
import { b } from './b.js'
if (b() != 'b') throw Error();
let control = 42;
Expand All @@ -1478,7 +1485,7 @@ import "/a.js";
scope,
false,
"file:///b.js",
"export function b() { return 'b' }",
b"export function b() { return 'b' }",
)
.unwrap();
let imports = module_map.get_requested_modules(mod_b).unwrap();
Expand Down Expand Up @@ -1561,7 +1568,7 @@ import "/a.js";
scope,
true,
&specifier_a,
r#"
br#"
import jsonData from './b.json' assert {type: "json"};
assert(jsonData.a == "b");
assert(jsonData.c.d == 10);
Expand All @@ -1582,7 +1589,7 @@ import "/a.js";
.new_json_module(
scope,
"file:///b.json",
"{\"a\": \"b\", \"c\": {\"d\": 10}}",
b"{\"a\": \"b\", \"c\": {\"d\": 10}}",
)
.unwrap();
let imports = module_map.get_requested_modules(mod_b).unwrap();
Expand Down Expand Up @@ -1692,7 +1699,9 @@ import "/a.js";
let info = ModuleSource {
module_url_specified: specifier.to_string(),
module_url_found: specifier.to_string(),
code: "export function b() { return 'b' }".to_owned(),
code: b"export function b() { return 'b' }"
.to_vec()
.into_boxed_slice(),
module_type: ModuleType::JavaScript,
};
async move { Ok(info) }.boxed()
Expand Down Expand Up @@ -1836,7 +1845,7 @@ import "/a.js";
let info = ModuleSource {
module_url_specified: specifier.to_string(),
module_url_found: specifier.to_string(),
code: code.to_owned(),
code: code.as_bytes().to_vec().into_boxed_slice(),
module_type: ModuleType::JavaScript,
};
async move { Ok(info) }.boxed()
Expand Down Expand Up @@ -2184,13 +2193,17 @@ if (import.meta.url != 'file:///main_with_code.js') throw Error();
"file:///main_module.js" => Ok(ModuleSource {
module_url_specified: "file:///main_module.js".to_string(),
module_url_found: "file:///main_module.js".to_string(),
code: "if (!import.meta.main) throw Error();".to_owned(),
code: b"if (!import.meta.main) throw Error();"
.to_vec()
.into_boxed_slice(),
module_type: ModuleType::JavaScript,
}),
"file:///side_module.js" => Ok(ModuleSource {
module_url_specified: "file:///side_module.js".to_string(),
module_url_found: "file:///side_module.js".to_string(),
code: "if (import.meta.main) throw Error();".to_owned(),
code: b"if (import.meta.main) throw Error();"
.to_vec()
.into_boxed_slice(),
module_type: ModuleType::JavaScript,
}),
_ => unreachable!(),
Expand Down
6 changes: 3 additions & 3 deletions core/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1554,7 +1554,7 @@ impl JsRuntime {
// main module
true,
specifier.as_str(),
&code,
code.as_bytes(),
)
.map_err(|e| match e {
ModuleError::Exception(exception) => {
Expand Down Expand Up @@ -1613,7 +1613,7 @@ impl JsRuntime {
// not main module
false,
specifier.as_str(),
&code,
code.as_bytes(),
)
.map_err(|e| match e {
ModuleError::Exception(exception) => {
Expand Down Expand Up @@ -3022,7 +3022,7 @@ assertEquals(1, notify_return_value);
) -> Pin<Box<ModuleSourceFuture>> {
async move {
Ok(ModuleSource {
code: "console.log('hello world');".to_string(),
code: b"console.log('hello world');".to_vec().into_boxed_slice(),
module_url_specified: "file:///main.js".to_string(),
module_url_found: "file:///main.js".to_string(),
module_type: ModuleType::JavaScript,
Expand Down