Use externalized strings for built-in ts/js modules

cloudflare · Dec 9, 2022 · a423774 · a423774
1 parent ee08680
commit a423774
Show file tree

Hide file tree

Showing 2 changed files with 45 additions and 7 deletions.
diff --git a/src/workerd/jsg/modules.c++ b/src/workerd/jsg/modules.c++
@@ -274,7 +274,8 @@ namespace {
 v8::Local<v8::Module> compileEsmModule(
     jsg::Lock& js,
     kj::StringPtr name,
-    kj::ArrayPtr<const char> content) {
+    kj::ArrayPtr<const char> content,
+    ModuleInfoCompileFlags flags) {
   // Must pass true for `is_module`, but we can skip everything else.
   const int resourceLineOffset = 0;
   const int resourceColumnOffset = 0;
@@ -289,7 +290,16 @@ v8::Local<v8::Module> compileEsmModule(
                           resourceColumnOffset,
                           resourceIsSharedCrossOrigin, scriptId, {},
                           resourceIsOpaque, isWasm, isModule);
-  v8::ScriptCompiler::Source source(jsg::v8Str(js.v8Isolate, content), origin);
+  v8::Local<v8::String> contentStr;
+  if ((flags & ModuleInfoCompileFlags::EXTERNAL) == ModuleInfoCompileFlags::EXTERNAL) {
+    // TODO(later): Use of newExternalOneByteString here limits our built-in source
+    // modules (for which this path is used) to only the latin1 character set. We
+    // may need to revisit that to import built-ins as UTF-16 (two-byte).
+    contentStr = jsg::check(jsg::newExternalOneByteString(js, content));
+  } else {
+    contentStr = jsg::v8Str(js.v8Isolate, content);
+  }
+  v8::ScriptCompiler::Source source(contentStr, origin);
   return jsg::check(v8::ScriptCompiler::CompileModule(js.v8Isolate, &source));
 }
 
@@ -322,8 +332,9 @@ ModuleRegistry::ModuleInfo::ModuleInfo(
 ModuleRegistry::ModuleInfo::ModuleInfo(
     jsg::Lock& js,
     kj::StringPtr name,
-    kj::ArrayPtr<const char> content)
-    : ModuleInfo(js, compileEsmModule(js, name, content)) {}
+    kj::ArrayPtr<const char> content,
+    CompileFlags flags)
+    : ModuleInfo(js, compileEsmModule(js, name, content, flags)) {}
 
 ModuleRegistry::ModuleInfo::ModuleInfo(
     jsg::Lock& js,

diff --git a/src/workerd/jsg/modules.h b/src/workerd/jsg/modules.h
@@ -175,7 +175,21 @@ class ModuleRegistry {
                v8::Local<v8::Module> module,
                kj::Maybe<SyntheticModuleInfo> maybeSynthetic = nullptr);
 
-    ModuleInfo(jsg::Lock& js, kj::StringPtr name, kj::ArrayPtr<const char> content);
+    enum class CompileFlags {
+      NONE,
+      EXTERNAL,
+      // The EXTERNAL flag tells the compile operation to treat the content as an external
+      // string wrapping an immutable buffer outside the V8 heap. When this flag is not set,
+      // the content is copied into the V8 heap.
+      // TODO(cleanup): Once we have a more complete set of options here for options that
+      // are used for all built-in modules, we'll likely collapse the flags into a single
+      // BUILTIN flag that encompasses multiple options.
+    };
+
+    ModuleInfo(jsg::Lock& js,
+               kj::StringPtr name,
+               kj::ArrayPtr<const char> content,
+               CompileFlags flags = CompileFlags::NONE);
 
     ModuleInfo(jsg::Lock& js, kj::StringPtr name,
                kj::Maybe<kj::ArrayPtr<kj::StringPtr>> maybeExports,
@@ -211,6 +225,19 @@ class ModuleRegistry {
   virtual void setDynamicImportCallback(kj::Function<DynamicImportCallback> func) = 0;
 };
 
+using ModuleInfoCompileFlags = ModuleRegistry::ModuleInfo::CompileFlags;
+
+inline constexpr ModuleInfoCompileFlags operator|(
+    ModuleInfoCompileFlags a,
+    ModuleInfoCompileFlags b) {
+  return static_cast<ModuleInfoCompileFlags>(static_cast<uint>(a) | static_cast<uint>(b));
+}
+inline constexpr ModuleInfoCompileFlags operator&(
+    ModuleInfoCompileFlags a,
+    ModuleInfoCompileFlags b) {
+  return static_cast<ModuleInfoCompileFlags>(static_cast<uint>(a) & static_cast<uint>(b));
+}
+
 template <typename TypeWrapper>
 class ModuleRegistryImpl final: public ModuleRegistry {
 public:
@@ -381,7 +408,7 @@ class ModuleRegistryImpl final: public ModuleRegistry {
           type(type),
           info(kj::mv(info)) {}
 
-    Entry(const kj::Path& specifier, Type type, kj::ArrayPtr<const char> src)
+    Entry(const kj::Path& specifier,Type type, kj::ArrayPtr<const char> src)
         : specifier(specifier.clone()),
           type(type),
           info(src) {}
@@ -402,7 +429,7 @@ class ModuleRegistryImpl final: public ModuleRegistry {
           return moduleInfo;
         }
         KJ_CASE_ONEOF(src, kj::ArrayPtr<const char>) {
-          info = ModuleInfo(js, specifier.toString(), src);
+          info = ModuleInfo(js, specifier.toString(), src, ModuleInfoCompileFlags::EXTERNAL);
           return KJ_ASSERT_NONNULL(info.tryGet<ModuleInfo>());
         }
         KJ_CASE_ONEOF(src, kj::Function<ModuleInfo(Lock&)>) {