Repository: denoland/deno_core
Pull request #431 (2024-01-09 23:37:12)

feat: Add ModuleSourceBytes enum
This commit changes `ModuleCode` to `ModuleCodeString` and
adds `ModuleCodeBytes` enum that allows to efficiently store
byte source code in various forms.

bartlomieju: Ref https://github.com/denoland/deno_core/pull/402

Patch:
diff --git a/core/extensions.rs b/core/extensions.rs
index 535a4fd..fd07107 100644
--- a/core/extensions.rs
+++ b/core/extensions.rs
@@ -1,5 +1,5 @@
 // Copyright 2018-2023 the Deno authors. All rights reserved. MIT license.
-use crate::modules::ModuleCode;
+use crate::modules::ModuleCodeString;
 use crate::OpState;
 use anyhow::Context as _;
 use anyhow::Error;
@@ -36,7 +36,7 @@ impl ExtensionFileSource {
     s.chars().filter(|c| !c.is_ascii()).collect::<String>()
   }
 
-  pub fn load(&self) -> Result<ModuleCode, Error> {
+  pub fn load(&self) -> Result<ModuleCodeString, Error> {
     match &self.code {
       ExtensionFileSourceCode::IncludedInBinary(code) => {
         debug_assert!(
@@ -45,7 +45,7 @@ impl ExtensionFileSource {
           self.specifier,
           Self::find_non_ascii(code)
         );
-        Ok(ModuleCode::from_static(code))
+        Ok(ModuleCodeString::from_static(code))
       }
       ExtensionFileSourceCode::LoadedFromFsDuringSnapshot(path) => {
         let msg = || format!("Failed to read \"{}\"", path);
@@ -65,7 +65,7 @@ impl ExtensionFileSource {
           self.specifier,
           Self::find_non_ascii(code)
         );
-        Ok(ModuleCode::Arc(code.clone()))
+        Ok(ModuleCodeString::Arc(code.clone()))
       }
     }
   }
diff --git a/core/lib.rs b/core/lib.rs
index cda4f05..924d119 100644
--- a/core/lib.rs
+++ b/core/lib.rs
@@ -92,7 +92,8 @@ pub use crate::module_specifier::ModuleResolutionError;
 pub use crate::module_specifier::ModuleSpecifier;
 pub use crate::modules::ExtModuleLoaderCb;
 pub use crate::modules::FsModuleLoader;
-pub use crate::modules::ModuleCode;
+pub use crate::modules::ModuleCodeBytes;
+pub use crate::modules::ModuleCodeString;
 pub use crate::modules::ModuleId;
 pub use crate::modules::ModuleLoader;
 pub use crate::modules::ModuleSource;
diff --git a/core/modules/loaders.rs b/core/modules/loaders.rs
index 916cb45..a74385e 100644
--- a/core/modules/loaders.rs
+++ b/core/modules/loaders.rs
@@ -3,7 +3,7 @@ use crate::error::generic_error;
 use crate::error::AnyError;
 use crate::extensions::ExtensionFileSource;
 use crate::module_specifier::ModuleSpecifier;
-use crate::modules::ModuleCode;
+use crate::modules::ModuleCodeString;
 use crate::modules::ModuleSource;
 use crate::modules::ModuleSourceFuture;
 use crate::modules::ModuleType;
@@ -106,7 +106,7 @@ impl ModuleLoader for NoopModuleLoader {
 /// Function that can be passed to the `ExtModuleLoader` that allows to
 /// transpile sources before passing to V8.
 pub type ExtModuleLoaderCb =
-  Box<dyn Fn(&ExtensionFileSource) -> Result<ModuleCode, Error>>;
+  Box<dyn Fn(&ExtensionFileSource) -> Result<ModuleCodeString, Error>>;
 
 pub(crate) struct ExtModuleLoader {
   sources: RefCell<HashMap<String, ExtensionFileSource>>,
@@ -308,7 +308,7 @@ impl ModuleLoader for FsModuleLoader {
       })?;
       let module = ModuleSource::new(
         module_type,
-        ModuleSourceCode::Bytes(code),
+        ModuleSourceCode::Bytes(code.into_boxed_slice().into()),
         module_specifier,
       );
       Ok(module)
@@ -321,13 +321,13 @@ impl ModuleLoader for FsModuleLoader {
 /// A module loader that you can pre-load a number of modules into and resolve from. Useful for testing and
 /// embedding situations where the filesystem and snapshot systems are not usable or a good fit.
 pub struct StaticModuleLoader {
-  map: HashMap<ModuleSpecifier, ModuleCode>,
+  map: HashMap<ModuleSpecifier, ModuleCodeString>,
 }
 
 impl StaticModuleLoader {
   /// Create a new [`StaticModuleLoader`] from an `Iterator` of specifiers and code.
   pub fn new(
-    from: impl IntoIterator<Item = (ModuleSpecifier, ModuleCode)>,
+    from: impl IntoIterator<Item = (ModuleSpecifier, ModuleCodeString)>,
   ) -> Self {
     Self {
       map: HashMap::from_iter(
@@ -339,7 +339,7 @@ impl StaticModuleLoader {
   }
 
   /// Create a new [`StaticModuleLoader`] from a single code item.
-  pub fn with(specifier: ModuleSpecifier, code: ModuleCode) -> Self {
+  pub fn with(specifier: ModuleSpecifier, code: ModuleCodeString) -> Self {
     Self::new([(specifier, code)])
   }
 }
diff --git a/core/modules/map.rs b/core/modules/map.rs
index 07ce45f..842d74c 100644
--- a/core/modules/map.rs
+++ b/core/modules/map.rs
@@ -9,7 +9,7 @@ use crate::modules::get_requested_module_type_from_attributes;
 use crate::modules::parse_import_attributes;
 use crate::modules::recursive_load::RecursiveModuleLoad;
 use crate::modules::ImportAttributesKind;
-use crate::modules::ModuleCode;
+use crate::modules::ModuleCodeString;
 use crate::modules::ModuleError;
 use crate::modules::ModuleId;
 use crate::modules::ModuleLoadId;
@@ -347,7 +347,7 @@ impl ModuleMap {
     scope: &mut v8::HandleScope,
     main: bool,
     name: ModuleName,
-    source: ModuleCode,
+    source: ModuleCodeString,
     is_dynamic_import: bool,
   ) -> Result<ModuleId, ModuleError> {
     let name_str = name.v8(scope);
@@ -450,7 +450,7 @@ impl ModuleMap {
     &self,
     scope: &mut v8::HandleScope,
     name: ModuleName,
-    code: ModuleCode,
+    code: ModuleCodeString,
   ) -> Result<ModuleId, ModuleError> {
     let source_str = v8::String::new_from_utf8(
       scope,
@@ -1303,7 +1303,7 @@ impl ModuleMap {
     &self,
     scope: &mut v8::HandleScope,
     module_specifier: &str,
-    source_code: ModuleCode,
+    source_code: ModuleCodeString,
   ) -> Result<v8::Global<v8::Value>, Error> {
     let specifier = ModuleSpecifier::parse(module_specifier)?;
     let mod_id = self
diff --git a/core/modules/mod.rs b/core/modules/mod.rs
index 1a6447f..2e0124c 100644
--- a/core/modules/mod.rs
+++ b/core/modules/mod.rs
@@ -10,6 +10,7 @@ use serde::Serialize;
 use std::borrow::Cow;
 use std::collections::HashMap;
 use std::future::Future;
+use std::sync::Arc;
 
 mod loaders;
 mod map;
@@ -42,15 +43,62 @@ pub(crate) type ModuleLoadId = i32;
 /// converted to a string or not.
 #[derive(Debug)]
 pub enum ModuleSourceCode {
-  String(ModuleCode),
-  Bytes(Vec<u8>),
+  String(ModuleCodeString),
+  Bytes(ModuleCodeBytes),
 }
 
-// TODO(bartlomieju): maybe remove it? It might be confusing between `ModuleSourceCode`
-// and `ModuleCode`.
-pub type ModuleCode = FastString;
+pub type ModuleCodeString = FastString;
 pub type ModuleName = FastString;
 
+#[derive(Debug)]
+pub enum ModuleCodeBytes {
+  /// Created from static data.
+  Static(&'static [u8]),
+
+  /// An owned chunk of data. Note that we use `Box` rather than `Vec` to avoid
+  /// the storage overhead.
+  Boxed(Box<[u8]>),
+
+  /// Code loaded from the `deno_graph` infrastructure.
+  Arc(Arc<[u8]>),
+}
+
+impl ModuleCodeBytes {
+  pub fn as_bytes(&self) -> &[u8] {
+    match self {
+      ModuleCodeBytes::Static(s) => s,
+      ModuleCodeBytes::Boxed(s) => s,
+      ModuleCodeBytes::Arc(s) => s,
+    }
+  }
+
+  pub fn to_vec(&self) -> Vec<u8> {
+    match self {
+      ModuleCodeBytes::Static(s) => s.to_vec(),
+      ModuleCodeBytes::Boxed(s) => s.to_vec(),
+      ModuleCodeBytes::Arc(s) => s.to_vec(),
+    }
+  }
+}
+
+impl From<Arc<[u8]>> for ModuleCodeBytes {
+  fn from(value: Arc<[u8]>) -> Self {
+    Self::Arc(value)
+  }
+}
+
+impl From<Box<[u8]>> for ModuleCodeBytes {
+  fn from(value: Box<[u8]>) -> Self {
+    Self::Boxed(value)
+  }
+}
+
+impl From<&'static [u8]> for ModuleCodeBytes {
+  fn from(value: &'static [u8]) -> Self {
+    Self::Static(value)
+  }
+}
+
 /// Callback to customize value of `import.meta.resolve("./foo.ts")`.
 pub type ImportMetaResolveCallback = Box<
   dyn Fn(&dyn ModuleLoader, String, String) -> Result<ModuleSpecifier, Error>,
@@ -275,14 +323,14 @@ impl ModuleSource {
   pub fn get_string_source(
     specifier: &str,
     code: ModuleSourceCode,
-  ) -> Result<ModuleCode, AnyError> {
+  ) -> Result<ModuleCodeString, AnyError> {
     match code {
       ModuleSourceCode::String(code) => Ok(code),
       ModuleSourceCode::Bytes(bytes) => {
-        let str_ = String::from_utf8(bytes).with_context(|| {
+        let str_ = String::from_utf8(bytes.to_vec()).with_context(|| {
           format!("Can't convert source code to string for {}", specifier)
         })?;
-        Ok(ModuleCode::from(str_))
+        Ok(ModuleCodeString::from(str_))
       }
     }
   }
diff --git a/core/runtime/jsrealm.rs b/core/runtime/jsrealm.rs
index dcf3f3e..752799d 100644
--- a/core/runtime/jsrealm.rs
+++ b/core/runtime/jsrealm.rs
@@ -4,7 +4,7 @@ use super::exception_state::ExceptionState;
 use super::op_driver::OpDriver;
 use crate::error::exception_to_err_result;
 use crate::module_specifier::ModuleSpecifier;
-use crate::modules::ModuleCode;
+use crate::modules::ModuleCodeString;
 use crate::modules::ModuleError;
 use crate::modules::ModuleId;
 use crate::modules::ModuleMap;
@@ -227,7 +227,7 @@ impl JsRealm {
 
   fn string_from_code<'a>(
     scope: &mut HandleScope<'a>,
-    code: &ModuleCode,
+    code: &ModuleCodeString,
   ) -> Option<Local<'a, v8::String>> {
     if let Some(code) = code.try_static_ascii() {
       v8::String::new_external_onebyte_static(scope, code)
@@ -261,7 +261,11 @@ impl JsRealm {
     name: &'static str,
     source_code: &'static str,
   ) -> Result<v8::Global<v8::Value>, Error> {
-    self.execute_script(isolate, name, ModuleCode::from_static(source_code))
+    self.execute_script(
+      isolate,
+      name,
+      ModuleCodeString::from_static(source_code),
+    )
   }
 
   /// Executes traditional JavaScript code (traditional = not ES modules) in the
@@ -282,7 +286,7 @@ impl JsRealm {
     &self,
     isolate: &mut v8::Isolate,
     name: &'static str,
-    source_code: ModuleCode,
+    source_code: ModuleCodeString,
   ) -> Result<v8::Global<v8::Value>, Error> {
     let scope = &mut self.0.handle_scope(isolate);
 
@@ -358,7 +362,7 @@ impl JsRealm {
     &self,
     isolate: &mut v8::Isolate,
     specifier: &ModuleSpecifier,
-    code: Option<ModuleCode>,
+    code: Option<ModuleCodeString>,
   ) -> Result<ModuleId, Error> {
     let module_map_rc = self.0.module_map();
     if let Some(code) = code {
@@ -415,7 +419,7 @@ impl JsRealm {
     &self,
     isolate: &mut v8::Isolate,
     specifier: &ModuleSpecifier,
-    code: Option<ModuleCode>,
+    code: Option<ModuleCodeString>,
   ) -> Result<ModuleId, Error> {
     let module_map_rc = self.0.module_map();
     if let Some(code) = code {
@@ -472,7 +476,7 @@ impl JsRealm {
     &self,
     isolate: &mut v8::Isolate,
     module_specifier: &str,
-    code: ModuleCode,
+    code: ModuleCodeString,
   ) -> Result<v8::Global<v8::Value>, Error> {
     let module_map_rc = self.0.module_map();
     let scope = &mut self.handle_scope(isolate);
diff --git a/core/runtime/jsruntime.rs b/core/runtime/jsruntime.rs
index 66640ac..a7ddac1 100644
--- a/core/runtime/jsruntime.rs
+++ b/core/runtime/jsruntime.rs
@@ -19,7 +19,7 @@ use crate::module_specifier::ModuleSpecifier;
 use crate::modules::default_import_meta_resolve_cb;
 use crate::modules::ExtModuleLoader;
 use crate::modules::ImportMetaResolveCallback;
-use crate::modules::ModuleCode;
+use crate::modules::ModuleCodeString;
 use crate::modules::ModuleId;
 use crate::modules::ModuleLoader;
 use crate::modules::ModuleMap;
@@ -1255,7 +1255,7 @@ impl JsRuntime {
   pub fn execute_script(
     &mut self,
     name: &'static str,
-    source_code: ModuleCode,
+    source_code: ModuleCodeString,
   ) -> Result<v8::Global<v8::Value>, Error> {
     let isolate = &mut self.inner.v8_isolate;
     self
@@ -1287,7 +1287,7 @@ impl JsRuntime {
     self.inner.main_realm.execute_script(
       isolate,
       name,
-      ModuleCode::from_static(source_code),
+      ModuleCodeString::from_static(source_code),
     )
   }
 
@@ -2059,7 +2059,7 @@ impl JsRuntime {
   pub async fn load_main_module(
     &mut self,
     specifier: &ModuleSpecifier,
-    code: Option<ModuleCode>,
+    code: Option<ModuleCodeString>,
   ) -> Result<ModuleId, Error> {
     let isolate = &mut self.inner.v8_isolate;
     self
@@ -2079,7 +2079,7 @@ impl JsRuntime {
   pub async fn load_side_module(
     &mut self,
     specifier: &ModuleSpecifier,
-    code: Option<ModuleCode>,
+    code: Option<ModuleCodeString>,
   ) -> Result<ModuleId, Error> {
     let isolate = &mut self.inner.v8_isolate;
     self
@@ -2099,7 +2099,7 @@ impl JsRuntime {
   pub fn lazy_load_es_module_from_code(
     &mut self,
     specifier: &str,
-    code: ModuleCode,
+    code: ModuleCodeString,
   ) -> Result<v8::Global<v8::Value>, Error> {
     let isolate = &mut self.inner.v8_isolate;
     self


Test patch:
diff --git a/core/modules/tests.rs b/core/modules/tests.rs
index 9418f51..1f609a6 100644
--- a/core/modules/tests.rs
+++ b/core/modules/tests.rs
@@ -13,7 +13,7 @@ use crate::resolve_url;
 use crate::runtime::JsRuntime;
 use crate::runtime::JsRuntimeForSnapshot;
 use crate::FastString;
-use crate::ModuleCode;
+use crate::ModuleCodeString;
 use crate::ModuleSource;
 use crate::ModuleSourceFuture;
 use crate::ModuleSpecifier;
@@ -1216,7 +1216,7 @@ fn dynamic_imports_snapshot() {
 #[test]
 fn import_meta_snapshot() {
   let snapshot = {
-    const MAIN_WITH_CODE_SRC: ModuleCode = ascii_str!(
+    const MAIN_WITH_CODE_SRC: ModuleCodeString = ascii_str!(
       r#"
   if (import.meta.url != 'file:///main_with_code.js') throw Error();
   globalThis.meta = import.meta;
@@ -1344,7 +1344,7 @@ async fn no_duplicate_loads() {
           .unwrap()
       };
       let module_source = ModuleSource {
-        code: ModuleSourceCode::String(ModuleCode::from(source_code)),
+        code: ModuleSourceCode::String(ModuleCodeString::from(source_code)),
         module_type: ModuleType::JavaScript,
         module_url_specified: module_specifier.clone().into(),
         module_url_found: found_specifier.map(|s| s.into()),