Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit 58abe99

Browse filesBrowse files
michaelsmithxyzaduh95
michaelsmithxyz
authored and
aduh95
committed
src: cache missing package.json files in the C++ package config cache
PR-URL: #60425 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 2a54209 commit 58abe99
Copy full SHA for 58abe99

4 files changed

+125-86Lines changed: 125 additions & 86 deletions

File tree

Expand file treeCollapse file tree
Open diff view settings
Filter options
Expand file treeCollapse file tree
Open diff view settings
Collapse file

‎lib/internal/modules/package_json_reader.js‎

Copy file name to clipboardExpand all lines: lib/internal/modules/package_json_reader.js
+59-74Lines changed: 59 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,7 @@ const {
66
ObjectDefineProperty,
77
RegExpPrototypeExec,
88
SafeMap,
9-
StringPrototypeEndsWith,
109
StringPrototypeIndexOf,
11-
StringPrototypeLastIndexOf,
1210
StringPrototypeSlice,
1311
} = primordials;
1412
const {
@@ -28,11 +26,9 @@ const {
2826
const { kEmptyObject } = require('internal/util');
2927
const modulesBinding = internalBinding('modules');
3028
const path = require('path');
31-
const permission = require('internal/process/permission');
3229
const { validateString } = require('internal/validators');
3330
const internalFsBinding = internalBinding('fs');
3431

35-
const nearestParentPackageJSONCache = new SafeMap();
3632

3733
/**
3834
* @typedef {import('typings/internalBinding/modules').DeserializedPackageConfig} DeserializedPackageConfig
@@ -68,28 +64,41 @@ function deserializePackageJSON(path, contents) {
6864

6965
const pjsonPath = optionalFilePath ?? path;
7066

71-
return {
72-
data: {
67+
const data = {
68+
__proto__: null,
69+
...(name != null && { name }),
70+
...(main != null && { main }),
71+
...(type != null && { type }),
72+
};
73+
74+
if (plainExports !== null) {
75+
ObjectDefineProperty(data, 'exports', {
76+
__proto__: null,
77+
configurable: true,
78+
enumerable: true,
79+
get() {
80+
const value = requiresJSONParse(plainExports) ? JSONParse(plainExports) : plainExports;
81+
ObjectDefineProperty(data, 'exports', { __proto__: null, enumerable: true, value });
82+
return value;
83+
},
84+
});
85+
}
86+
87+
if (plainImports !== null) {
88+
ObjectDefineProperty(data, 'imports', {
7389
__proto__: null,
74-
...(name != null && { name }),
75-
...(main != null && { main }),
76-
...(type != null && { type }),
77-
...(plainImports != null && {
78-
// This getters are used to lazily parse the imports and exports fields.
79-
get imports() {
80-
const value = requiresJSONParse(plainImports) ? JSONParse(plainImports) : plainImports;
81-
ObjectDefineProperty(this, 'imports', { __proto__: null, value });
82-
return this.imports;
83-
},
84-
}),
85-
...(plainExports != null && {
86-
get exports() {
87-
const value = requiresJSONParse(plainExports) ? JSONParse(plainExports) : plainExports;
88-
ObjectDefineProperty(this, 'exports', { __proto__: null, value });
89-
return this.exports;
90-
},
91-
}),
92-
},
90+
configurable: true,
91+
enumerable: true,
92+
get() {
93+
const value = requiresJSONParse(plainImports) ? JSONParse(plainImports) : plainImports;
94+
ObjectDefineProperty(data, 'imports', { __proto__: null, enumerable: true, value });
95+
return value;
96+
},
97+
});
98+
}
99+
100+
return {
101+
data,
93102
exists: true,
94103
path: pjsonPath,
95104
};
@@ -131,43 +140,23 @@ function read(jsonPath, { base, specifier, isESM } = kEmptyObject) {
131140
}
132141

133142
/**
134-
* Given a file path, walk the filesystem upwards until we find its closest parent
135-
* `package.json` file, stopping when:
136-
* 1. we find a `package.json` file;
137-
* 2. we find a path that we do not have permission to read;
138-
* 3. we find a containing `node_modules` directory;
139-
* 4. or, we reach the filesystem root
140-
* @returns {undefined | string}
143+
* A cache mapping a module's path to its parent `package.json` file's path.
144+
* This is used in concert with `deserializedPackageJSONCache` to improve
145+
* the performance of `getNearestParentPackageJSON` when called repeatedly
146+
* on the same module paths.
141147
*/
142-
function findParentPackageJSON(checkPath) {
143-
const enabledPermission = permission.isEnabled();
144-
145-
const rootSeparatorIndex = StringPrototypeIndexOf(checkPath, path.sep);
146-
let separatorIndex;
147-
148-
do {
149-
separatorIndex = StringPrototypeLastIndexOf(checkPath, path.sep);
150-
checkPath = StringPrototypeSlice(checkPath, 0, separatorIndex);
148+
const moduleToParentPackageJSONCache = new SafeMap();
151149

152-
if (enabledPermission && !permission.has('fs.read', checkPath + path.sep)) {
153-
return undefined;
154-
}
155-
156-
if (StringPrototypeEndsWith(checkPath, path.sep + 'node_modules')) {
157-
return undefined;
158-
}
159-
160-
const maybePackageJSONPath = checkPath + path.sep + 'package.json';
161-
const stat = internalFsBinding.internalModuleStat(checkPath + path.sep + 'package.json');
162-
163-
const packageJSONExists = stat === 0;
164-
if (packageJSONExists) {
165-
return maybePackageJSONPath;
166-
}
167-
} while (separatorIndex > rootSeparatorIndex);
168-
169-
return undefined;
170-
}
150+
/**
151+
* A cache mapping the path of a `package.json` file to its
152+
* {@link DeserializedPackageConfig deserialized representation},
153+
* as produced by {@link deserializedPackageJSONCache}. The purpose of this
154+
* cache is to ensure that we always return the same
155+
* {@link DeserializedPackageConfig} instance for a given `package.json`,
156+
* which is necessary to ensure that we don't re-parse `imports` and
157+
* `exports` redundantly.
158+
*/
159+
const deserializedPackageJSONCache = new SafeMap();
171160

172161
/**
173162
* Get the nearest parent package.json file from a given path.
@@ -176,26 +165,22 @@ function findParentPackageJSON(checkPath) {
176165
* @returns {undefined | DeserializedPackageConfig}
177166
*/
178167
function getNearestParentPackageJSON(checkPath) {
179-
const nearestParentPackageJSON = findParentPackageJSON(checkPath);
180-
181-
if (nearestParentPackageJSON === undefined) {
182-
return undefined;
168+
const parentPackageJSONPath = moduleToParentPackageJSONCache.get(checkPath);
169+
if (parentPackageJSONPath !== undefined) {
170+
return deserializedPackageJSONCache.get(parentPackageJSONPath);
183171
}
184172

185-
if (nearestParentPackageJSONCache.has(nearestParentPackageJSON)) {
186-
return nearestParentPackageJSONCache.get(nearestParentPackageJSON);
187-
}
173+
const result = modulesBinding.getNearestParentPackageJSON(checkPath);
174+
const packageConfig = deserializePackageJSON(checkPath, result);
188175

189-
const result = modulesBinding.readPackageJSON(nearestParentPackageJSON);
176+
moduleToParentPackageJSONCache.set(checkPath, packageConfig.path);
190177

191-
if (result === undefined) {
192-
nearestParentPackageJSONCache.set(checkPath, undefined);
193-
return undefined;
178+
const maybeCachedPackageConfig = deserializedPackageJSONCache.get(packageConfig.path);
179+
if (maybeCachedPackageConfig !== undefined) {
180+
return maybeCachedPackageConfig;
194181
}
195182

196-
const packageConfig = deserializePackageJSON(checkPath, result);
197-
nearestParentPackageJSONCache.set(nearestParentPackageJSON, packageConfig);
198-
183+
deserializedPackageJSONCache.set(packageConfig.path, packageConfig);
199184
return packageConfig;
200185
}
201186

Collapse file

‎src/node_modules.cc‎

Copy file name to clipboardExpand all lines: src/node_modules.cc
+53-11Lines changed: 53 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -97,15 +97,27 @@ const BindingData::PackageConfig* BindingData::GetPackageJSON(
9797

9898
auto cache_entry = binding_data->package_configs_.find(path.data());
9999
if (cache_entry != binding_data->package_configs_.end()) {
100-
return &cache_entry->second;
100+
auto& cache_value = cache_entry->second;
101+
if (cache_value) {
102+
return &*cache_value;
103+
}
104+
105+
// If we have a cache entry without a value, we've already
106+
// attempted to open and read this path and couldn't (it most
107+
// likely doesn't exist)
108+
return nullptr;
101109
}
102110

103111
PackageConfig package_config{};
104112
package_config.file_path = path;
105113
// No need to exclude BOM since simdjson will skip it.
106114
if (ReadFileSync(&package_config.raw_json, path.data()) < 0) {
115+
// Add `nullopt` to the package config cache so that we don't
116+
// need to open and attempt to read this path again
117+
binding_data->package_configs_.insert({std::string(path), std::nullopt});
107118
return nullptr;
108119
}
120+
109121
simdjson::ondemand::document document;
110122
simdjson::ondemand::object main_object;
111123
simdjson::error_code error =
@@ -238,7 +250,7 @@ const BindingData::PackageConfig* BindingData::GetPackageJSON(
238250
auto cached = binding_data->package_configs_.insert(
239251
{std::string(path), std::move(package_config)});
240252

241-
return &cached.first->second;
253+
return &*cached.first->second;
242254
}
243255

244256
void BindingData::ReadPackageJSON(const FunctionCallbackInfo<Value>& args) {
@@ -321,24 +333,49 @@ const BindingData::PackageConfig* BindingData::TraverseParent(
321333
return nullptr;
322334
}
323335

324-
void BindingData::GetNearestParentPackageJSONType(
325-
const FunctionCallbackInfo<Value>& args) {
336+
const std::filesystem::path BindingData::NormalizePath(
337+
Realm* realm, BufferValue* path_value) {
338+
// Check if the path has a trailing slash. If so, add it after
339+
// ToNamespacedPath() as it will be deleted by ToNamespacedPath()
340+
bool slashCheck = path_value->ToStringView().ends_with(kPathSeparator);
341+
342+
ToNamespacedPath(realm->env(), path_value);
343+
344+
auto path = path_value->ToPath();
345+
346+
if (slashCheck) {
347+
path /= "";
348+
}
349+
350+
return path;
351+
}
352+
353+
void BindingData::GetNearestParentPackageJSON(
354+
const v8::FunctionCallbackInfo<v8::Value>& args) {
326355
CHECK_GE(args.Length(), 1);
327356
CHECK(args[0]->IsString());
328357

329358
Realm* realm = Realm::GetCurrent(args);
330359
BufferValue path_value(realm->isolate(), args[0]);
331-
// Check if the path has a trailing slash. If so, add it after
332-
// ToNamespacedPath() as it will be deleted by ToNamespacedPath()
333-
bool slashCheck = path_value.ToStringView().ends_with(kPathSeparator);
334360

335-
ToNamespacedPath(realm->env(), &path_value);
361+
auto path = NormalizePath(realm, &path_value);
336362

337-
auto path = path_value.ToPath();
363+
auto package_json = TraverseParent(realm, path);
338364

339-
if (slashCheck) {
340-
path /= "";
365+
if (package_json != nullptr) {
366+
args.GetReturnValue().Set(package_json->Serialize(realm));
341367
}
368+
}
369+
370+
void BindingData::GetNearestParentPackageJSONType(
371+
const FunctionCallbackInfo<Value>& args) {
372+
CHECK_GE(args.Length(), 1);
373+
CHECK(args[0]->IsString());
374+
375+
Realm* realm = Realm::GetCurrent(args);
376+
BufferValue path_value(realm->isolate(), args[0]);
377+
378+
auto path = NormalizePath(realm, &path_value);
342379

343380
auto package_json = TraverseParent(realm, path);
344381

@@ -569,6 +606,10 @@ void BindingData::CreatePerIsolateProperties(IsolateData* isolate_data,
569606
target,
570607
"getNearestParentPackageJSONType",
571608
GetNearestParentPackageJSONType);
609+
SetMethod(isolate,
610+
target,
611+
"getNearestParentPackageJSON",
612+
GetNearestParentPackageJSON);
572613
SetMethod(
573614
isolate, target, "getPackageScopeConfig", GetPackageScopeConfig<false>);
574615
SetMethod(isolate, target, "getPackageType", GetPackageScopeConfig<true>);
@@ -624,6 +665,7 @@ void BindingData::RegisterExternalReferences(
624665
ExternalReferenceRegistry* registry) {
625666
registry->Register(ReadPackageJSON);
626667
registry->Register(GetNearestParentPackageJSONType);
668+
registry->Register(GetNearestParentPackageJSON);
627669
registry->Register(GetPackageScopeConfig<false>);
628670
registry->Register(GetPackageScopeConfig<true>);
629671
registry->Register(EnableCompileCache);
Collapse file

‎src/node_modules.h‎

Copy file name to clipboardExpand all lines: src/node_modules.h
+12-1Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -55,6 +55,8 @@ class BindingData : public SnapshotableObject {
5555
SET_MEMORY_INFO_NAME(BindingData)
5656

5757
static void ReadPackageJSON(const v8::FunctionCallbackInfo<v8::Value>& args);
58+
static void GetNearestParentPackageJSON(
59+
const v8::FunctionCallbackInfo<v8::Value>& args);
5860
static void GetNearestParentPackageJSONType(
5961
const v8::FunctionCallbackInfo<v8::Value>& args);
6062
template <bool return_only_type>
@@ -72,8 +74,17 @@ class BindingData : public SnapshotableObject {
7274
static void RegisterExternalReferences(ExternalReferenceRegistry* registry);
7375

7476
private:
75-
std::unordered_map<std::string, PackageConfig> package_configs_;
77+
/*
78+
* This map caches `PackageConfig` values by `package.json` path.
79+
* An empty optional value indicates that no `package.json` file
80+
* at the given path exists, which we cache to avoid repeated
81+
* attempts to open the same non-existent paths.
82+
*/
83+
std::unordered_map<std::string, std::optional<PackageConfig> >
84+
package_configs_;
7685
simdjson::ondemand::parser json_parser;
86+
static const std::filesystem::path NormalizePath(Realm* realm,
87+
BufferValue* path_value);
7788
// returns null on error
7889
static const PackageConfig* GetPackageJSON(
7990
Realm* realm,
Collapse file

‎typings/internalBinding/modules.d.ts‎

Copy file name to clipboardExpand all lines: typings/internalBinding/modules.d.ts
+1Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ export type SerializedPackageConfig = [
2323
export interface ModulesBinding {
2424
readPackageJSON(path: string): SerializedPackageConfig | undefined;
2525
getNearestParentPackageJSONType(path: string): PackageConfig['type']
26+
getNearestParentPackageJSON(path: string): SerializedPackageConfig | undefined
2627
getPackageScopeConfig(path: string): SerializedPackageConfig | undefined
2728
getPackageType(path: string): PackageConfig['type'] | undefined
2829
enableCompileCache(path?: string): { status: number, message?: string, directory?: string }

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.