Skip to content

Commit

Permalink
module: refactor to use iterable-weak-map
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
bcoe authored and danielleadams committed Nov 9, 2020
1 parent 1dd744a commit 8d76db8
Show file tree
Hide file tree
Showing 16 changed files with 281 additions and 62 deletions.
13 changes: 4 additions & 9 deletions doc/api/module.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,26 +135,22 @@ import { findSourceMap, SourceMap } from 'module';
const { findSourceMap, SourceMap } = require('module');
```
### `module.findSourceMap(path[, error])`
<!-- Anchors to make sure old links find a target -->
<a id="module_module_findsourcemap_path_error"></a>
### `module.findSourceMap(path)`
<!-- YAML
added:
- v13.7.0
- v12.17.0
-->
* `path` {string}
* `error` {Error}
* Returns: {module.SourceMap}
`path` is the resolved path for the file for which a corresponding source map
should be fetched.
The `error` instance should be passed as the second parameter to `findSourceMap`
in exceptional flows, such as when an overridden
[`Error.prepareStackTrace(error, trace)`][] is invoked. Modules are not added to
the module cache until they are successfully loaded. In these cases, source maps
are associated with the `error` instance along with the `path`.
### Class: `module.SourceMap`
<!-- YAML
added:
Expand Down Expand Up @@ -204,7 +200,6 @@ consists of the following keys:
[ES Modules]: esm.md
[Source map v3 format]: https://sourcemaps.info/spec.html#h.mofvlxcwqzej
[`--enable-source-maps`]: cli.md#cli_enable_source_maps
[`Error.prepareStackTrace(error, trace)`]: https://v8.dev/docs/stack-trace-api#customizing-stack-traces
[`NODE_V8_COVERAGE=dir`]: cli.md#cli_node_v8_coverage_dir
[`SourceMap`]: #module_class_module_sourcemap
[`createRequire()`]: #module_module_createrequire_filename
Expand Down
2 changes: 1 addition & 1 deletion doc/api/modules.md
Original file line number Diff line number Diff line change
Expand Up @@ -967,7 +967,7 @@ This section was moved to
[Modules: `module` core module](module.md#module_source_map_v3_support).

<!-- Anchors to make sure old links find a target -->
* <a id="modules_module_findsourcemap_path_error" href="module.html#module_module_findsourcemap_path_error">`module.findSourceMap(path[, error])`</a>
* <a id="modules_module_findsourcemap_path_error" href="module.html#module_module_findsourcemap_path">`module.findSourceMap(path)`</a>
* <a id="modules_class_module_sourcemap" href="module.html#module_class_module_sourcemap">Class: `module.SourceMap`</a>
* <a id="modules_new_sourcemap_payload" href="module.html#module_new_sourcemap_payload">`new SourceMap(payload)`</a>
* <a id="modules_sourcemap_payload" href="module.html#module_sourcemap_payload">`sourceMap.payload`</a>
Expand Down
1 change: 1 addition & 0 deletions lib/internal/per_context/primordials.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ function makeSafe(unsafe, safe) {
Object.freeze(safe);
return safe;
}
primordials.makeSafe = makeSafe;

// Subclass the constructors because we need to use their prototype
// methods later.
Expand Down
3 changes: 2 additions & 1 deletion lib/internal/source_map/prepare_stack_trace.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ const prepareStackTrace = (globalThis, error, trace) => {
let str = i !== 0 ? '\n at ' : '';
str = `${str}${t}`;
try {
const sm = findSourceMap(t.getFileName(), error);
const sm = findSourceMap(t.getFileName());
if (sm) {
// Source Map V3 lines/columns use zero-based offsets whereas, in
// stack traces, they start at 1/1.
Expand Down Expand Up @@ -119,6 +119,7 @@ function getErrorSource(payload, originalSource, firstLine, firstColumn) {
source = readFileSync(originalSourceNoScheme, 'utf8');
} catch (err) {
debug(err);
return '';
}
}

Expand Down
88 changes: 38 additions & 50 deletions lib/internal/source_map/source_map_cache.js
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
'use strict';

const {
ArrayPrototypeMap,
JSONParse,
ObjectCreate,
ObjectKeys,
ObjectGetOwnPropertyDescriptor,
ObjectPrototypeHasOwnProperty,
Map,
MapPrototypeEntries,
WeakMap,
WeakMapPrototypeGet,
RegExpPrototypeTest,
SafeMap,
StringPrototypeMatch,
StringPrototypeSplit,
uncurryThis,
} = primordials;

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

let sourceMapsEnabled;
Expand Down Expand Up @@ -70,13 +73,14 @@ function maybeCacheSourceMap(filename, content, cjsModuleInstance) {
debug(err.stack);
return;
}

const match = content.match(/\/[*/]#\s+sourceMappingURL=(?<sourceMappingURL>[^\s]+)/);
const match = StringPrototypeMatch(
content,
/\/[*/]#\s+sourceMappingURL=(?<sourceMappingURL>[^\s]+)/
);
if (match) {
const data = dataFromUrl(filename, match.groups.sourceMappingURL);
const url = data ? null : match.groups.sourceMappingURL;
if (cjsModuleInstance) {
if (!Module) Module = require('internal/modules/cjs/loader').Module;
cjsSourceMapCache.set(cjsModuleInstance, {
filename,
lineLengths: lineLengths(content),
Expand Down Expand Up @@ -120,7 +124,7 @@ function lineLengths(content) {
// We purposefully keep \r as part of the line-length calculation, in
// cases where there is a \r\n separator, so that this can be taken into
// account in coverage calculations.
return content.split(/\n|\u2028|\u2029/).map((line) => {
return ArrayPrototypeMap(StringPrototypeSplit(content, /\n|\u2028|\u2029/), (line) => {
return line.length;
});
}
Expand All @@ -139,8 +143,8 @@ function sourceMapFromFile(mapURL) {
// data:[<mediatype>][;base64],<data> see:
// https://tools.ietf.org/html/rfc2397#section-2
function sourceMapFromDataUrl(sourceURL, url) {
const [format, data] = url.split(',');
const splitFormat = format.split(';');
const [format, data] = StringPrototypeSplit(url, ',');
const splitFormat = StringPrototypeSplit(format, ';');
const contentType = splitFormat[0];
const base64 = splitFormat[splitFormat.length - 1] === 'base64';
if (contentType === 'application/json') {
Expand Down Expand Up @@ -207,48 +211,32 @@ function sourceMapCacheToObject() {
return obj;
}

// Since WeakMap can't be iterated over, we use Module._cache's
// keys to facilitate Source Map serialization.
//
// TODO(bcoe): this means we don't currently serialize source-maps attached
// to error instances, only module instances.
function appendCJSCache(obj) {
if (!Module) return;
const cjsModuleCache = ObjectGetValueSafe(Module, '_cache');
const cjsModules = ObjectKeys(cjsModuleCache);
for (let i = 0; i < cjsModules.length; i++) {
const key = cjsModules[i];
const module = ObjectGetValueSafe(cjsModuleCache, key);
const value = WeakMapPrototypeGet(cjsSourceMapCache, module);
if (value) {
// This is okay because `obj` has a null prototype.
obj[`file://${key}`] = {
lineLengths: ObjectGetValueSafe(value, 'lineLengths'),
data: ObjectGetValueSafe(value, 'data'),
url: ObjectGetValueSafe(value, 'url')
};
}
for (const value of cjsSourceMapCache) {
obj[ObjectGetValueSafe(value, 'filename')] = {
lineLengths: ObjectGetValueSafe(value, 'lineLengths'),
data: ObjectGetValueSafe(value, 'data'),
url: ObjectGetValueSafe(value, 'url')
};
}
}

