Turbopack: store serialized sourcemaps instead #75791

mischnic · 2025-02-07T13:00:37Z

Store sourcemaps JSON-serialized (as opposed to structs containing arrays of decoded mappings), which is more memory efficient (and the main operation we perform is concatenation anyway, lookups are only used for error message locations)

This will also deduplicate sourcemaps (because cloning a Rope is just a reference-counted pointer).

testing against 0293c96cf32

canary e5fc495e3d
19,1 GB
TURBO_ENGINE_READ_ONLY=1 NEXT_TURBOPACK_TRACING= TURBOPACK=1 TURBOPACK_BUILD=  526.06s user 87.43s system 853% cpu 1:11.91 total
TURBO_ENGINE_READ_ONLY=1 NEXT_TURBOPACK_TRACING= TURBOPACK=1 TURBOPACK_BUILD=  510.69s user 82.93s system 835% cpu 1:11.06 total

sourcemap-rope 2dc238215a
16.94 GB

TURBO_ENGINE_READ_ONLY=1 NEXT_TURBOPACK_TRACING= TURBOPACK=1 TURBOPACK_BUILD=  535.61s user 85.04s system 855% cpu 1:12.55 total
TURBO_ENGINE_READ_ONLY=1 NEXT_TURBOPACK_TRACING= TURBOPACK=1 TURBOPACK_BUILD=  531.07s user 82.25s system 861% cpu 1:11.22 total

Closes PACK-3947

mischnic · 2025-02-07T13:00:54Z

Turbopack: store serialized sourcemaps instead #75791 👈 (View in Graphite)
canary

This stack of pull requests is managed by Graphite. Learn more about stacking.

ijjk · 2025-02-07T13:07:37Z

Failing test suites

Commit: b24caae

pnpm test test/integration/bigint/test/index.test.js (turbopack)

bigint API route support > development mode > should return 200
bigint API route support > development mode > should return the BigInt result text

Expand output

● bigint API route support › development mode › should return 200

expect(received).toEqual(expected) // deep equality

Expected: 200
Received: 500

  22 |       method: 'GET',
  23 |     })
> 24 |     expect(res.status).toEqual(200)
     |                        ^
  25 |   })
  26 |
  27 |   it('should return the BigInt result text', async () => {

  at Object.toEqual (integration/bigint/test/index.test.js:24:24)

● bigint API route support › development mode › should return the BigInt result text

expect(received).toEqual(expected) // deep equality

Expected: "3"
Received: false

  29 |       method: 'GET',
  30 |     }).then((res) => res.ok && res.text())
> 31 |     expect(resText).toEqual('3')
     |                     ^
  32 |   })
  33 | }
  34 |

  at Object.toEqual (integration/bigint/test/index.test.js:31:21)

Read more about building and testing Next.js in contributing.md.

ijjk · 2025-02-07T13:07:38Z

Tests Passed

ijjk · 2025-02-07T13:12:32Z

Stats from current PR

Default Build

General

	vercel/next.js canary	vercel/next.js mischnic/store-sourcemap-string	Change
buildDuration	21.3s	18.3s	N/A
buildDurationCached	17.4s	14.5s	N/A
nodeModulesSize	393 MB	393 MB	✓
nextStartRea..uration (ms)	469ms	458ms	N/A

Client Bundles (main, webpack)

	vercel/next.js canary	vercel/next.js mischnic/store-sourcemap-string	Change
5306-HASH.js gzip	54.8 kB	54.8 kB	N/A
8276.HASH.js gzip	169 B	168 B	N/A
8377-HASH.js gzip	5.46 kB	5.46 kB	N/A
bccd1874-HASH.js gzip	53 kB	53 kB	✓
framework-HASH.js gzip	57.5 kB	57.5 kB	N/A
main-app-HASH.js gzip	245 B	245 B	✓
main-HASH.js gzip	34.6 kB	34.5 kB	N/A
webpack-HASH.js gzip	1.71 kB	1.71 kB	N/A
Overall change	53.2 kB	53.2 kB	✓

Legacy Client Bundles (polyfills)

	vercel/next.js canary	vercel/next.js mischnic/store-sourcemap-string	Change
