Skip to content

Commit

Permalink
lib: optimize prepareStackTrace on builtin frames
Browse files Browse the repository at this point in the history
Only invalidates source map lookup cache when a new source map is found.
This improves when user codes interleave with builtin functions, like
`array.map`.

PR-URL: nodejs#56299
Refs: nodejs#56296
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Xuguang Mei <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Chemi Atlow <[email protected]>
Reviewed-By: Pietro Marchini <[email protected]>
  • Loading branch information
legendecas authored and aduh95 committed Dec 20, 2024
1 parent a33ec12 commit 28557ef
Show file tree
Hide file tree
Showing 4 changed files with 31 additions and 17 deletions.
21 changes: 11 additions & 10 deletions benchmark/fixtures/simple-error-stack.js

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

12 changes: 7 additions & 5 deletions benchmark/fixtures/simple-error-stack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,13 @@
const lorem = 'Lorem ipsum dolor sit amet, consectetur adipiscing elit, sed do eiusmod tempor incididunt ut labore et dolore magna aliqua. Ut enim ad minim veniam, quis nostrud exercitation ullamco laboris nisi ut aliquip ex ea commodo consequat. Duis aute irure dolor in reprehenderit in voluptate velit esse cillum dolore eu fugiat nulla pariatur. Excepteur sint occaecat cupidatat non proident, sunt in culpa qui officia deserunt mollit anim id est laborum.';

function simpleErrorStack() {
try {
(lorem as any).BANG();
} catch (e) {
return e.stack;
}
[1].map(() => {
try {
(lorem as any).BANG();
} catch (e) {
return e.stack;
}
})
}

export {
Expand Down
10 changes: 8 additions & 2 deletions lib/internal/source_map/prepare_stack_trace.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,9 +53,15 @@ function prepareStackTraceWithSourceMaps(error, trace) {
const sm = fileName === lastFileName ?
lastSourceMap :
findSourceMap(fileName);
lastSourceMap = sm;
lastFileName = fileName;
// Only when a source map is found, cache it for the next iteration.
// This is a performance optimization to avoid interleaving with JS builtin function
// invalidating the cache.
// - at myFunc (file:///path/to/file.js:1:2)
// - at Array.map (<anonymous>)
// - at myFunc (file:///path/to/file.js:3:4)
if (sm) {
lastSourceMap = sm;
lastFileName = fileName;
return `${kStackLineAt}${serializeJSStackFrame(sm, callSite, trace[i + 1])}`;
}
} catch (err) {
Expand Down
5 changes: 5 additions & 0 deletions lib/internal/source_map/source_map_cache.js
Original file line number Diff line number Diff line change
Expand Up @@ -347,6 +347,11 @@ function findSourceMap(sourceURL) {
return undefined;
}

// No source maps for builtin modules.
if (sourceURL.startsWith('node:')) {
return undefined;
}

SourceMap ??= require('internal/source_map/source_map').SourceMap;
try {
if (RegExpPrototypeExec(kLeadingProtocol, sourceURL) === null) {
Expand Down

0 comments on commit 28557ef

Please sign in to comment.