From 3508783084de27f3a93cac77dba545d7b29052a9 Mon Sep 17 00:00:00 2001
From: Joyee Cheung <joyeec9h3@gmail.com>
Date: Wed, 4 Sep 2024 20:18:12 +0200
Subject: [PATCH] module: implement flushCompileCache()

This implements an API for users to intentionally flush the
accumulated compile cache instead of waiting until process
shutdown. It may be useful for application that loads dependencies
first and then either reload itself in other instances, or spawning
other instances that load an overlapping set of its dependencies -
in this case its useful to flush the cache early instead of waiting
until the shutdown of itself.

Currently flushing is triggered by either process
shutdown or user requests. In the future we should simply start the
writes right after module loading on a separate thread, and this method
only blocks until all the pending writes (if any) on the other thread
are finished. In that case, the off-thread writes should finish long
before any attempt of flushing is made so the method would then only
incur a negligible overhead from thread synchronization.

PR-URL: https://github.com/nodejs/node/pull/54971
Fixes: https://github.com/nodejs/node/issues/54770
Fixes: https://github.com/nodejs/node/issues/54465
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
---
 doc/api/module.md                             | 23 +++++++++
 lib/internal/modules/helpers.js               |  2 +
 lib/module.js                                 |  3 ++
 src/compile_cache.cc                          |  7 +++
 src/env.cc                                    | 14 +++---
 src/node_modules.cc                           | 21 +++++++++
 test/fixtures/compile-cache-flush.js          | 21 +++++++++
 test/parallel/test-compile-cache-api-flush.js | 47 +++++++++++++++++++
 8 files changed, 130 insertions(+), 8 deletions(-)
 create mode 100644 test/fixtures/compile-cache-flush.js
 create mode 100644 test/parallel/test-compile-cache-api-flush.js

diff --git a/doc/api/module.md b/doc/api/module.md
index 3bb25c3f313da1..665a64c0505b66 100644
--- a/doc/api/module.md
+++ b/doc/api/module.md
@@ -199,6 +199,13 @@ Compilation cache generated by one version of Node.js can not be reused by a dif
 version of Node.js. Cache generated by different versions of Node.js will be stored
 separately if the same base directory is used to persist the cache, so they can co-exist.
 
+At the moment, when the compile cache is enabled and a module is loaded afresh, the
+code cache is generated from the compiled code immediately, but will only be written
+to disk when the Node.js instance is about to exit. This is subject to change. The
+[`module.flushCompileCache()`][] method can be used to ensure the accumulated code cache
+is flushed to disk in case the application wants to spawn other Node.js instances
+and let them share the cache long before the parent exits.
+
 ### `module.getCompileCacheDir()`
 
 <!-- YAML
@@ -1101,6 +1108,21 @@ added:
 `path` is the resolved path for the file for which a corresponding source map
 should be fetched.
 
+### `module.flushCompileCache()`
+
+<!-- YAML
+added:
+ - REPLACEME
+-->
+
+> Stability: 1.1 - Active Development
+
+Flush the [module compile cache][] accumulated from modules already loaded
+in the current Node.js instance to disk. This returns after all the flushing
+file system operations come to an end, no matter they succeed or not. If there
+are any errors, this will fail silently, since compile cache misses should not
+interfer with the actual operation of the application.
+
 ### Class: `module.SourceMap`
 
 <!-- YAML
@@ -1216,6 +1238,7 @@ returned object contains the following keys:
 [`initialize`]: #initialize
 [`module.constants.compileCacheStatus`]: #moduleconstantscompilecachestatus
 [`module.enableCompileCache()`]: #moduleenablecompilecachecachedir
+[`module.flushCompileCache()`]: #moduleflushcompilecache
 [`module.getCompileCacheDir()`]: #modulegetcompilecachedir
 [`module`]: #the-module-object
 [`os.tmpdir()`]: os.md#ostmpdir
