FazBrowse GitHub Viewer | Trending |
URL:
| Home
Tools: [Download Repo ZIP]   [Original HTTPS Page]

module: refactor to use iterable-weak-map · nodejs/node@8d76db8 · GitHub

/ node Public

Commit 8d76db8

Browse files
bcoe
authored and
danielleadams
committed
module: refactor to use iterable-weak-map
Using an iterable WeakMap (a data-structure that uses WeakRef and WeakMap), we are able to: stop relying on Module._cache to serialize source maps; stop requiring an error object when calling findSourceMap(). PR-URL: #35915 Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com> Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
1 parent 1dd744a commit 8d76db8

16 files changed

Lines changed: 281 additions & 62 deletions

‎doc/api/module.md‎

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -135,26 +135,22 @@ import { findSourceMap, SourceMap } from 'module';
135135
const { findSourceMap, SourceMap } = require('module');
136136
```
137137
138-
### `module.findSourceMap(path[, error])`
138+
<!-- Anchors to make sure old links find a target -->
139+
<a id="module_module_findsourcemap_path_error"></a>
140+
### `module.findSourceMap(path)`
141+
139142
<!-- YAML
140143
added:
141144
- v13.7.0
142145
- v12.17.0
143146
-->
144147
145148
* `path` {string}
146-
* `error` {Error}
147149
* Returns: {module.SourceMap}
148150
149151
`path` is the resolved path for the file for which a corresponding source map
150152
should be fetched.
151153
152-
The `error` instance should be passed as the second parameter to `findSourceMap`
153-
in exceptional flows, such as when an overridden
154-
[`Error.prepareStackTrace(error, trace)`][] is invoked. Modules are not added to
155-
the module cache until they are successfully loaded. In these cases, source maps
156-
are associated with the `error` instance along with the `path`.
157-
158154
### Class: `module.SourceMap`
159155
<!-- YAML
160156
added:
@@ -204,7 +200,6 @@ consists of the following keys:
204200
[ES Modules]: esm.md
205201
[Source map v3 format]: https://sourcemaps.info/spec.html#h.mofvlxcwqzej
206202
[`--enable-source-maps`]: cli.md#cli_enable_source_maps
207-
[`Error.prepareStackTrace(error, trace)`]: https://v8.dev/docs/stack-trace-api#customizing-stack-traces
208203
[`NODE_V8_COVERAGE=dir`]: cli.md#cli_node_v8_coverage_dir
209204
[`SourceMap`]: #module_class_module_sourcemap
210205
[`createRequire()`]: #module_module_createrequire_filename

‎doc/api/modules.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -967,7 +967,7 @@ This section was moved to
967967
[Modules: `module` core module](module.md#module_source_map_v3_support).
968968

969969
<!-- Anchors to make sure old links find a target -->
970-
* <a id="modules_module_findsourcemap_path_error" href="module.html#module_module_findsourcemap_path_error">`module.findSourceMap(path[, error])`</a>
970+
* <a id="modules_module_findsourcemap_path_error" href="module.html#module_module_findsourcemap_path">`module.findSourceMap(path)`</a>
971971
* <a id="modules_class_module_sourcemap" href="module.html#module_class_module_sourcemap">Class: `module.SourceMap`</a>
972972
* <a id="modules_new_sourcemap_payload" href="module.html#module_new_sourcemap_payload">`new SourceMap(payload)`</a>
973973
* <a id="modules_sourcemap_payload" href="module.html#module_sourcemap_payload">`sourceMap.payload`</a>

‎lib/internal/per_context/primordials.js‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ function makeSafe(unsafe, safe) {
8686
Object.freeze(safe);
8787
return safe;
8888
}
89+
primordials.makeSafe = makeSafe;
8990

9091
// Subclass the constructors because we need to use their prototype
9192
// methods later.

‎lib/internal/source_map/prepare_stack_trace.js‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ const prepareStackTrace = (globalThis, error, trace) => {
5757
let str = i !== 0 ? '\n at ' : '';
5858
str = `${str}${t}`;
5959
try {
60-
const sm = findSourceMap(t.getFileName(), error);
60+
const sm = findSourceMap(t.getFileName());
6161
if (sm) {
6262
// Source Map V3 lines/columns use zero-based offsets whereas, in
6363
// stack traces, they start at 1/1.
@@ -119,6 +119,7 @@ function getErrorSource(payload, originalSource, firstLine, firstColumn) {
119119
source = readFileSync(originalSourceNoScheme, 'utf8');
120120
} catch (err) {
121121
debug(err);
122+
return '';
122123
}
123124
}
124125

‎lib/internal/source_map/source_map_cache.js‎

Lines changed: 38 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,18 @@
11
'use strict';
22

33
const {
4+
ArrayPrototypeMap,
45
JSONParse,
56
ObjectCreate,
67
ObjectKeys,
78
ObjectGetOwnPropertyDescriptor,
89
ObjectPrototypeHasOwnProperty,
910
Map,
1011
MapPrototypeEntries,
11-
WeakMap,
12-
WeakMapPrototypeGet,
12+
RegExpPrototypeTest,
13+
SafeMap,
14+
StringPrototypeMatch,
15+
StringPrototypeSplit,
1316
uncurryThis,
1417
} = primordials;
1518

@@ -27,17 +30,17 @@ let debug = require('internal/util/debuglog').debuglog('source_map', (fn) => {
2730
});
2831
const fs = require('fs');
2932
const { getOptionValue } = require('internal/options');
33+
const { IterableWeakMap } = require('internal/util/iterable_weak_map');
3034
const {
3135
normalizeReferrerURL,
3236
} = require('internal/modules/cjs/helpers');
33-
// For cjs, since Module._cache is exposed to users, we use a WeakMap
34-
// keyed on module, facilitating garbage collection.
35-
const cjsSourceMapCache = new WeakMap();
36-
// The esm cache is not exposed to users, so we can use a Map keyed
37-
// on filenames.
38-
const esmSourceMapCache = new Map();
39-
const { fileURLToPath, URL } = require('url');
40-
let Module;
37+
// Since the CJS module cache is mutable, which leads to memory leaks when
38+
// modules are deleted, we use a WeakMap so that the source map cache will
39+
// be purged automatically:
40+
const cjsSourceMapCache = new IterableWeakMap();
41+
// The esm cache is not mutable, so we can use a Map without memory concerns:
42+
const esmSourceMapCache = new SafeMap();
43+
const { fileURLToPath, pathToFileURL, URL } = require('internal/url');
4144
let SourceMap;
4245

4346
let sourceMapsEnabled;
@@ -70,13 +73,14 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance) {
7073
debug(err.stack);
7174
return;
7275
}
73-
74-
const match = content.match(/\/[*/]#\s+sourceMappingURL=(?<sourceMappingURL>[^\s]+)/);
76+
const match = StringPrototypeMatch(
77+
content,
78+
/\/[*/]#\s+sourceMappingURL=(?<sourceMappingURL>[^\s]+)/
79+
);
7580
if (match) {
7681
const data = dataFromUrl(filename, match.groups.sourceMappingURL);
7782
const url = data ? null : match.groups.sourceMappingURL;
7883
if (cjsModuleInstance) {
79-
if (!Module) Module = require('internal/modules/cjs/loader').Module;
8084
cjsSourceMapCache.set(cjsModuleInstance, {
8185
filename,
8286
lineLengths: lineLengths(content),
@@ -120,7 +124,7 @@ function lineLengths(content) {
120124
// We purposefully keep \r as part of the line-length calculation, in
121125
// cases where there is a \r\n separator, so that this can be taken into
122126
// account in coverage calculations.
123-
return content.split(/\n|\u2028|\u2029/).map((line) => {
127+
return ArrayPrototypeMap(StringPrototypeSplit(content, /\n|\u2028|\u2029/), (line) => {
124128
return line.length;
125129
});
126130
}
@@ -139,8 +143,8 @@ function sourceMapFromFile(mapURL) {
139143
// data:[<mediatype>][;base64],<data> see:
140144
// https://tools.ietf.org/html/rfc2397#section-2
141145
function sourceMapFromDataUrl(sourceURL, url) {
142-
const [format, data] = url.split(',');
143-
const splitFormat = format.split(';');
146+
const [format, data] = StringPrototypeSplit(url, ',');
147+
const splitFormat = StringPrototypeSplit(format, ';');
144148
const contentType = splitFormat[0];
145149
const base64 = splitFormat[splitFormat.length - 1] === 'base64';
146150
if (contentType === 'application/json') {
@@ -207,48 +211,32 @@ function sourceMapCacheToObject() {
207211
return obj;
208212
}
209213

210-
// Since WeakMap can't be iterated over, we use Module._cache's
211-
// keys to facilitate Source Map serialization.
212-
//
213-
// TODO(bcoe): this means we don't currently serialize source-maps attached
214-
// to error instances, only module instances.
215214
function appendCJSCache(obj) {
216-
if (!Module) return;
217-
const cjsModuleCache = ObjectGetValueSafe(Module, '_cache');
218-
const cjsModules = ObjectKeys(cjsModuleCache);
219-
for (let i = 0; i < cjsModules.length; i++) {
220-
const key = cjsModules[i];
221-
const module = ObjectGetValueSafe(cjsModuleCache, key);
222-
const value = WeakMapPrototypeGet(cjsSourceMapCache, module);
223-
if (value) {
224-
// This is okay because `obj` has a null prototype.
225-
obj[`file://${key}`] = {
226-
lineLengths: ObjectGetValueSafe(value, 'lineLengths'),
227-
data: ObjectGetValueSafe(value, 'data'),
228-
url: ObjectGetValueSafe(value, 'url')
229-
};
230-
}
215+
for (const value of cjsSourceMapCache) {
216+
obj[ObjectGetValueSafe(value, 'filename')] = {
217+
lineLengths: ObjectGetValueSafe(value, 'lineLengths'),
218+
data: ObjectGetValueSafe(value, 'data'),
219+
url: ObjectGetValueSafe(value, 'url')
220+
};
231221
}
232222
}
233223

