bootstrap: include code cache in the embedded snapshot

Since V8 code cache encodes indices to the read-only space it is safer to make sure that the code cache is generated in the same heap used to generate the embdded snapshot. This patch merges the code cache builder into the snapshot builder and makes the code cache part of node::SnapshotData that is deserialized into the native module loader during bootstrap. PR-URL: #43023 Fixes: #31074 Refs: #35711 Reviewed-By: Matteo Collina <matteo.collina@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
nodejs · May 17, 2022 · fbe1478 · fbe1478
1 parent de0b6dc
commit fbe1478
Show file tree

Hide file tree

Showing 19 changed files with 216 additions and 427 deletions.
diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS
@@ -107,10 +107,8 @@
 
 /benchmark/misc/startup.js @nodejs/startup
 /src/node.cc @nodejs/startup
-/src/node_code_cache_stub.cc @nodejs/startup
 /src/node_native_module* @nodejs/startup
 /lib/internal/bootstrap/* @nodejs/startup
-/tools/code_cache/* @nodejs/startup
 /tools/snapshot/* @nodejs/startup
 
 # V8

diff --git a/configure.py b/configure.py
@@ -1249,7 +1249,7 @@ def configure_node(o):
     o['variables']['node_use_node_snapshot'] = b(
       not cross_compiling and not options.shared)
 
-  if options.without_node_code_cache or options.node_builtin_modules_path:
+  if options.without_node_code_cache or options.without_node_snapshot or options.node_builtin_modules_path:
     o['variables']['node_use_node_code_cache'] = 'false'
   else:
     # TODO(refack): fix this when implementing embedded code-cache when cross-compiling.

diff --git a/lib/internal/bootstrap/node.js b/lib/internal/bootstrap/node.js
@@ -270,9 +270,8 @@ const features = {
   tls_sni: hasOpenSSL,
   tls_ocsp: hasOpenSSL,
   tls: hasOpenSSL,
-  // This needs to be dynamic because snapshot is built without code cache.
-  // TODO(joyeecheung): https://github.com/nodejs/node/issues/31074
-  // Make it possible to build snapshot with code cache
+  // This needs to be dynamic because --no-node-snapshot disables the
+  // code cache even if the binary is built with embedded code cache.
   get cached_builtins() {
     return nativeModule.hasCachedBuiltins();
   }

diff --git a/node.gyp b/node.gyp
@@ -55,7 +55,6 @@
       'deps/undici/undici.js',
     ],
     'node_mksnapshot_exec': '<(PRODUCT_DIR)/<(EXECUTABLE_PREFIX)node_mksnapshot<(EXECUTABLE_SUFFIX)',
-    'mkcodecache_exec': '<(PRODUCT_DIR)/<(EXECUTABLE_PREFIX)mkcodecache<(EXECUTABLE_SUFFIX)',
     'conditions': [
       ['GENERATOR == "ninja"', {
         'node_text_start_object_path': 'src/large_pages/node_text_start.node_text_start.o'
@@ -304,32 +303,7 @@
             },
           },
          }],
-        ['node_use_node_code_cache=="true"', {
-          'dependencies': [
-            'mkcodecache',
-          ],
-          'actions': [
-            {
-              'action_name': 'run_mkcodecache',
-              'process_outputs_as_sources': 1,
-              'inputs': [
-                '<(mkcodecache_exec)',
-              ],
-              'outputs': [
-                '<(SHARED_INTERMEDIATE_DIR)/node_code_cache.cc',
-              ],
-              'action': [
-                '<@(_inputs)',
-                '<@(_outputs)',
-              ],
-            },
-          ],
-        }, {
-          'sources': [
-            'src/node_code_cache_stub.cc'
-          ],
-        }],
-        ['node_use_node_snapshot=="true"', {
+         ['node_use_node_snapshot=="true"', {
           'dependencies': [
             'node_mksnapshot',
           ],
@@ -737,7 +711,6 @@
         [ 'node_shared=="true"', {
           'sources': [
             'src/node_snapshot_stub.cc',
-            'src/node_code_cache_stub.cc',
           ]
         }],
         [ 'node_shared=="true" and node_module_version!="" and OS!="win"', {
@@ -747,6 +720,11 @@
               '@rpath/lib<(node_core_target_name).<(shlib_suffix)'
           },
         }],
+        [ 'node_use_node_code_cache=="true"', {
+          'defines': [
+            'NODE_USE_NODE_CODE_CACHE=1',
+          ],
+        }],
         ['node_shared=="true" and OS=="aix"', {
           'product_name': 'node_base',
         }],
@@ -1147,7 +1125,6 @@
       ],
       'sources': [
         'src/node_snapshot_stub.cc',
-        'src/node_code_cache_stub.cc',
         'test/fuzzers/fuzz_url.cc',
       ],
       'conditions': [
@@ -1190,7 +1167,6 @@
       ],
       'sources': [
         'src/node_snapshot_stub.cc',
-        'src/node_code_cache_stub.cc',
         'test/fuzzers/fuzz_env.cc',
       ],
       'conditions': [
@@ -1240,7 +1216,6 @@
 
       'sources': [
         'src/node_snapshot_stub.cc',
-        'src/node_code_cache_stub.cc',
         'test/cctest/node_test_fixture.cc',
         'test/cctest/node_test_fixture.h',
         'test/cctest/test_aliased_buffer.cc',
@@ -1333,7 +1308,6 @@
 
       'sources': [
         'src/node_snapshot_stub.cc',
-        'src/node_code_cache_stub.cc',
         'test/embedding/embedtest.cc',
       ],
 
@@ -1377,68 +1351,6 @@
         }],
       ]
     }, # overlapped-checker
-
-    # TODO(joyeecheung): do not depend on node_lib,
-    # instead create a smaller static library node_lib_base that does
-    # just enough for node_native_module.cc and the cache builder to
-    # compile without compiling the generated code cache C++ file.
-    # So generate_code_cache -> mkcodecache -> node_lib_base,
-    #    node_lib -> node_lib_base & generate_code_cache
-    {
-      'target_name': 'mkcodecache',
-      'type': 'executable',
-
-      'dependencies': [
-        '<(node_lib_target_name)',
-        'deps/histogram/histogram.gyp:histogram',
-        'deps/uvwasi/uvwasi.gyp:uvwasi',
-      ],
-
-      'includes': [
-        'node.gypi'
-      ],
-
-      'include_dirs': [
-        'src',
-        'tools/msvs/genfiles',
-        'deps/v8/include',
-        'deps/cares/include',
-        'deps/uv/include',
-        'deps/uvwasi/include',
-      ],
-
-      'defines': [
-        'NODE_WANT_INTERNALS=1'
-      ],
-      'sources': [
-        'src/node_snapshot_stub.cc',
-        'src/node_code_cache_stub.cc',
-        'tools/code_cache/mkcodecache.cc',
-        'tools/code_cache/cache_builder.cc',
-        'tools/code_cache/cache_builder.h',
-      ],
-
-      'conditions': [
-        [ 'node_use_openssl=="true"', {
-          'defines': [
-            'HAVE_OPENSSL=1',
-          ],
-        }],
-        ['v8_enable_inspector==1', {
-          'defines': [
-            'HAVE_INSPECTOR=1',
-          ],
-        }],
-        ['OS=="win"', {
-          'libraries': [
-            'dbghelp.lib',
-            'PsApi.lib',
-            'winmm.lib',
-            'Ws2_32.lib',
-          ],
-        }],
-      ],
-    }, # mkcodecache
     {
       'target_name': 'node_mksnapshot',
       'type': 'executable',
@@ -1466,7 +1378,6 @@
 
       'sources': [
         'src/node_snapshot_stub.cc',
-        'src/node_code_cache_stub.cc',
         'tools/snapshot/node_mksnapshot.cc',
       ],
 
@@ -1476,6 +1387,11 @@
             'HAVE_OPENSSL=1',
           ],
         }],
+        [ 'node_use_node_code_cache=="true"', {
+          'defines': [
+            'NODE_USE_NODE_CODE_CACHE=1',
+          ],
+        }],
         ['v8_enable_inspector==1', {
           'defines': [
             'HAVE_INSPECTOR=1',

diff --git a/src/env.h b/src/env.h
@@ -34,7 +34,6 @@
 #include "handle_wrap.h"
 #include "node.h"
 #include "node_binding.h"
-#include "node_external_reference.h"
 #include "node_main_instance.h"
 #include "node_native_module.h"
 #include "node_options.h"
@@ -996,6 +995,11 @@ struct SnapshotData {
   // TODO(joyeecheung): there should be a vector of env_info once we snapshot
   // the worker environments.
   EnvSerializeInfo env_info;
+  // A vector of built-in ids and v8::ScriptCompiler::CachedData, this can be
+  // shared across Node.js instances because they are supposed to share the
+  // read only space. We use native_module::CodeCacheInfo because
+  // v8::ScriptCompiler::CachedData is not copyable.
+  std::vector<native_module::CodeCacheInfo> code_cache;
 };
 
 class Environment : public MemoryRetainer {

diff --git a/src/node.cc b/src/node.cc
@@ -979,8 +979,6 @@ int InitializeNodeWithArgs(std::vector<std::string>* argv,
 
 #endif  // defined(NODE_HAVE_I18N_SUPPORT)
 
-  NativeModuleEnv::InitializeCodeCache();
-
   // We should set node_is_initialized here instead of in node::Start,
   // otherwise embedders using node::Init to initialize everything will not be
   // able to set it and native modules will not load for them.
@@ -1174,6 +1172,10 @@ int Start(int argc, char** argv) {
                           : nullptr;
     uv_loop_configure(uv_default_loop(), UV_METRICS_IDLE_TIME);
 
+    if (snapshot_data != nullptr) {
+      native_module::NativeModuleEnv::RefreshCodeCache(
+          snapshot_data->code_cache);
+    }
     NodeMainInstance main_instance(snapshot_data,
                                    uv_default_loop(),
                                    per_process::v8_platform.Platform(),

diff --git a/src/node_code_cache_stub.cc b/src/node_code_cache_stub.cc
diff --git a/src/node_main_instance.cc b/src/node_main_instance.cc
@@ -6,6 +6,7 @@
 #include "debug_utils-inl.h"
 #include "node_external_reference.h"
 #include "node_internals.h"
+#include "node_native_module_env.h"
 #include "node_options-inl.h"
 #include "node_snapshot_builder.h"
 #include "node_snapshotable.h"
@@ -189,6 +190,7 @@ NodeMainInstance::CreateMainEnvironment(int* exit_code) {
 
     CHECK(!context.IsEmpty());
     Context::Scope context_scope(context);
+
     CHECK(InitializeContextRuntime(context).IsJust());
     SetIsolateErrorHandlers(isolate_, {});
     env->InitializeMainContext(context, &(snapshot_data_->env_info));

diff --git a/src/node_native_module.cc b/src/node_native_module.cc
@@ -1,6 +1,7 @@
 #include "node_native_module.h"
-#include "util-inl.h"
 #include "debug_utils-inl.h"
+#include "node_internals.h"
+#include "util-inl.h"
 
 namespace node {
 namespace native_module {
@@ -277,6 +278,11 @@ MaybeLocal<Function> NativeModuleLoader::LookupAndCompile(
                 : ScriptCompiler::kEagerCompile;
   ScriptCompiler::Source script_source(source, origin, cached_data);
 
+  per_process::Debug(DebugCategory::CODE_CACHE,
+                     "Compiling %s %s code cache\n",
+                     id,
+                     has_cache ? "with" : "without");
+
   MaybeLocal<Function> maybe_fun =
       ScriptCompiler::CompileFunctionInContext(context,
                                                &script_source,
@@ -304,17 +310,34 @@ MaybeLocal<Function> NativeModuleLoader::LookupAndCompile(
   *result = (has_cache && !script_source.GetCachedData()->rejected)
                 ? Result::kWithCache
                 : Result::kWithoutCache;
+
+  if (has_cache) {
+    per_process::Debug(DebugCategory::CODE_CACHE,
+                       "Code cache of %s (%s) %s\n",
+                       id,
+                       script_source.GetCachedData()->buffer_policy ==
+                               ScriptCompiler::CachedData::BufferNotOwned
+                           ? "BufferNotOwned"
+                           : "BufferOwned",
+                       script_source.GetCachedData()->rejected ? "is rejected"
+                                                               : "is accepted");
+  }
+
   // Generate new cache for next compilation
   std::unique_ptr<ScriptCompiler::CachedData> new_cached_data(
       ScriptCompiler::CreateCodeCacheForFunction(fun));
   CHECK_NOT_NULL(new_cached_data);
 
   {
     Mutex::ScopedLock lock(code_cache_mutex_);
-    // The old entry should've been erased by now so we can just emplace.
-    // If another thread did the same thing in the meantime, that should not
-    // be an issue.
-    code_cache_.emplace(id, std::move(new_cached_data));
+    const auto it = code_cache_.find(id);
+    // TODO(joyeecheung): it's safer for each thread to have its own
+    // copy of the code cache map.
+    if (it == code_cache_.end()) {
+      code_cache_.emplace(id, std::move(new_cached_data));
+    } else {
+      it->second.reset(new_cached_data.release());
+    }
   }
 
   return scope.Escape(fun);

diff --git a/src/node_native_module.h b/src/node_native_module.h
@@ -15,13 +15,19 @@
 class PerProcessTest;
 
 namespace node {
+class SnapshotBuilder;
 namespace native_module {
 
 using NativeModuleRecordMap = std::map<std::string, UnionBytes>;
 using NativeModuleCacheMap =
     std::unordered_map<std::string,
                        std::unique_ptr<v8::ScriptCompiler::CachedData>>;
 
+struct CodeCacheInfo {
+  std::string id;
+  std::vector<uint8_t> data;
+};
+
 // The native (C++) side of the NativeModule in JS land, which
 // handles compilation and caching of builtin modules (NativeModule)
 // and bootstrappers, whose source are bundled into the binary
@@ -66,6 +72,8 @@ class NODE_EXTERN_PRIVATE NativeModuleLoader {
   bool CannotBeRequired(const char* id);
 
   NativeModuleCacheMap* code_cache();
+  const Mutex& code_cache_mutex() const { return code_cache_mutex_; }
+
   v8::ScriptCompiler::CachedData* GetCodeCache(const char* id) const;
   enum class Result { kWithCache, kWithoutCache };
   v8::MaybeLocal<v8::String> LoadBuiltinModuleSource(v8::Isolate* isolate,