diff --git a/lib/internal/modules/helpers.js b/lib/internal/modules/helpers.js
index a31c2a74d88b36..e5cd20ed0369c4 100644
--- a/lib/internal/modules/helpers.js
+++ b/lib/internal/modules/helpers.js
@@ -40,6 +40,7 @@ const {
   enableCompileCache: _enableCompileCache,
   getCompileCacheDir: _getCompileCacheDir,
   compileCacheStatus: _compileCacheStatus,
+  flushCompileCache,
 } = internalBinding('modules');
 
 let debug = require('internal/util/debuglog').debuglog('module', (fn) => {
@@ -485,6 +486,7 @@ module.exports = {
   assertBufferSource,
   constants,
   enableCompileCache,
+  flushCompileCache,
   getBuiltinModule,
   getCjsConditions,
   getCompileCacheDir,
diff --git a/lib/module.js b/lib/module.js
index fe0eb937eaeac0..48ba9215b081eb 100644
--- a/lib/module.js
+++ b/lib/module.js
@@ -7,6 +7,7 @@ const { SourceMap } = require('internal/source_map/source_map');
 const {
   constants,
   enableCompileCache,
+  flushCompileCache,
   getCompileCacheDir,
 } = require('internal/modules/helpers');
 
@@ -15,5 +16,7 @@ Module.register = register;
 Module.SourceMap = SourceMap;
 Module.constants = constants;
 Module.enableCompileCache = enableCompileCache;
+Module.flushCompileCache = flushCompileCache;
+
 Module.getCompileCacheDir = getCompileCacheDir;
 module.exports = Module;
diff --git a/src/compile_cache.cc b/src/compile_cache.cc
index e5262a19d1549c..d4ae86669712b8 100644
--- a/src/compile_cache.cc
+++ b/src/compile_cache.cc
@@ -309,6 +309,13 @@ void CompileCacheHandler::Persist() {
 
   // TODO(joyeecheung): do this using a separate event loop to utilize the
   // libuv thread pool and do the file system operations concurrently.
+  // TODO(joyeecheung): Currently flushing is triggered by either process
+  // shutdown or user requests. In the future we should simply start the
+  // writes right after module loading on a separate thread, and this method
+  // only blocks until all the pending writes (if any) on the other thread are
+  // finished. In that case, the off-thread writes should finish long
+  // before any attempt of flushing is made so the method would then only
+  // incur a negligible overhead from thread synchronization.
   for (auto& pair : compiler_cache_store_) {
     auto* entry = pair.second.get();
     if (entry->cache == nullptr) {
diff --git a/src/env.cc b/src/env.cc
index 3a15a63f878a0d..c7c34be24b62c1 100644
--- a/src/env.cc
+++ b/src/env.cc
@@ -847,14 +847,12 @@ Environment::Environment(IsolateData* isolate_data,
     }
   }
 
-  // We are supposed to call builtin_loader_.SetEagerCompile() in
-  // snapshot mode here because it's beneficial to compile built-ins
-  // loaded in the snapshot eagerly and include the code of inner functions
-  // that are likely to be used by user since they are part of the core
-  // startup. But this requires us to start the coverage collections
-  // before Environment/Context creation which is not currently possible.
-  // TODO(joyeecheung): refactor V8ProfilerConnection classes to parse
-  // JSON without v8 and lift this restriction.
+  // Compile builtins eagerly when building the snapshot so that inner functions
+  // of essential builtins that are loaded in the snapshot can have faster first
+  // invocation.
+  if (isolate_data->is_building_snapshot()) {
+    builtin_loader()->SetEagerCompile();
+  }
 
   // We'll be creating new objects so make sure we've entered the context.
   HandleScope handle_scope(isolate);
diff --git a/src/node_modules.cc b/src/node_modules.cc
index 721b5e1e4b457d..821eaa08741c11 100644
--- a/src/node_modules.cc
+++ b/src/node_modules.cc
@@ -435,6 +435,25 @@ void BindingData::GetPackageScopeConfig(
           .ToLocalChecked());
 }
 
+void FlushCompileCache(const FunctionCallbackInfo<Value>& args) {
+  Isolate* isolate = args.GetIsolate();
+  Local<Context> context = isolate->GetCurrentContext();
+  Environment* env = Environment::GetCurrent(context);
+
+  if (!args[0]->IsBoolean() && !args[0]->IsUndefined()) {
+    THROW_ERR_INVALID_ARG_TYPE(env,
+                               "keepDeserializedCache should be a boolean");
+    return;
+  }
+  Debug(env,
+        DebugCategory::COMPILE_CACHE,
+        "[compile cache] module.flushCompileCache() requested.\n");
+  env->FlushCompileCache();
+  Debug(env,
+        DebugCategory::COMPILE_CACHE,
+        "[compile cache] module.flushCompileCache() finished.\n");
+}
+
 void EnableCompileCache(const FunctionCallbackInfo<Value>& args) {
   Isolate* isolate = args.GetIsolate();
   Local<Context> context = isolate->GetCurrentContext();
@@ -480,6 +499,7 @@ void BindingData::CreatePerIsolateProperties(IsolateData* isolate_data,
   SetMethod(isolate, target, "getPackageScopeConfig", GetPackageScopeConfig);
   SetMethod(isolate, target, "enableCompileCache", EnableCompileCache);
   SetMethod(isolate, target, "getCompileCacheDir", GetCompileCacheDir);
+  SetMethod(isolate, target, "flushCompileCache", FlushCompileCache);
 }
 
 void BindingData::CreatePerContextProperties(Local<Object> target,
@@ -512,6 +532,7 @@ void BindingData::RegisterExternalReferences(
   registry->Register(GetPackageScopeConfig);
   registry->Register(EnableCompileCache);
   registry->Register(GetCompileCacheDir);
+  registry->Register(FlushCompileCache);
 }
 
 }  // namespace modules
diff --git a/test/fixtures/compile-cache-flush.js b/test/fixtures/compile-cache-flush.js
new file mode 100644
index 00000000000000..8054f67b0ddaea
--- /dev/null
+++ b/test/fixtures/compile-cache-flush.js
@@ -0,0 +1,21 @@
+'use strict';
+
+const { flushCompileCache, getCompileCacheDir } = require('module');
+const { spawnSync } = require('child_process');
+const assert = require('assert');
+
+if (process.argv[2] !== 'child') {
+  // The test should be run with the compile cache already enabled and NODE_DEBUG_NATIVE=COMPILE_CACHE.
+  assert(getCompileCacheDir());
+  assert(process.env.NODE_DEBUG_NATIVE.includes('COMPILE_CACHE'));
+
+  flushCompileCache();
+
+  const child1 = spawnSync(process.execPath, [__filename, 'child']);
+  console.log(child1.stderr.toString().trim().split('\n').map(line => `[child1]${line}`).join('\n'));
+
+  flushCompileCache();
+
+  const child2 = spawnSync(process.execPath, [__filename, 'child']);
+  console.log(child2.stderr.toString().trim().split('\n').map(line => `[child2]${line}`).join('\n'));
+}
diff --git a/test/parallel/test-compile-cache-api-flush.js b/test/parallel/test-compile-cache-api-flush.js
new file mode 100644
index 00000000000000..7e66b1fee103db
--- /dev/null
+++ b/test/parallel/test-compile-cache-api-flush.js
@@ -0,0 +1,47 @@
+'use strict';
+
+// This tests module.flushCompileCache() works as expected.
+
+require('../common');
+const { spawnSyncAndAssert } = require('../common/child_process');
+const assert = require('assert');
+const tmpdir = require('../common/tmpdir');
+const fixtures = require('../common/fixtures');
+
+{
+  // Test that it works with non-existent directory.
+  tmpdir.refresh();
+  const cacheDir = tmpdir.resolve('compile_cache');
+  spawnSyncAndAssert(
+    process.execPath,
+    [fixtures.path('compile-cache-flush.js')],
+    {
+      env: {
+        ...process.env,
+        NODE_DEBUG_NATIVE: 'COMPILE_CACHE',
+        NODE_COMPILE_CACHE: cacheDir,
+      },
+      cwd: tmpdir.path
+    },
+    {
+      stdout(output) {
+        // This contains output from the nested spawnings of compile-cache-flush.js.
+        assert.match(output, /child1.* cache for .*compile-cache-flush\.js was accepted, keeping the in-memory entry/);
+        assert.match(output, /child2.* cache for .*compile-cache-flush\.js was accepted, keeping the in-memory entry/);
+        return true;
+      },
+      stderr(output) {
+        // This contains output from the top-level spawning of compile-cache-flush.js.
+        assert.match(output, /reading cache from .*compile_cache.* for CommonJS .*compile-cache-flush\.js/);
+        assert.match(output, /compile-cache-flush\.js was not initialized, initializing the in-memory entry/);
+
+        const writeRE = /writing cache for .*compile-cache-flush\.js.*success/;
+        const flushRE = /module\.flushCompileCache\(\) finished/;
+        assert.match(output, writeRE);
+        assert.match(output, flushRE);
+        // The cache writing should happen before flushing finishes i.e. it's not delayed until process shutdown.
+        assert(output.match(writeRE).index < output.match(flushRE).index);
+        return true;
+      }
+    });
+}