// Attempt to lookup a source map, which is either attached to a file URI, or
// keyed on an error instance.
// TODO(bcoe): once WeakRefs are available in Node.js, refactor to drop
// requirement of error parameter.
function findSourceMap(uri, error) {
if (!Module) Module = require('internal/modules/cjs/loader').Module;
function findSourceMap(sourceURL) {
if (!RegExpPrototypeTest(/^\w+:\/\//, sourceURL)) {
sourceURL = pathToFileURL(sourceURL).href;
}
if (!SourceMap) {
SourceMap = require('internal/source_map/source_map').SourceMap;
}
let sourceMap = cjsSourceMapCache.get(Module._cache[uri]);
if (!uri.startsWith('file://')) uri = normalizeReferrerURL(uri);
if (sourceMap === undefined) {
sourceMap = esmSourceMapCache.get(uri);
}
let sourceMap = esmSourceMapCache.get(sourceURL);
if (sourceMap === undefined) {
const candidateSourceMap = cjsSourceMapCache.get(error);
if (candidateSourceMap && uri === candidateSourceMap.filename) {
sourceMap = candidateSourceMap;
for (const value of cjsSourceMapCache) {
const filename = ObjectGetValueSafe(value, 'filename');
if (sourceURL === filename) {
sourceMap = {
data: ObjectGetValueSafe(value, 'data')
};
}
}
}
if (sourceMap && sourceMap.data) {
Expand Down
86 changes: 86 additions & 0 deletions lib/internal/util/iterable_weak_map.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,86 @@
'use strict';

const {
makeSafe,
Object,
SafeSet,
SafeWeakMap,
SymbolIterator,
} = primordials;

// TODO(aduh95): Add FinalizationRegistry to primordials
const SafeFinalizationRegistry = makeSafe(
globalThis.FinalizationRegistry,
class SafeFinalizationRegistry extends globalThis.FinalizationRegistry {}
);

// TODO(aduh95): Add WeakRef to primordials
const SafeWeakRef = makeSafe(
globalThis.WeakRef,
class SafeWeakRef extends globalThis.WeakRef {}
);

// This class is modified from the example code in the WeakRefs specification:
// https://github.com/tc39/proposal-weakrefs
// Licensed under ECMA's MIT-style license, see:
// https://github.com/tc39/ecma262/blob/master/LICENSE.md
class IterableWeakMap {
#weakMap = new SafeWeakMap();
#refSet = new SafeSet();
#finalizationGroup = new SafeFinalizationRegistry(cleanup);

set(key, value) {
const entry = this.#weakMap.get(key);
if (entry) {
// If there's already an entry for the object represented by "key",
// the value can be updated without creating a new WeakRef:
this.#weakMap.set(key, { value, ref: entry.ref });
} else {
const ref = new SafeWeakRef(key);
this.#weakMap.set(key, { value, ref });
this.#refSet.add(ref);
this.#finalizationGroup.register(key, {
set: this.#refSet,
ref
}, ref);
}
}

get(key) {
return this.#weakMap.get(key)?.value;
}

has(key) {
return this.#weakMap.has(key);
}

delete(key) {
const entry = this.#weakMap.get(key);
if (!entry) {
return false;
}
this.#weakMap.delete(key);
this.#refSet.delete(entry.ref);
this.#finalizationGroup.unregister(entry.ref);
return true;
}

*[SymbolIterator]() {
for (const ref of this.#refSet) {
const key = ref.deref();
if (!key) continue;
const { value } = this.#weakMap.get(key);
yield value;
}
}
}

function cleanup({ set, ref }) {
set.delete(ref);
}

Object.freeze(IterableWeakMap.prototype);

module.exports = {
IterableWeakMap,
};
1 change: 1 addition & 0 deletions node.gyp
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,7 @@
'lib/internal/util/debuglog.js',
'lib/internal/util/inspect.js',
'lib/internal/util/inspector.js',
'lib/internal/util/iterable_weak_map.js',
'lib/internal/util/types.js',
'lib/internal/http2/core.js',
'lib/internal/http2/compat.js',
Expand Down
4 changes: 4 additions & 0 deletions test/fixtures/source-map/throw-on-require-entry.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions test/fixtures/source-map/throw-on-require-entry.js.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

15 changes: 15 additions & 0 deletions test/fixtures/source-map/throw-on-require.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions test/fixtures/source-map/throw-on-require.js.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

12 changes: 12 additions & 0 deletions test/fixtures/source-map/throw-on-require.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
enum ATrue {
IsTrue = 1,
IsFalse = 0
}

if (false) {
console.info('unreachable')
} else if (true) {
throw Error('throw early')
} else {
console.info('unreachable')
}
1 change: 1 addition & 0 deletions test/parallel/test-bootstrap-modules.js
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ const expectedModules = new Set([
'NativeModule internal/util',
'NativeModule internal/util/debuglog',
'NativeModule internal/util/inspect',
'NativeModule internal/util/iterable_weak_map',
'NativeModule internal/util/types',
'NativeModule internal/validators',
'NativeModule internal/vm/module',
Expand Down
Loading

0 comments on commit 8d76db8

Please sign in to comment.