polyfills-HASH.js gzip	39.4 kB	39.4 kB	✓
Overall change	39.4 kB	39.4 kB	✓

Client Pages

	vercel/next.js canary	vercel/next.js mischnic/store-sourcemap-string	Change
_app-HASH.js gzip	193 B	193 B	✓
_error-HASH.js gzip	193 B	193 B	✓
amp-HASH.js gzip	512 B	510 B	N/A
css-HASH.js gzip	343 B	342 B	N/A
dynamic-HASH.js gzip	1.84 kB	1.84 kB	✓
edge-ssr-HASH.js gzip	265 B	265 B	✓
head-HASH.js gzip	363 B	362 B	N/A
hooks-HASH.js gzip	393 B	392 B	N/A
image-HASH.js gzip	4.59 kB	4.58 kB	N/A
index-HASH.js gzip	268 B	268 B	✓
link-HASH.js gzip	2.35 kB	2.35 kB	N/A
routerDirect..HASH.js gzip	328 B	328 B	✓
script-HASH.js gzip	397 B	397 B	✓
withRouter-HASH.js gzip	323 B	326 B	N/A
1afbb74e6ecf..834.css gzip	106 B	106 B	✓
Overall change	3.59 kB	3.59 kB	✓

Client Build Manifests

	vercel/next.js canary	vercel/next.js mischnic/store-sourcemap-string	Change
_buildManifest.js gzip	748 B	747 B	N/A
Overall change	0 B	0 B	✓

Rendered Page Sizes

	vercel/next.js canary	vercel/next.js mischnic/store-sourcemap-string	Change
index.html gzip	524 B	524 B	✓
link.html gzip	540 B	537 B	N/A
withRouter.html gzip	521 B	520 B	N/A
Overall change	524 B	524 B	✓

Edge SSR bundle Size

	vercel/next.js canary	vercel/next.js mischnic/store-sourcemap-string	Change
edge-ssr.js gzip	130 kB	130 kB	N/A
page.js gzip	211 kB	211 kB	N/A
Overall change	0 B	0 B	✓

Middleware size

	vercel/next.js canary	vercel/next.js mischnic/store-sourcemap-string	Change
middleware-b..fest.js gzip	672 B	672 B	✓
middleware-r..fest.js gzip	155 B	156 B	N/A
middleware.js gzip	31.3 kB	31.3 kB	N/A
edge-runtime..pack.js gzip	844 B	844 B	✓
Overall change	1.52 kB	1.52 kB	✓

Next Runtimes

	vercel/next.js canary	vercel/next.js mischnic/store-sourcemap-string	Change
app-page-exp...dev.js gzip	393 kB	393 kB	✓
app-page-exp..prod.js gzip	132 kB	132 kB	✓
app-page-tur..prod.js gzip	145 kB	145 kB	✓
app-page-tur..prod.js gzip	141 kB	141 kB	✓
app-page.run...dev.js gzip	381 kB	381 kB	✓
app-page.run..prod.js gzip	129 kB	129 kB	✓
app-route-ex...dev.js gzip	39.3 kB	39.3 kB	✓
app-route-ex..prod.js gzip	25.6 kB	25.6 kB	✓
app-route-tu..prod.js gzip	25.6 kB	25.6 kB	✓
app-route-tu..prod.js gzip	25.4 kB	25.4 kB	✓
app-route.ru...dev.js gzip	40.9 kB	40.9 kB	✓
app-route.ru..prod.js gzip	25.4 kB	25.4 kB	✓
dist_client_...dev.js gzip	356 B	356 B	✓
dist_client_...dev.js gzip	349 B	349 B	✓
pages-api-tu..prod.js gzip	9.69 kB	9.69 kB	✓
pages-api.ru...dev.js gzip	11.8 kB	11.8 kB	✓
pages-api.ru..prod.js gzip	9.68 kB	9.68 kB	✓
pages-turbo...prod.js gzip	21.9 kB	21.9 kB	✓
pages.runtim...dev.js gzip	31.5 kB	31.5 kB	✓
pages.runtim..prod.js gzip	21.9 kB	21.9 kB	✓
server.runti..prod.js gzip	61 kB	61 kB	✓
Overall change	1.67 MB	1.67 MB	✓

