Skip to content

[sourcemaps] Improve ignore-listing performance #81311

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 39 additions & 20 deletions packages/next/src/server/dev/middleware-webpack.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@ import { SourceMapConsumer } from 'next/dist/compiled/source-map08'
import type { StackFrame } from 'next/dist/compiled/stacktrace-parser'
import { getSourceMapFromFile } from './get-source-map-from-file'
import {
findApplicableSourceMapPayload,
sourceMapIgnoreListsEverything,
type BasicSourceMapPayload,
type ModernSourceMapPayload,
} from '../lib/source-maps'
import { openFileInEditor } from '../../next-devtools/server/launch-editor'
Expand Down Expand Up @@ -50,13 +52,13 @@ type SourceAttributes = {
type Source =
| {
type: 'file'
sourceMap: ModernSourceMapPayload
sourceMap: BasicSourceMapPayload
ignoredSources: IgnoredSources
moduleURL: string
}
| {
type: 'bundle'
sourceMap: ModernSourceMapPayload
sourceMap: BasicSourceMapPayload
ignoredSources: IgnoredSources
compilation: webpack.Compilation
moduleId: string
Expand Down Expand Up @@ -284,11 +286,16 @@ async function getSourceMapFromCompilation(
}

async function getSource(
sourceURL: string,
frame: {
file: string | null
lineNumber: number | null
column: number | null
},
options: {
getCompilations: () => webpack.Compilation[]
}
): Promise<Source | undefined> {
let sourceURL = frame.file ?? ''
const { getCompilations } = options

// Rspack is now using file:// URLs for source maps. Remove the rsc prefix to produce the file:/// url.
Expand All @@ -308,7 +315,11 @@ async function getSource(
const sourceMapPayload = nativeSourceMap.payload
return {
type: 'file',
sourceMap: sourceMapPayload,
sourceMap: findApplicableSourceMapPayload(
frame.lineNumber ?? 0,
frame.column ?? 0,
sourceMapPayload
)!,

ignoredSources: getIgnoredSources(
// @ts-expect-error -- TODO: Support IndexSourceMap
Expand Down Expand Up @@ -435,7 +446,7 @@ async function getOriginalStackFrame({
rootDirectory: string
}): Promise<OriginalStackFrameResponse> {
const filename = frame.file ?? ''
const source = await getSource(filename, {
const source = await getSource(frame, {
getCompilations: () => {
const compilations: webpack.Compilation[] = []

Expand Down Expand Up @@ -658,23 +669,31 @@ export function getSourceMapMiddleware(options: {
let source: Source | undefined

try {
source = await getSource(filename, {
getCompilations: () => {
const compilations: webpack.Compilation[] = []

for (const stats of [
clientStats(),
serverStats(),
edgeServerStats(),
]) {
if (stats?.compilation) {
compilations.push(stats.compilation)
source = await getSource(
{
file: filename,
// Webpack doesn't use Index Source Maps
lineNumber: null,
column: null,
},
{
getCompilations: () => {
const compilations: webpack.Compilation[] = []

for (const stats of [
clientStats(),
serverStats(),
edgeServerStats(),
]) {
if (stats?.compilation) {
compilations.push(stats.compilation)
}
}
}

return compilations
},
})
return compilations
},
}
)
} catch (error) {
return middlewareResponse.internalServerError(res, error)
}
Expand Down
52 changes: 29 additions & 23 deletions packages/next/src/server/lib/source-maps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ interface IndexSourceMap {
}

/** https://tc39.es/ecma426/#sec-source-map-format */
interface BasicSourceMapPayload {
export interface BasicSourceMapPayload {
version: number
// TODO: Move to https://github.com/jridgewell/sourcemaps which is actively maintained
/** WARNING: `file` is optional. */
Expand All @@ -34,17 +34,9 @@ interface BasicSourceMapPayload {

export type ModernSourceMapPayload = BasicSourceMapPayload | IndexSourceMap

// TODO: This should take the BasicSourceMapPayload. Only the relevant section
// needs to ignore list everything making this check effectively O(1) multiplied
// by the complexity of finding the section.
export function sourceMapIgnoreListsEverything(
sourceMap: ModernSourceMapPayload
sourceMap: BasicSourceMapPayload
): boolean {
if ('sections' in sourceMap) {
return sourceMap.sections.every((section) => {
return sourceMapIgnoreListsEverything(section.map)
})
}
return (
sourceMap.ignoreList !== undefined &&
sourceMap.sources.length === sourceMap.ignoreList.length
Expand All @@ -56,26 +48,40 @@ export function sourceMapIgnoreListsEverything(
* Equal to the input unless an Index Source Map is used.
*/
export function findApplicableSourceMapPayload(
lineNumber: number,
columnNumber: number,
line: number,
column: number,
payload: ModernSourceMapPayload
): BasicSourceMapPayload | undefined {
if ('sections' in payload) {
if (payload.sections.length === 0) {
return undefined
}

// Sections must not overlap and must be sorted: https://tc39.es/source-map/#section-object
// Therefore the last section that has an offset less than or equal to the frame is the applicable one.
// TODO(veil): Binary search
let section: IndexSourceMapSection | undefined = payload.sections[0]
for (
let i = 0;
i < payload.sections.length &&
payload.sections[i].offset.line <= lineNumber &&
payload.sections[i].offset.column <= columnNumber;
i++
) {
section = payload.sections[i]
const sections = payload.sections
let left = 0
let right = sections.length - 1
let result: IndexSourceMapSection | null = null

while (left <= right) {
// fast Math.floor
const middle = ~~((left + right) / 2)
const section = sections[middle]
const offset = section.offset

if (
offset.line < line ||
(offset.line === line && offset.column <= column)
) {
result = section
left = middle + 1
} else {
right = middle - 1
}
}

return section === undefined ? undefined : section.map
return result === null ? undefined : result.map
} else {
return payload
}
Expand Down
14 changes: 8 additions & 6 deletions packages/next/src/server/patch-error-inspect.ts
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,14 @@ function getSourcemappedFrameIfPossible(
line: frame.lineNumber ?? 1,
})

let ignored = sourceMapIgnoreListsEverything(sourceMapPayload)
const applicableSourceMap = findApplicableSourceMapPayload(
frame.lineNumber ?? 0,
frame.column ?? 0,
sourceMapPayload
)
let ignored =
applicableSourceMap !== undefined &&
sourceMapIgnoreListsEverything(applicableSourceMap)
if (sourcePosition.source === null) {
return {
stack: {
Expand All @@ -223,11 +230,6 @@ function getSourcemappedFrameIfPossible(
}
}

const applicableSourceMap = findApplicableSourceMapPayload(
frame.lineNumber ?? 0,
frame.column ?? 0,
sourceMapPayload
)
// TODO(veil): Upstream a method to sourcemap consumer that immediately says if a frame is ignored or not.
if (applicableSourceMap === undefined) {
console.error('No applicable source map found in sections for frame', frame)
Expand Down
Loading