Skip to content

composeSourceMaps: Keep mappings that share a generated column - #1981

Open
robhogan wants to merge 1 commit into
pr1980from
pr1981
Open

robhogan wants to merge 1 commit into
pr1980from
pr1981

Conversation

@robhogan

@robhogan robhogan commented Sep 26, 2026 •

Copy link
Copy Markdown
Collaborator

Context

composeSourceMaps iterates the mappings of the last map, and looked each one's generated position up again in that same map before tracing it back through the earlier maps:

for (const consumer of consumers) {
if (currentLine == null || currentColumn == null) {
return {line: null, column: null, source: null, name: null};
}
original = consumer.originalPositionFor({
line: currentLine,
column: currentColumn,
});
currentLine = original.line;
currentColumn = original.column;

That's redundant, except where the last map has more than one mapping at the same generated column, which hermesc emits (15.7k in our benchmark's Hermes map). There, every mapping at the column resolved to the last one and the duplicates were collapsed.

This change

This traces each mapping from its own original position, so the composed map keeps all of them. Lookups through Metro's Consumer are unchanged, since it resolves a column to its last mapping. Consumers that take the first mapping at a column, like @jridgewell/trace-mapping, now get that mapping's own position.

composeSourceMaps is 8.1% faster than #1980 (95% CI 7.6-8.7%) and 2.59x faster than main (878ms -> 805ms on our benchmark app). The output gains 13,867 mappings (+0.8%) at repeated columns and is otherwise identical.

Benchmark

AI-driven.

Time (median) vs previous vs main Composition memory vs previous vs main Peak RSS
main 2,088ms (2,082 to 2,094) 1,070MB (1,070 to 1,070) 1,664MB
#1980 878ms (872 to 883) -58.0% (-58.1 to -57.8) 908MB (908 to 909) -15.1% (-15.2 to -15.0) 1,502MB
This PR 805ms (802 to 811) -8.1% (-8.7 to -7.6) -61.4% (-61.6 to -61.1) 911MB (910 to 912) +0.2% (+0.1 to +0.4) -14.9% (-15.0 to -14.7) 1,505MB

Changelog

 - **[Fix]**: `composeSourceMaps` keeps every mapping at a generated column that the last map maps more than once

Test plan

New test fails on #1980 (only the last position survives) and passes here.

Benchmark (AI-driven): Mattermost Mobile 2.45.0 (React Native 0.83.9, Metro 0.83.7), iOS release bundle, unminified: 7,086 sources, 52.6MB bundle, 82.4MB flat source map. Composed with its hermesc -O -output-source-map map (hermes-compiler 0.14.1, 10.7MB, 1.84M segments). Timings are composeSourceMaps alone, excluding JSON parsing, over 105 rounds on Node 22.13.1 on an M5 Pro. Each round runs main, every PR in this stack and a memory baseline, in a shuffled order. Figures are medians with 95% bootstrap confidence intervals, for changes of the per-round paired ratio. Composition memory is peak RSS less that of the same process parsing both maps without composing them (594MB).

@robhogan
robhogan added this pull request to stack #1982 September 26, 2026 07:54
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 26, 2026
@robhogan
robhogan force-pushed the pr1981 branch 2 times, most recently from 9160f47 to b25b719 Compare September 26, 2026 08:39
@robhogan
robhogan requested a lite review from Copilot September 28, 2026 12:20

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

馃煝 Approval recommended

No unresolved review issues were identified, and regression coverage is included.

Review effort: Lite
Findings: None

What changed in this PR

Updates composeSourceMaps to preserve distinct mappings sharing a generated column.

Changes:

  • Traces each mapping from its own original position.
  • Avoids redundant lookups.
  • Adds regression coverage for repeated generated columns.
File Description
packages/鈥媘etro-source-map/鈥媠rc/鈥媍omposeSourceMaps.js Preserves and composes each mapping independently.
packages/鈥媘etro-source-map/鈥媠rc/鈥媉_tests__/鈥媍omposeSourceMaps-test.js Verifies repeated-column mappings are retained.

馃挕 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@robhogan
robhogan marked this pull request as ready for review September 28, 2026 12:22
@robhogan
robhogan force-pushed the pr1981 branch 3 times, most recently from ee369eb to ba21973 Compare September 29, 2026 15:43
`composeSourceMaps` iterates the mappings of the last map, and looked each one's generated position up again in that same map before tracing it back through the earlier maps. That second lookup is redundant, except where the last map has more than one mapping at the same generated column - which `hermesc` emits (15,689 in our benchmark app's Hermes map*). There, every mapping at the column resolved to the last one, and the generator collapsed the duplicates.

This traces each mapping from its own original position instead, so the composed map keeps all of them. Lookups through Metro's `Consumer` are unchanged, because it resolves a column to its last mapping. Consumers that take the first mapping at a column, like `@jridgewell/trace-mapping`, now get the first mapping's own position rather than the last's.

`composeSourceMaps` with a bundle map and its Hermes map is 8.1% faster than the previous diff (95% CI 7.6-8.7%) and 2.59x faster than `main` (2.57-2.61x; 878ms -> 805ms on our benchmark app*). The output gains 13,867 mappings (+0.8%) at repeated columns and is otherwise identical. Composition's memory is within 0.4% of the previous diff (+2MB*).

| | Time (median) | vs previous | vs `main` | Composition memory* | vs previous | vs `main` | Peak RSS |
|---|---|---|---|---|---|---|---|
| `main` | 2,088ms (2,082 to 2,094) | | | 1,070MB (1,070 to 1,070) | | | 1,664MB |
| Previous diff | 878ms (872 to 883) |  | -58.0% (-58.1 to -57.8) | 908MB (908 to 909) |  | -15.1% (-15.2 to -15.0) | 1,502MB |
| This diff | 805ms (802 to 811) | -8.1% (-8.7 to -7.6) | -61.4% (-61.6 to -61.1) | 911MB (910 to 912) | +0.2% (+0.1 to +0.4) | -14.9% (-15.0 to -14.7) | 1,505MB |

## Changelog

```
 - **[Fix]**: `composeSourceMaps` keeps every mapping at a generated column that the last map maps more than once
```

## Test plan

The new test fails on the previous diff (only the last position survives) and passes on this one.

```
yarn jest packages/metro-source-map packages/metro-symbolicate
yarn flow check
yarn lint
yarn jest
```

\* Benchmark: Mattermost Mobile 2.45.0 (React Native 0.83.9, Metro 0.83.7), iOS release bundle, unminified: 7,086 sources, 52.6MB bundle, 82.4MB flat source map. Composed with its `hermesc -O -output-source-map` map (hermes-compiler 0.14.1; 10.7MB, 1.84M segments). Timings are `composeSourceMaps` alone, excluding JSON parsing, over 105 rounds on Node 22.13.1 on an M5 Pro; each round runs `main`, every diff in this stack and a memory baseline, in a shuffled order. Figures are medians with 95% bootstrap confidence intervals - for changes, of the per-round paired ratio. Composition memory is peak RSS less that of the same process parsing both maps without composing them (594MB).

This branch has not been deployed

No deployments
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants