From d054af577579ca7b39923950c33281e3c58eac15 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 19 Nov 2024 13:48:19 +0100 Subject: [PATCH 1/6] feat(core): Further optimize debug ID parsing --- packages/core/src/utils/prepareEvent.ts | 2 +- packages/node/src/integrations/anr/index.ts | 16 +--- packages/utils/src/debug-ids.ts | 95 ++++++++++++++------- 3 files changed, 67 insertions(+), 46 deletions(-) diff --git a/packages/core/src/utils/prepareEvent.ts b/packages/core/src/utils/prepareEvent.ts index 9ee0a0f1b1b6..a8fd3d695f6b 100644 --- a/packages/core/src/utils/prepareEvent.ts +++ b/packages/core/src/utils/prepareEvent.ts @@ -180,7 +180,7 @@ export function applyDebugIds(event: Event, stackParser: StackParser): void { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion exception.stacktrace!.frames!.forEach(frame => { if (frame.filename) { - frame.debug_id = filenameDebugIdMap[frame.filename]; + frame.debug_id = filenameDebugIdMap.get(frame.filename); } }); }); diff --git a/packages/node/src/integrations/anr/index.ts b/packages/node/src/integrations/anr/index.ts index c5f5b28e0888..78dc1a275794 100644 --- a/packages/node/src/integrations/anr/index.ts +++ b/packages/node/src/integrations/anr/index.ts @@ -1,4 +1,3 @@ -import * as diagnosticsChannel from 'node:diagnostics_channel'; import { Worker } from 'node:worker_threads'; import { defineIntegration, getCurrentScope, getGlobalScope, getIsolationScope, mergeScopeData } from '@sentry/core'; import type { Contexts, Event, EventHint, Integration, IntegrationFn, ScopeData } from '@sentry/types'; @@ -101,13 +100,6 @@ type AnrReturn = (options?: Partial) => Integration & Anr export const anrIntegration = defineIntegration(_anrIntegration) as AnrReturn; -function onModuleLoad(callback: () => void): void { - // eslint-disable-next-line deprecation/deprecation - diagnosticsChannel.channel('module.require.end').subscribe(() => callback()); - // eslint-disable-next-line deprecation/deprecation - diagnosticsChannel.channel('module.import.asyncEnd').subscribe(() => callback()); -} - /** * Starts the ANR worker thread * @@ -161,12 +153,6 @@ async function _startWorker( } } - let debugImages: Record = getFilenameToDebugIdMap(initOptions.stackParser); - - onModuleLoad(() => { - debugImages = getFilenameToDebugIdMap(initOptions.stackParser); - }); - const worker = new Worker(new URL(`data:application/javascript;base64,${base64WorkerScript}`), { workerData: options, // We don't want any Node args to be passed to the worker @@ -185,7 +171,7 @@ async function _startWorker( // serialized without making it a SerializedSession const session = currentSession ? { ...currentSession, toJSON: undefined } : undefined; // message the worker to tell it the main event loop is still running - worker.postMessage({ session, debugImages }); + worker.postMessage({ session, debugImages: getFilenameToDebugIdMap(initOptions.stackParser) }); } catch (_) { // } diff --git a/packages/utils/src/debug-ids.ts b/packages/utils/src/debug-ids.ts index 4802b9356965..3d1e521034ed 100644 --- a/packages/utils/src/debug-ids.ts +++ b/packages/utils/src/debug-ids.ts @@ -1,49 +1,83 @@ -import type { DebugImage, StackFrame, StackParser } from '@sentry/types'; +import type { DebugImage, StackParser } from '@sentry/types'; import { GLOBAL_OBJ } from './worldwide'; -const debugIdStackParserCache = new WeakMap>(); +type StackString = string; + +interface CachedResult { + filename: string; + debugId: string; +} + +let debugIdStackParserCache: WeakMap> | undefined; + +function getCacheForStackParser(stackParser: StackParser): Map { + if (!debugIdStackParserCache) { + debugIdStackParserCache = new WeakMap(); + } + + let result = debugIdStackParserCache.get(stackParser); + + if (!result) { + result = new Map(); + debugIdStackParserCache.set(stackParser, result); + } + + return result; +} + +let lastDebugIdKeyCount = 0; +let cachedFilenameToDebugId: Map | undefined; /** * Returns a map of filenames to debug identifiers. */ -export function getFilenameToDebugIdMap(stackParser: StackParser): Record { +export function getFilenameToDebugIdMap(stackParser: StackParser): Map { const debugIdMap = GLOBAL_OBJ._sentryDebugIds; if (!debugIdMap) { - return {}; + return new Map(); } - let debugIdStackFramesCache: Map; - const cachedDebugIdStackFrameCache = debugIdStackParserCache.get(stackParser); - if (cachedDebugIdStackFrameCache) { - debugIdStackFramesCache = cachedDebugIdStackFrameCache; - } else { - debugIdStackFramesCache = new Map(); - debugIdStackParserCache.set(stackParser, debugIdStackFramesCache); + const debugIdKeys = Object.keys(debugIdMap); + + // If the count of registered globals hasn't changed since the last call, we + // can just return the cached result. + if (debugIdKeys.length === lastDebugIdKeyCount && cachedFilenameToDebugId) { + return cachedFilenameToDebugId; } + const debugIdStackFramesCache = getCacheForStackParser(stackParser); + // Build a map of filename -> debug_id. - return Object.keys(debugIdMap).reduce>((acc, debugIdStackTrace) => { - let parsedStack: StackFrame[]; - - const cachedParsedStack = debugIdStackFramesCache.get(debugIdStackTrace); - if (cachedParsedStack) { - parsedStack = cachedParsedStack; - } else { - parsedStack = stackParser(debugIdStackTrace); - debugIdStackFramesCache.set(debugIdStackTrace, parsedStack); - } + const output = debugIdKeys.reduce>((acc, debugIdStackTrace) => { + let result = debugIdStackFramesCache.get(debugIdStackTrace); + + if (!result) { + const parsedStack = stackParser(debugIdStackTrace); - for (let i = parsedStack.length - 1; i >= 0; i--) { - const stackFrame = parsedStack[i]; - const file = stackFrame && stackFrame.filename; + for (let i = parsedStack.length - 1; i >= 0; i--) { + const stackFrame = parsedStack[i]; + const filename = stackFrame && stackFrame.filename; + const debugId = debugIdMap[debugIdStackTrace]; - if (stackFrame && file) { - acc[file] = debugIdMap[debugIdStackTrace] as string; - break; + if (filename && debugId) { + result = { filename, debugId }; + debugIdStackFramesCache.set(debugIdStackTrace, result); + break; + } } } + + if (result) { + acc.set(result.filename, result.debugId); + } + return acc; - }, {}); + }, new Map()); + + lastDebugIdKeyCount = Object.keys(debugIdMap).length; + cachedFilenameToDebugId = output; + + return output; } /** @@ -57,11 +91,12 @@ export function getDebugImagesForResources( const images: DebugImage[] = []; for (const path of resource_paths) { - if (path && filenameDebugIdMap[path]) { + const debug_id = filenameDebugIdMap.get(path); + if (path && debug_id) { images.push({ type: 'sourcemap', code_file: path, - debug_id: filenameDebugIdMap[path] as string, + debug_id, }); } } From 8fe053164d00d7658613708dc28479eba9642a74 Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 19 Nov 2024 13:55:18 +0100 Subject: [PATCH 2/6] Return undefined so we don't allocate --- packages/core/src/utils/prepareEvent.ts | 4 ++++ packages/utils/src/debug-ids.ts | 8 ++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/packages/core/src/utils/prepareEvent.ts b/packages/core/src/utils/prepareEvent.ts index a8fd3d695f6b..13796fc00657 100644 --- a/packages/core/src/utils/prepareEvent.ts +++ b/packages/core/src/utils/prepareEvent.ts @@ -174,6 +174,10 @@ export function applyDebugIds(event: Event, stackParser: StackParser): void { // Build a map of filename -> debug_id const filenameDebugIdMap = getFilenameToDebugIdMap(stackParser); + if (!filenameDebugIdMap) { + return; + } + try { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion event!.exception!.values!.forEach(exception => { diff --git a/packages/utils/src/debug-ids.ts b/packages/utils/src/debug-ids.ts index 3d1e521034ed..b0b9ef914925 100644 --- a/packages/utils/src/debug-ids.ts +++ b/packages/utils/src/debug-ids.ts @@ -31,10 +31,10 @@ let cachedFilenameToDebugId: Map | undefined; /** * Returns a map of filenames to debug identifiers. */ -export function getFilenameToDebugIdMap(stackParser: StackParser): Map { +export function getFilenameToDebugIdMap(stackParser: StackParser): Map | undefined { const debugIdMap = GLOBAL_OBJ._sentryDebugIds; if (!debugIdMap) { - return new Map(); + return undefined; } const debugIdKeys = Object.keys(debugIdMap); @@ -89,6 +89,10 @@ export function getDebugImagesForResources( ): DebugImage[] { const filenameDebugIdMap = getFilenameToDebugIdMap(stackParser); + if (!filenameDebugIdMap) { + return []; + } + const images: DebugImage[] = []; for (const path of resource_paths) { const debug_id = filenameDebugIdMap.get(path); From 1433e0f4b63b46177414d3e6e0ffc4701e9ab52e Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 19 Nov 2024 13:56:59 +0100 Subject: [PATCH 3/6] Less code --- packages/core/src/utils/prepareEvent.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/packages/core/src/utils/prepareEvent.ts b/packages/core/src/utils/prepareEvent.ts index 13796fc00657..51bcaa2cd540 100644 --- a/packages/core/src/utils/prepareEvent.ts +++ b/packages/core/src/utils/prepareEvent.ts @@ -174,16 +174,12 @@ export function applyDebugIds(event: Event, stackParser: StackParser): void { // Build a map of filename -> debug_id const filenameDebugIdMap = getFilenameToDebugIdMap(stackParser); - if (!filenameDebugIdMap) { - return; - } - try { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion event!.exception!.values!.forEach(exception => { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion exception.stacktrace!.frames!.forEach(frame => { - if (frame.filename) { + if (filenameDebugIdMap && frame.filename) { frame.debug_id = filenameDebugIdMap.get(frame.filename); } }); From 57b6625de29fd8186563ddd2115f88fa28944bea Mon Sep 17 00:00:00 2001 From: Tim Fish Date: Tue, 19 Nov 2024 14:16:13 +0100 Subject: [PATCH 4/6] Try and improve bundle size --- packages/utils/src/debug-ids.ts | 35 +++++++++++++-------------------- 1 file changed, 14 insertions(+), 21 deletions(-) diff --git a/packages/utils/src/debug-ids.ts b/packages/utils/src/debug-ids.ts index b0b9ef914925..a170c2d11fca 100644 --- a/packages/utils/src/debug-ids.ts +++ b/packages/utils/src/debug-ids.ts @@ -2,11 +2,7 @@ import type { DebugImage, StackParser } from '@sentry/types'; import { GLOBAL_OBJ } from './worldwide'; type StackString = string; - -interface CachedResult { - filename: string; - debugId: string; -} +type CachedResult = [string, string]; let debugIdStackParserCache: WeakMap> | undefined; @@ -25,8 +21,8 @@ function getCacheForStackParser(stackParser: StackParser): Map | undefined; +let lastCount = 0; +let cachedFilenameDebugIds: Map | undefined; /** * Returns a map of filenames to debug identifiers. @@ -41,17 +37,19 @@ export function getFilenameToDebugIdMap(stackParser: StackParser): Map debug_id. - const output = debugIdKeys.reduce>((acc, debugIdStackTrace) => { - let result = debugIdStackFramesCache.get(debugIdStackTrace); + cachedFilenameDebugIds = debugIdKeys.reduce>((acc, debugIdStackTrace) => { + const result = debugIdStackFramesCache.get(debugIdStackTrace); - if (!result) { + if (result) { + acc.set(result[0], result[1]); + } else { const parsedStack = stackParser(debugIdStackTrace); for (let i = parsedStack.length - 1; i >= 0; i--) { @@ -60,24 +58,19 @@ export function getFilenameToDebugIdMap(stackParser: StackParser): Map Date: Tue, 19 Nov 2024 14:47:55 +0100 Subject: [PATCH 5/6] Don't use maps --- packages/core/src/utils/prepareEvent.ts | 2 +- packages/utils/src/debug-ids.ts | 29 ++++++++++++------------- 2 files changed, 15 insertions(+), 16 deletions(-) diff --git a/packages/core/src/utils/prepareEvent.ts b/packages/core/src/utils/prepareEvent.ts index 51bcaa2cd540..1cb40415d43a 100644 --- a/packages/core/src/utils/prepareEvent.ts +++ b/packages/core/src/utils/prepareEvent.ts @@ -180,7 +180,7 @@ export function applyDebugIds(event: Event, stackParser: StackParser): void { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion exception.stacktrace!.frames!.forEach(frame => { if (filenameDebugIdMap && frame.filename) { - frame.debug_id = filenameDebugIdMap.get(frame.filename); + frame.debug_id = filenameDebugIdMap[frame.filename]; } }); }); diff --git a/packages/utils/src/debug-ids.ts b/packages/utils/src/debug-ids.ts index a170c2d11fca..5f4bc26c6c9c 100644 --- a/packages/utils/src/debug-ids.ts +++ b/packages/utils/src/debug-ids.ts @@ -4,9 +4,9 @@ import { GLOBAL_OBJ } from './worldwide'; type StackString = string; type CachedResult = [string, string]; -let debugIdStackParserCache: WeakMap> | undefined; +let debugIdStackParserCache: WeakMap> | undefined; -function getCacheForStackParser(stackParser: StackParser): Map { +function getCacheForStackParser(stackParser: StackParser): Record { if (!debugIdStackParserCache) { debugIdStackParserCache = new WeakMap(); } @@ -14,7 +14,7 @@ function getCacheForStackParser(stackParser: StackParser): Map | undefined; +let cachedFilenameDebugIds: Record | undefined; /** * Returns a map of filenames to debug identifiers. */ -export function getFilenameToDebugIdMap(stackParser: StackParser): Map | undefined { +export function getFilenameToDebugIdMap(stackParser: StackParser): Record { const debugIdMap = GLOBAL_OBJ._sentryDebugIds; if (!debugIdMap) { - return undefined; + return {}; } const debugIdKeys = Object.keys(debugIdMap); @@ -44,11 +44,11 @@ export function getFilenameToDebugIdMap(stackParser: StackParser): Map debug_id. - cachedFilenameDebugIds = debugIdKeys.reduce>((acc, debugIdStackTrace) => { - const result = debugIdStackFramesCache.get(debugIdStackTrace); + cachedFilenameDebugIds = debugIdKeys.reduce>((acc, debugIdStackTrace) => { + const result = debugIdStackFramesCache[debugIdStackTrace]; if (result) { - acc.set(result[0], result[1]); + acc[result[0]] = result[1]; } else { const parsedStack = stackParser(debugIdStackTrace); @@ -58,15 +58,15 @@ export function getFilenameToDebugIdMap(stackParser: StackParser): Map Date: Tue, 19 Nov 2024 16:09:25 +0100 Subject: [PATCH 6/6] Remove the cache per stack parser --- packages/utils/src/debug-ids.ts | 40 +++++++++++---------------------- 1 file changed, 13 insertions(+), 27 deletions(-) diff --git a/packages/utils/src/debug-ids.ts b/packages/utils/src/debug-ids.ts index 5f4bc26c6c9c..d407e6176e45 100644 --- a/packages/utils/src/debug-ids.ts +++ b/packages/utils/src/debug-ids.ts @@ -4,24 +4,8 @@ import { GLOBAL_OBJ } from './worldwide'; type StackString = string; type CachedResult = [string, string]; -let debugIdStackParserCache: WeakMap> | undefined; - -function getCacheForStackParser(stackParser: StackParser): Record { - if (!debugIdStackParserCache) { - debugIdStackParserCache = new WeakMap(); - } - - let result = debugIdStackParserCache.get(stackParser); - - if (!result) { - result = {}; - debugIdStackParserCache.set(stackParser, result); - } - - return result; -} - -let lastCount = 0; +let parsedStackResults: Record | undefined; +let lastKeysCount: number | undefined; let cachedFilenameDebugIds: Record | undefined; /** @@ -37,29 +21,33 @@ export function getFilenameToDebugIdMap(stackParser: StackParser): Record debug_id. - cachedFilenameDebugIds = debugIdKeys.reduce>((acc, debugIdStackTrace) => { - const result = debugIdStackFramesCache[debugIdStackTrace]; + cachedFilenameDebugIds = debugIdKeys.reduce>((acc, stackKey) => { + if (!parsedStackResults) { + parsedStackResults = {}; + } + + const result = parsedStackResults[stackKey]; if (result) { acc[result[0]] = result[1]; } else { - const parsedStack = stackParser(debugIdStackTrace); + const parsedStack = stackParser(stackKey); for (let i = parsedStack.length - 1; i >= 0; i--) { const stackFrame = parsedStack[i]; const filename = stackFrame && stackFrame.filename; - const debugId = debugIdMap[debugIdStackTrace]; + const debugId = debugIdMap[stackKey]; if (filename && debugId) { acc[filename] = debugId; - debugIdStackFramesCache[debugIdStackTrace] = [filename, debugId]; + parsedStackResults[stackKey] = [filename, debugId]; break; } } @@ -68,8 +56,6 @@ export function getFilenameToDebugIdMap(stackParser: StackParser): Record