234-
// Attempt to lookup a source map, which is either attached to a file URI, or
235-
// keyed on an error instance.
236-
// TODO(bcoe): once WeakRefs are available in Node.js, refactor to drop
237-
// requirement of error parameter.
238-
function findSourceMap(uri, error) {
239-
if (!Module) Module = require('internal/modules/cjs/loader').Module;
224+
function findSourceMap(sourceURL) {
225+
if (!RegExpPrototypeTest(/^\w+:\/\//, sourceURL)) {
226+
sourceURL = pathToFileURL(sourceURL).href;
227+
}
240228
if (!SourceMap) {
241229
SourceMap = require('internal/source_map/source_map').SourceMap;
242230
}
243-
let sourceMap = cjsSourceMapCache.get(Module._cache[uri]);
244-
if (!uri.startsWith('file://')) uri = normalizeReferrerURL(uri);
245-
if (sourceMap === undefined) {
246-
sourceMap = esmSourceMapCache.get(uri);
247-
}
231+
let sourceMap = esmSourceMapCache.get(sourceURL);
248232
if (sourceMap === undefined) {
249-
const candidateSourceMap = cjsSourceMapCache.get(error);
250-
if (candidateSourceMap && uri === candidateSourceMap.filename) {
251-
sourceMap = candidateSourceMap;
233+
for (const value of cjsSourceMapCache) {
234+
const filename = ObjectGetValueSafe(value, 'filename');
235+
if (sourceURL === filename) {
236+
sourceMap = {
237+
data: ObjectGetValueSafe(value, 'data')
238+
};
239+
}
252240
}
253241
}
254242
if (sourceMap && sourceMap.data) {

‎lib/internal/util/iterable_weak_map.js‎

Lines changed: 86 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,86 @@
1+
'use strict';
2+
3+
const {
4+
makeSafe,
5+
Object,
6+
SafeSet,
7+
SafeWeakMap,
8+
SymbolIterator,
9+
} = primordials;
10+
11+
// TODO(aduh95): Add FinalizationRegistry to primordials
12+
const SafeFinalizationRegistry = makeSafe(
13+
globalThis.FinalizationRegistry,
14+
class SafeFinalizationRegistry extends globalThis.FinalizationRegistry {}
15+
);
16+
17+
// TODO(aduh95): Add WeakRef to primordials
18+
const SafeWeakRef = makeSafe(
19+
globalThis.WeakRef,
20+
class SafeWeakRef extends globalThis.WeakRef {}
21+
);
22+
23+
// This class is modified from the example code in the WeakRefs specification:
24+
// https://github.com/tc39/proposal-weakrefs
25+
// Licensed under ECMA's MIT-style license, see:
26+
// https://github.com/tc39/ecma262/blob/master/LICENSE.md
27+
class IterableWeakMap {
28+
#weakMap = new SafeWeakMap();
29+
#refSet = new SafeSet();
30+
#finalizationGroup = new SafeFinalizationRegistry(cleanup);
31+
32+
set(key, value) {
33+
const entry = this.#weakMap.get(key);
34+
if (entry) {
35+
// If there's already an entry for the object represented by "key",
36+
// the value can be updated without creating a new WeakRef:
37+
this.#weakMap.set(key, { value, ref: entry.ref });
38+
} else {
39+
const ref = new SafeWeakRef(key);
40+
this.#weakMap.set(key, { value, ref });
41+
this.#refSet.add(ref);
42+
this.#finalizationGroup.register(key, {
43+
set: this.#refSet,
44+
ref
45+
}, ref);
46+
}
47+
}
48+
49+
get(key) {
50+
return this.#weakMap.get(key)?.value;
51+
}
52+
53+
has(key) {
54+
return this.#weakMap.has(key);
55+
}
56+
57+
delete(key) {
58+
const entry = this.#weakMap.get(key);
59+
if (!entry) {
60+
return false;
61+
}
62+
this.#weakMap.delete(key);
63+
this.#refSet.delete(entry.ref);
64+
this.#finalizationGroup.unregister(entry.ref);
65+
return true;
66+
}
67+
68+
*[SymbolIterator]() {
69+
for (const ref of this.#refSet) {
70+
const key = ref.deref();
71+
if (!key) continue;
72+
const { value } = this.#weakMap.get(key);
73+
yield value;
74+
}
75+
}
76+
}
77+
78+
function cleanup({ set, ref }) {
79+
set.delete(ref);
80+
}
81+
82+
Object.freeze(IterableWeakMap.prototype);
83+
84+
module.exports = {
85+
IterableWeakMap,
86+
};

‎node.gyp‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,7 @@
227227
'lib/internal/util/debuglog.js',
228228
'lib/internal/util/inspect.js',
229229
'lib/internal/util/inspector.js',
230+
'lib/internal/util/iterable_weak_map.js',
230231
'lib/internal/util/types.js',
231232
'lib/internal/http2/core.js',
232233
'lib/internal/http2/compat.js',

‎test/fixtures/source-map/throw-on-require-entry.js‎

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/fixtures/source-map/throw-on-require-entry.js.map‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎test/fixtures/source-map/throw-on-require.js‎

Lines changed: 15 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)
Back | FazBrowse Home | New Git URL