build cache

	vercel/next.js canary	vercel/next.js mischnic/store-sourcemap-string	Change
0.pack gzip	2.11 MB	2.11 MB	N/A
index.pack gzip	76.4 kB	76 kB	N/A
Overall change	0 B	0 B	✓

Diff details

Diff for main-HASH.js

Diff too large to display

Commit: 2733c73

sokra · 2025-02-11T22:15:08Z

turbopack/crates/turbopack-core/src/code_builder.rs

 };

 /// A mapping of byte-offset in the code string to an associated source map.
-pub type Mapping = (usize, Option<ResolvedVc<Box<dyn GenerateSourceMap>>>);
+pub type Mapping = (usize, Option<Rope>);


Suggested change

pub type Mapping = (usize, Option<Rope>);

pub type Mapping = (usize, Option<ResolvedVc<Rope>>);

It would be better if we keep these Ropes in Vcs for better persistent caching. Otherwise it duplicates the data when serialized

sokra · 2025-02-11T22:31:53Z

turbopack/crates/turbopack-core/src/source_map/utils.rs

+#[derive(Serialize, Deserialize)]
+#[serde(rename_all = "camelCase")]
+struct SourceMapJson {
+    version: u32,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    file: Option<String>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    source_root: Option<String>,
+    // Technically a required field, but we don't want to error here.
+    #[serde(skip_serializing_if = "Option::is_none")]
+    sources: Option<Vec<Option<String>>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    sources_content: Option<Vec<Option<String>>>,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    names: Option<Vec<String>>,
+    mappings: String,
+    #[serde(skip_serializing_if = "Option::is_none")]
+    ignore_list: Option<Vec<u32>>,
+
+    // A somewhat widespread non-standard extension
+    debug_id: Option<String>,
+
+    #[serde(skip_serializing_if = "Option::is_none")]
+    sections: Option<Vec<SourceMapSectionItemJson>>,
+}


Instead of demanding this strict structure from the source map, you could deserialize it as serde_json::Value and only pick the fields you are interested in and deserialize them with serde_json::from_value.

sokra · 2025-02-11T22:37:14Z

turbopack/crates/turbopack-css/src/chunk/mod.rs

