src: cache missing package.json files in the C++ package config cache

Author: michaelsmithxyz

lib/internal/modules/package_json_reader.js

@@ -6,9 +6,7 @@ const {
   ObjectDefineProperty,
   RegExpPrototypeExec,
   SafeMap,
-  StringPrototypeEndsWith,
   StringPrototypeIndexOf,
-  StringPrototypeLastIndexOf,
   StringPrototypeSlice,
 } = primordials;
 const {
@@ -28,11 +26,9 @@ const {
 const { kEmptyObject } = require('internal/util');
 const modulesBinding = internalBinding('modules');
 const path = require('path');
-const permission = require('internal/process/permission');
 const { validateString } = require('internal/validators');
 const internalFsBinding = internalBinding('fs');
 
-const nearestParentPackageJSONCache = new SafeMap();
 
 /**
  * @typedef {import('typings/internalBinding/modules').DeserializedPackageConfig} DeserializedPackageConfig
@@ -68,28 +64,41 @@ function deserializePackageJSON(path, contents) {
 
   const pjsonPath = optionalFilePath ?? path;
 
-  return {
-    data: {
+  const data = {
+    __proto__: null,
+    ...(name != null && { name }),
+    ...(main != null && { main }),
+    ...(type != null && { type }),
+  };
+
+  if (plainExports !== null) {
+    ObjectDefineProperty(data, 'exports', {
+      __proto__: null,
+      configurable: true,
+      enumerable: true,
+      get() {
+        const value = requiresJSONParse(plainExports) ? JSONParse(plainExports) : plainExports;
+        ObjectDefineProperty(data, 'exports', { __proto__: null, enumerable: true, value });
+        return value;
+      },
+    });
+  }
+
+  if (plainImports !== null) {
+    ObjectDefineProperty(data, 'imports', {
       __proto__: null,
-      ...(name != null && { name }),
-      ...(main != null && { main }),
-      ...(type != null && { type }),
-      ...(plainImports != null && {
-        // This getters are used to lazily parse the imports and exports fields.
-        get imports() {
-          const value = requiresJSONParse(plainImports) ? JSONParse(plainImports) : plainImports;
-          ObjectDefineProperty(this, 'imports', { __proto__: null, value });
-          return this.imports;
-        },
-      }),
-      ...(plainExports != null && {
-        get exports() {
-          const value = requiresJSONParse(plainExports) ? JSONParse(plainExports) : plainExports;
-          ObjectDefineProperty(this, 'exports', { __proto__: null, value });
-          return this.exports;
-        },
-      }),
-    },
+      configurable: true,
+      enumerable: true,
+      get() {
+        const value = requiresJSONParse(plainImports) ? JSONParse(plainImports) : plainImports;
+        ObjectDefineProperty(data, 'imports', { __proto__: null, enumerable: true, value });
+        return value;
+      },
+    });
+  }
+
+  return {
+    data,
     exists: true,
     path: pjsonPath,
   };
@@ -131,43 +140,23 @@ function read(jsonPath, { base, specifier, isESM } = kEmptyObject) {
 }
 
 /**
- * Given a file path, walk the filesystem upwards until we find its closest parent
- * `package.json` file, stopping when:
- * 1. we find a `package.json` file;
- * 2. we find a path that we do not have permission to read;
- * 3. we find a containing `node_modules` directory;
- * 4. or, we reach the filesystem root
- * @returns {undefined | string}
+ * A cache mapping a module's path to its parent `package.json` file's path.
+ * This is used in concert with `deserializedPackageJSONCache` to improve
+ * the performance of `getNearestParentPackageJSON` when called repeatedly
+ * on the same module paths.
  */
-function findParentPackageJSON(checkPath) {
-  const enabledPermission = permission.isEnabled();
-
-  const rootSeparatorIndex = StringPrototypeIndexOf(checkPath, path.sep);
-  let separatorIndex;
-
-  do {
-    separatorIndex = StringPrototypeLastIndexOf(checkPath, path.sep);
-    checkPath = StringPrototypeSlice(checkPath, 0, separatorIndex);
+const moduleToParentPackageJSONCache = new SafeMap();
 
-    if (enabledPermission && !permission.has('fs.read', checkPath + path.sep)) {
-      return undefined;
-    }
-
-    if (StringPrototypeEndsWith(checkPath, path.sep + 'node_modules')) {
-      return undefined;
-    }
-
-    const maybePackageJSONPath = checkPath + path.sep + 'package.json';
-    const stat = internalFsBinding.internalModuleStat(checkPath + path.sep + 'package.json');
-
-    const packageJSONExists = stat === 0;
-    if (packageJSONExists) {
-      return maybePackageJSONPath;
-    }
-  } while (separatorIndex > rootSeparatorIndex);
-
-  return undefined;
-}
+/**
+ * A cache mapping the path of a `package.json` file to its
+ * {@link DeserializedPackageConfig deserialized representation},
+ * as produced by {@link deserializedPackageJSONCache}. The purpose of this
+ * cache is to ensure that we always return the same
+ * {@link DeserializedPackageConfig} instance for a given `package.json`,
+ * which is necessary to ensure that we don't re-parse `imports` and
+ * `exports` redundantly.
+ */
+const deserializedPackageJSONCache = new SafeMap();
 
 /**
  * Get the nearest parent package.json file from a given path.
@@ -176,26 +165,22 @@ function findParentPackageJSON(checkPath) {
  * @returns {undefined | DeserializedPackageConfig}
  */
 function getNearestParentPackageJSON(checkPath) {
-  const nearestParentPackageJSON = findParentPackageJSON(checkPath);
-
-  if (nearestParentPackageJSON === undefined) {
-    return undefined;
+  const parentPackageJSONPath = moduleToParentPackageJSONCache.get(checkPath);
+  if (parentPackageJSONPath !== undefined) {
+    return deserializedPackageJSONCache.get(parentPackageJSONPath);
   }
 
-  if (nearestParentPackageJSONCache.has(nearestParentPackageJSON)) {
-    return nearestParentPackageJSONCache.get(nearestParentPackageJSON);
-  }
+  const result = modulesBinding.getNearestParentPackageJSON(checkPath);
+  const packageConfig = deserializePackageJSON(checkPath, result);
 
-  const result = modulesBinding.readPackageJSON(nearestParentPackageJSON);
+  moduleToParentPackageJSONCache.set(checkPath, packageConfig.path);
 
-  if (result === undefined) {
-    nearestParentPackageJSONCache.set(checkPath, undefined);
-    return undefined;
+  const maybeCachedPackageConfig = deserializedPackageJSONCache.get(packageConfig.path);
+  if (maybeCachedPackageConfig !== undefined) {
+    return maybeCachedPackageConfig;
   }
 
-  const packageConfig = deserializePackageJSON(checkPath, result);
-  nearestParentPackageJSONCache.set(nearestParentPackageJSON, packageConfig);
-
+  deserializedPackageJSONCache.set(packageConfig.path, packageConfig);
   return packageConfig;
 }
 

src/node_modules.cc

@@ -98,15 +98,27 @@ const BindingData::PackageConfig* BindingData::GetPackageJSON(
 
   auto cache_entry = binding_data->package_configs_.find(path.data());
   if (cache_entry != binding_data->package_configs_.end()) {
-    return &cache_entry->second;
+    auto& cache_value = cache_entry->second;
+    if (cache_value) {
+      return &*cache_value;
+    }
+
+    // If we have a cache entry without a value, we've already
+    // attempted to open and read this path and couldn't (it most
+    // likely doesn't exist)
+    return nullptr;
   }
 
   PackageConfig package_config{};
   package_config.file_path = path;
   // No need to exclude BOM since simdjson will skip it.
   if (ReadFileSync(&package_config.raw_json, path.data()) < 0) {
+    // Add `nullopt` to the package config cache so that we don't
+    // need to open and attempt to read this path again
+    binding_data->package_configs_.insert({std::string(path), std::nullopt});
     return nullptr;
   }
+
   simdjson::ondemand::document document;
   simdjson::ondemand::object main_object;
   simdjson::error_code error =
@@ -239,7 +251,7 @@ const BindingData::PackageConfig* BindingData::GetPackageJSON(
   auto cached = binding_data->package_configs_.insert(
       {std::string(path), std::move(package_config)});
 
-  return &cached.first->second;
+  return &*cached.first->second;
 }
 
 void BindingData::ReadPackageJSON(const FunctionCallbackInfo<Value>& args) {
@@ -322,6 +334,40 @@ const BindingData::PackageConfig* BindingData::TraverseParent(
   return nullptr;
 }
 
+void BindingData::GetNearestParentPackageJSON(
+    const v8::FunctionCallbackInfo<v8::Value>& args) {
+  CHECK_GE(args.Length(), 1);
+  CHECK(args[0]->IsString());
+
+  Realm* realm = Realm::GetCurrent(args);
+  BufferValue path_value(realm->isolate(), args[0]);
+  // Check if the path has a trailing slash. If so, add it after
+  // ToNamespacedPath() as it will be deleted by ToNamespacedPath()
+  bool slashCheck = path_value.ToStringView().ends_with(kPathSeparator);
+
+  ToNamespacedPath(realm->env(), &path_value);
+
+  std::string path_value_str = path_value.ToString();
+  if (slashCheck) {
+    path_value_str.push_back(kPathSeparator);
+  }
+
+  std::filesystem::path path;
+
+#ifdef _WIN32
+  std::wstring wide_path = ConvertToWideString(path_value_str, GetACP());
+  path = std::filesystem::path(wide_path);
+#else
+  path = std::filesystem::path(path_value_str);
+#endif
+
+  auto package_json = TraverseParent(realm, path);
+
+  if (package_json != nullptr) {
+    args.GetReturnValue().Set(package_json->Serialize(realm));
+  }
+}
+
 void BindingData::GetNearestParentPackageJSONType(
     const FunctionCallbackInfo<Value>& args) {
   CHECK_GE(args.Length(), 1);
@@ -640,6 +686,10 @@ void BindingData::CreatePerIsolateProperties(IsolateData* isolate_data,
             target,
             "getNearestParentPackageJSONType",
             GetNearestParentPackageJSONType);
+  SetMethod(isolate,
+            target,
+            "getNearestParentPackageJSON",
+            GetNearestParentPackageJSON);
   SetMethod(
       isolate, target, "getPackageScopeConfig", GetPackageScopeConfig<false>);
   SetMethod(isolate, target, "getPackageType", GetPackageScopeConfig<true>);
@@ -696,6 +746,7 @@ void BindingData::RegisterExternalReferences(
     ExternalReferenceRegistry* registry) {
   registry->Register(ReadPackageJSON);
   registry->Register(GetNearestParentPackageJSONType);
+  registry->Register(GetNearestParentPackageJSON);
   registry->Register(GetPackageScopeConfig<false>);
   registry->Register(GetPackageScopeConfig<true>);
   registry->Register(EnableCompileCache);

src/node_modules.h

@@ -55,6 +55,8 @@ class BindingData : public SnapshotableObject {
   SET_MEMORY_INFO_NAME(BindingData)
 
   static void ReadPackageJSON(const v8::FunctionCallbackInfo<v8::Value>& args);
+  static void GetNearestParentPackageJSON(
+      const v8::FunctionCallbackInfo<v8::Value>& args);
   static void GetNearestParentPackageJSONType(
       const v8::FunctionCallbackInfo<v8::Value>& args);
   template <bool return_only_type>
@@ -72,7 +74,14 @@ class BindingData : public SnapshotableObject {
   static void RegisterExternalReferences(ExternalReferenceRegistry* registry);
 
  private:
-  std::unordered_map<std::string, PackageConfig> package_configs_;
+  /*
+   * This map caches `PackageConfig` values by `package.json` path.
+   * An empty optional value indicates that no `package.json` file
+   * at the given path exists, which we cache to avoid repeated
+   * attempts to open the same non-existent paths.
+   */
+  std::unordered_map<std::string, std::optional<PackageConfig> >
+      package_configs_;
   simdjson::ondemand::parser json_parser;
   // returns null on error
   static const PackageConfig* GetPackageJSON(

typings/internalBinding/modules.d.ts

@@ -23,6 +23,7 @@ export type SerializedPackageConfig = [
 export interface ModulesBinding {
   readPackageJSON(path: string): SerializedPackageConfig | undefined;
   getNearestParentPackageJSONType(path: string): PackageConfig['type']
+  getNearestParentPackageJSON(path: string): SerializedPackageConfig | undefined
   getPackageScopeConfig(path: string): SerializedPackageConfig | undefined
   getPackageType(path: string): PackageConfig['type'] | undefined
   enableCompileCache(path?: string): { status: number, message?: string, directory?: string }