@@ -391,7 +389,7 @@ pub struct CssChunkItemContent {
    pub import_context: Option<ResolvedVc<ImportContext>>,
    pub imports: Vec<CssImport>,
    pub inner_code: Rope,
-    pub source_map: Option<ResolvedVc<ParseCssResultSourceMap>>,
+    pub source_map: Option<Rope>,


Maybe store that as ResolvedVc too?

sokra

Sounds fine to merge and comments could be a follow up PR

eps1lon · 2025-02-12T10:00:24Z

Seems like this fixed ignoreList propagation (e.g. when an input file had an existing sourcemap that ignore-listed that file). We only recently added a test that was now updated: #75954

#75791 made the sourcemap handling stricter, it caused this when encountering invalid input source maps (i.e. the ones that are published to npm). Let's keep the old behavior for now (which is also what Webpack does). ``` Caused by: 0: Execution of run_inner_operation failed 1: Execution of run_test_operation failed 2: Execution of ModuleGraph::from_modules failed 3: Execution of SingleModuleGraph::new_with_entries failed 4: [project]/turbopack/crates/turbopack-tests/tests/snapshot/source_maps/invalid/input/index.js [test] (ecmascript) 5: Execution of primary_chunkable_referenced_modules failed 6: Execution of <EcmascriptModuleAsset as Module>::references failed 7: Execution of analyse_ecmascript_module failed 8: failed to analyse ecmascript module '[project]/turbopack/crates/turbopack-tests/tests/snapshot/source_maps/invalid/input/index.js [test] (ecmascript)' 9: Execution of <SourceMapReference as GenerateSourceMap>::generate_source_map failed 10: invalid type: string "use client", expected struct SourceMapJson at line 1 column 12 ```

ijjk added created-by: Turbopack team PRs by the Turbopack team. Turbopack Related to Turbopack with Next.js. labels Feb 7, 2025

mischnic changed the title ~~wip~~ Turbopack: store sourcemaps serialized Feb 7, 2025

mischnic changed the title ~~Turbopack: store sourcemaps serialized~~ Turbopack: store serialized sourcemaps instead Feb 7, 2025

mischnic force-pushed the mischnic/store-sourcemap-string branch 6 times, most recently from 8053c55 to cd1830a Compare February 11, 2025 09:11

ijjk added the tests label Feb 11, 2025

mischnic requested a review from sokra February 11, 2025 15:38

mischnic marked this pull request as ready for review February 11, 2025 15:38

mischnic force-pushed the mischnic/store-sourcemap-string branch from 541aaf9 to e21cf0f Compare February 11, 2025 17:28

mischnic added 12 commits February 11, 2025 20:27

wip

2724fca

wip

9103b20

wip

fce6b5a

wip

5035e3a

wip

2663a0a

wip

f7b29b7

move fileify

ef8750c

resolve_source_map_sources

cae6c5d

fixups

e1bd363

cleanup

4e91db8

flatten sourcemaps for swc

0431a86

cleanup

3c0cba5

mischnic added 10 commits February 11, 2025 20:30

Fix lookup_token_and_source

91f57e1

cleanup

8f979fc

Not sure where that compile error comes from

d3cb78c

More permissive sourcemap parsing

1785fc4

That behavior actually correct now

a7ca213

cleanup

7e74915

concat ropes without parsing

3a92526

SourceMap::new_from_rope_cached

d4ddcdc

Fixup

1d00d4b

cleanup

2733c73

mischnic force-pushed the mischnic/store-sourcemap-string branch from a0e6698 to 2733c73 Compare February 11, 2025 19:32

sokra reviewed Feb 11, 2025

View reviewed changes

sokra approved these changes Feb 11, 2025

View reviewed changes

mischnic merged commit 58b627c into canary Feb 12, 2025
131 checks passed

mischnic deleted the mischnic/store-sourcemap-string branch February 12, 2025 07:49

eps1lon mentioned this pull request Feb 12, 2025

Update sourcemap tests due to recent Turbopack changes #75954

Merged

mischnic mentioned this pull request Feb 19, 2025

Turbopack: ignore invalid sourcemaps for now #76215

Merged

github-actions bot added the locked label Feb 26, 2025

github-actions bot locked as resolved and limited conversation to collaborators Feb 26, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Turbopack: store serialized sourcemaps instead #75791

Turbopack: store serialized sourcemaps instead #75791

mischnic commented Feb 7, 2025 •

edited by padmaia

Loading

mischnic commented Feb 7, 2025

ijjk commented Feb 7, 2025

ijjk commented Feb 7, 2025 •

edited

Loading

ijjk commented Feb 7, 2025 •

edited

Loading

sokra Feb 11, 2025

sokra Feb 11, 2025

sokra Feb 11, 2025

sokra left a comment

eps1lon commented Feb 12, 2025

	pub type Mapping = (usize, Option<Rope>);
	pub type Mapping = (usize, Option<ResolvedVc<Rope>>);

Turbopack: store serialized sourcemaps instead #75791

Turbopack: store serialized sourcemaps instead #75791

Conversation

mischnic commented Feb 7, 2025 • edited by padmaia Loading

mischnic commented Feb 7, 2025

ijjk commented Feb 7, 2025

Failing test suites

ijjk commented Feb 7, 2025 • edited Loading

Tests Passed

ijjk commented Feb 7, 2025 • edited Loading

Stats from current PR

sokra Feb 11, 2025

Choose a reason for hiding this comment

sokra Feb 11, 2025

Choose a reason for hiding this comment

sokra Feb 11, 2025

Choose a reason for hiding this comment

sokra left a comment

Choose a reason for hiding this comment

eps1lon commented Feb 12, 2025

mischnic commented Feb 7, 2025 •

edited by padmaia

Loading

ijjk commented Feb 7, 2025 •

edited

Loading

ijjk commented Feb 7, 2025 •

edited

Loading