Skip to content

Commit da7cbb1

Browse files
lukesandberglukesandberg
authored
[turbopack] Use Arrays instead of objects to bootstrap chunks (vercel#81877)
### Improve performance of chunk bootstrapping leveraging arrays and maps Currently we bootstrap the set of the modules in a chunk by passing an object to the runtime, this is suboptimal for a few reasons: * The runtime just wants to iterate the object and copy it into a runtime datastructure and iterating objects is relatively slow * The browser needs to spend time computing the 'shape' of this object and possibly changing internal representations The solution to both these problems is of course, Arrays!. so instead of an object like ``` { [moduleId]: (__turbopack_context__) => {},...} ``` we use an array like this ``` [ moduleId, (__turbopack_context__) => {}] ``` * a non-empty sequence of module ids come first - we may have more than one to support scope hoisted modules * the factory function Because these objects are temporary this should make both constructing and iterating them faster. Finally, in the runtime what we want to do is copy these modules into a global map that is represented as an `Object`. Because we are constantly inserting new keys, this is a suboptimal case and a `Map` should be faster for both insertions and lookups. So this adapts the moduleFactory type to use an es6 `Map` which is more optimized for this particular usecase. Ideally we would do the same for the moduleCache but that would complicate `require.cache` usecases. We could perhaps leverage a `Proxy` to maintain support but this is left as future work. ### Aside: why are arrays faster than objects? Arrays are faster to parse because they incur fewer 'shape transitions' (background reading: https://v8.dev/blog/fast-properties vs https://v8.dev/blog/elements-kinds). Basically for every new property v8 parses on the object literal it changes the hidden type (or 'shape'). For arrays the same thing happens but the process is much simpler as there is a fixed set of possible shapes and the browser will hit the terminal one after evaluating the second element. Also arrays are simply faster to iterate than objects (but this is generally a non-controversial statement). ### Performance Based on the `module-cost` benchmark this is a consistent win in all scenarios | Scenario | Metric | HEAD (ms) | CHANGE (ms) | Delta (ms) | % Change | |----------|--------|-----------|-------------|------------|----------| | **Pages API ESM** | Load Duration | 33.46 | 27.87 | -5.59 | -16.7% ✅ | | | Execution Duration | 63.22 | 62.54 | -0.68 | -1.1% ✅ | | **Pages API CommonJS** | Load Duration | 44.11 | 33.76 | -10.35 | -23.5% ✅ | | | Execution Duration | 32.05 | 25.70 | -6.35 | -19.8% ✅ | | **Client ESM** | Load Duration | 34.74 | 27.65 | -7.09 | -20.4% ✅ | | | Execution Duration | 46.13 | 40.35 | -5.78 | -12.5% ✅ | | **Client CommonJS** | Load Duration | 30.43 | 24.48 | -5.95 | -19.6% ✅ | | | Execution Duration | 25.15 | 17.18 | -7.97 | -31.7% ✅ | ### Risks The biggest risk of this change is subtle bugs introduced due to `string` vs `number` confusion. I found one subtle issue in how we rewrite `new Worker()` constructors, there could be more not covered by tests.
1 parent 825753a commit da7cbb1

File tree

311 files changed

+1200
-1417
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

311 files changed

+1200
-1417
lines changed

‎test/development/app-dir/ssr-in-rsc/ssr-in-rsc.test.ts

Lines changed: 27 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -369,16 +369,16 @@ describe('react-dom/server in React Server environment', () => {
369369
`)
370370
} else {
371371
expect(redbox).toMatchInlineSnapshot(`
372-
{
373-
"description": "react-dom/server is not supported in React Server Components.",
374-
"source": "app/exports/app-code/react-dom-server-node-explicit/page.js (1:1) @ {module evaluation}
372+
{
373+
"description": "react-dom/server is not supported in React Server Components.",
374+
"source": "app/exports/app-code/react-dom-server-node-explicit/page.js (1:1) @ {module evaluation}
375375
376-
> 1 | import * as ReactDOMServerNode from 'react-dom/server.node'
377-
| ^
378-
2 | // Fine to drop once React is on ESM
379-
3 | import ReactDOMServerNodeDefault from 'react-dom/server.node'
380-
4 |",
381-
}
376+
> 1 | import * as ReactDOMServerNode from 'react-dom/server.node'
377+
| ^
378+
2 | // Fine to drop once React is on ESM
379+
3 | import ReactDOMServerNodeDefault from 'react-dom/server.node'
380+
4 |",
381+
}
382382
`)
383383
}
384384
} else {
@@ -509,16 +509,16 @@ describe('react-dom/server in React Server environment', () => {
509509
`)
510510
} else {
511511
expect(redbox).toMatchInlineSnapshot(`
512-
{
513-
"description": "react-dom/server is not supported in React Server Components.",
514-
"source": "internal-pkg/server.node.js (1:1) @ {module evaluation}
512+
{
513+
"description": "react-dom/server is not supported in React Server Components.",
514+
"source": "internal-pkg/server.node.js (1:1) @ {module evaluation}
515515
516-
> 1 | import * as ReactDOMServerEdge from 'react-dom/server.node'
517-
| ^
518-
2 | // Fine to drop once React is on ESM
519-
3 | import ReactDOMServerEdgeDefault from 'react-dom/server.node'
520-
4 |",
521-
}
516+
> 1 | import * as ReactDOMServerEdge from 'react-dom/server.node'
517+
| ^
518+
2 | // Fine to drop once React is on ESM
519+
3 | import ReactDOMServerEdgeDefault from 'react-dom/server.node'
520+
4 |",
521+
}
522522
`)
523523
}
524524
} else {
@@ -807,16 +807,16 @@ describe('react-dom/server in React Server environment', () => {
807807
`)
808808
} else {
809809
expect(redbox).toMatchInlineSnapshot(`
810-
{
811-
"description": "react-dom/server is not supported in React Server Components.",
812-
"source": "internal-pkg/server.node.js (1:1) @ {module evaluation}
810+
{
811+
"description": "react-dom/server is not supported in React Server Components.",
812+
"source": "internal-pkg/server.node.js (1:1) @ {module evaluation}
813813
814-
> 1 | import * as ReactDOMServerEdge from 'react-dom/server.node'
815-
| ^
816-
2 | // Fine to drop once React is on ESM
817-
3 | import ReactDOMServerEdgeDefault from 'react-dom/server.node'
818-
4 |",
819-
}
814+
> 1 | import * as ReactDOMServerEdge from 'react-dom/server.node'
815+
| ^
816+
2 | // Fine to drop once React is on ESM
817+
3 | import ReactDOMServerEdgeDefault from 'react-dom/server.node'
818+
4 |",
819+
}
820820
`)
821821
}
822822
} else {

‎test/development/pages-dir/client-navigation/index.test.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -29,14 +29,14 @@ describe('Client Navigation', () => {
2929
})
3030
await browser.elementByCss('#empty-props').click()
3131
await expect(browser).toDisplayRedbox(`
32-
{
33-
"description": ""EmptyInitialPropsPage.getInitialProps()" should resolve to an object. But found "null" instead.",
34-
"environmentLabel": null,
35-
"label": "Runtime Error",
36-
"source": null,
37-
"stack": [],
38-
}
39-
`)
32+
{
33+
"description": ""EmptyInitialPropsPage.getInitialProps()" should resolve to an object. But found "null" instead.",
34+
"environmentLabel": null,
35+
"label": "Runtime Error",
36+
"source": null,
37+
"stack": [],
38+
}
39+
`)
4040
expect(pageErrors).toEqual([
4141
expect.objectContaining({
4242
message:

‎test/e2e/app-dir/cache-components-errors/cache-components-errors.test.ts

Lines changed: 104 additions & 104 deletions
Original file line numberDiff line numberDiff line change
@@ -1115,60 +1115,60 @@ describe('Cache Components Errors', () => {
11151115

11161116
if (isTurbopack) {
11171117
await expect(browser).toDisplayRedbox(`
1118-
[
1119-
{
1120-
"description": "Route "/sync-cookies" used \`cookies().get\`. \`cookies()\` should be awaited before using its value. Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis",
1121-
"environmentLabel": "Prerender",
1122-
"label": "Console Error",
1123-
"source": "app/sync-cookies/page.tsx (17:26) @ CookiesReadingComponent
1124-
> 17 | const _token = (cookies() as unknown as UnsafeUnwrappedCookies).get('token')
1125-
| ^",
1126-
"stack": [
1127-
"CookiesReadingComponent app/sync-cookies/page.tsx (17:26)",
1128-
"Page app/sync-cookies/page.tsx (11:7)",
1129-
],
1130-
},
1131-
{
1132-
"description": "(0 , <turbopack-module-id>.cookies)(...).get is not a function",
1133-
"environmentLabel": "Prerender",
1134-
"label": "Runtime TypeError",
1135-
"source": "app/sync-cookies/page.tsx (17:67) @ CookiesReadingComponent
1136-
> 17 | const _token = (cookies() as unknown as UnsafeUnwrappedCookies).get('token')
1137-
| ^",
1138-
"stack": [
1139-
"CookiesReadingComponent app/sync-cookies/page.tsx (17:67)",
1140-
],
1141-
},
1142-
]
1143-
`)
1118+
[
1119+
{
1120+
"description": "Route "/sync-cookies" used \`cookies().get\`. \`cookies()\` should be awaited before using its value. Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis",
1121+
"environmentLabel": "Prerender",
1122+
"label": "Console Error",
1123+
"source": "app/sync-cookies/page.tsx (17:26) @ CookiesReadingComponent
1124+
> 17 | const _token = (cookies() as unknown as UnsafeUnwrappedCookies).get('token')
1125+
| ^",
1126+
"stack": [
1127+
"CookiesReadingComponent app/sync-cookies/page.tsx (17:26)",
1128+
"Page app/sync-cookies/page.tsx (11:7)",
1129+
],
1130+
},
1131+
{
1132+
"description": "(0 , <turbopack-module-id>.cookies)(...).get is not a function",
1133+
"environmentLabel": "Prerender",
1134+
"label": "Runtime TypeError",
1135+
"source": "app/sync-cookies/page.tsx (17:67) @ CookiesReadingComponent
1136+
> 17 | const _token = (cookies() as unknown as UnsafeUnwrappedCookies).get('token')
1137+
| ^",
1138+
"stack": [
1139+
"CookiesReadingComponent app/sync-cookies/page.tsx (17:67)",
1140+
],
1141+
},
1142+
]
1143+
`)
11441144
} else {
11451145
await expect(browser).toDisplayRedbox(`
1146-
[
1147-
{
1148-
"description": "Route "/sync-cookies" used \`cookies().get\`. \`cookies()\` should be awaited before using its value. Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis",
1149-
"environmentLabel": "Prerender",
1150-
"label": "Console Error",
1151-
"source": "app/sync-cookies/page.tsx (17:18) @ CookiesReadingComponent
1152-
> 17 | const _token = (cookies() as unknown as UnsafeUnwrappedCookies).get('token')
1153-
| ^",
1154-
"stack": [
1155-
"CookiesReadingComponent app/sync-cookies/page.tsx (17:18)",
1156-
"Page app/sync-cookies/page.tsx (11:7)",
1157-
],
1158-
},
1159-
{
1160-
"description": "(0 , <webpack-module-id>.cookies)(...).get is not a function",
1161-
"environmentLabel": "Prerender",
1162-
"label": "Runtime TypeError",
1163-
"source": "app/sync-cookies/page.tsx (17:67) @ CookiesReadingComponent
1164-
> 17 | const _token = (cookies() as unknown as UnsafeUnwrappedCookies).get('token')
1165-
| ^",
1166-
"stack": [
1167-
"CookiesReadingComponent app/sync-cookies/page.tsx (17:67)",
1168-
],
1169-
},
1170-
]
1171-
`)
1146+
[
1147+
{
1148+
"description": "Route "/sync-cookies" used \`cookies().get\`. \`cookies()\` should be awaited before using its value. Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis",
1149+
"environmentLabel": "Prerender",
1150+
"label": "Console Error",
1151+
"source": "app/sync-cookies/page.tsx (17:18) @ CookiesReadingComponent
1152+
> 17 | const _token = (cookies() as unknown as UnsafeUnwrappedCookies).get('token')
1153+
| ^",
1154+
"stack": [
1155+
"CookiesReadingComponent app/sync-cookies/page.tsx (17:18)",
1156+
"Page app/sync-cookies/page.tsx (11:7)",
1157+
],
1158+
},
1159+
{
1160+
"description": "(0 , <webpack-module-id>.cookies)(...).get is not a function",
1161+
"environmentLabel": "Prerender",
1162+
"label": "Runtime TypeError",
1163+
"source": "app/sync-cookies/page.tsx (17:67) @ CookiesReadingComponent
1164+
> 17 | const _token = (cookies() as unknown as UnsafeUnwrappedCookies).get('token')
1165+
| ^",
1166+
"stack": [
1167+
"CookiesReadingComponent app/sync-cookies/page.tsx (17:67)",
1168+
],
1169+
},
1170+
]
1171+
`)
11721172
}
11731173
})
11741174
} else {
@@ -1340,60 +1340,60 @@ describe('Cache Components Errors', () => {
13401340

13411341
if (isTurbopack) {
13421342
await expect(browser).toDisplayRedbox(`
1343-
[
1344-
{
1345-
"description": "Route "/sync-headers" used \`headers().get\`. \`headers()\` should be awaited before using its value. Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis",
1346-
"environmentLabel": "Prerender",
1347-
"label": "Console Error",
1348-
"source": "app/sync-headers/page.tsx (17:29) @ HeadersReadingComponent
1349-
> 17 | const userAgent = (headers() as unknown as UnsafeUnwrappedHeaders).get(
1350-
| ^",
1351-
"stack": [
1352-
"HeadersReadingComponent app/sync-headers/page.tsx (17:29)",
1353-
"Page app/sync-headers/page.tsx (11:7)",
1354-
],
1355-
},
1356-
{
1357-
"description": "(0 , <turbopack-module-id>.headers)(...).get is not a function",
1358-
"environmentLabel": "Prerender",
1359-
"label": "Runtime TypeError",
1360-
"source": "app/sync-headers/page.tsx (17:70) @ HeadersReadingComponent
1361-
> 17 | const userAgent = (headers() as unknown as UnsafeUnwrappedHeaders).get(
1362-
| ^",
1363-
"stack": [
1364-
"HeadersReadingComponent app/sync-headers/page.tsx (17:70)",
1365-
],
1366-
},
1367-
]
1368-
`)
1343+
[
1344+
{
1345+
"description": "Route "/sync-headers" used \`headers().get\`. \`headers()\` should be awaited before using its value. Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis",
1346+
"environmentLabel": "Prerender",
1347+
"label": "Console Error",
1348+
"source": "app/sync-headers/page.tsx (17:29) @ HeadersReadingComponent
1349+
> 17 | const userAgent = (headers() as unknown as UnsafeUnwrappedHeaders).get(
1350+
| ^",
1351+
"stack": [
1352+
"HeadersReadingComponent app/sync-headers/page.tsx (17:29)",
1353+
"Page app/sync-headers/page.tsx (11:7)",
1354+
],
1355+
},
1356+
{
1357+
"description": "(0 , <turbopack-module-id>.headers)(...).get is not a function",
1358+
"environmentLabel": "Prerender",
1359+
"label": "Runtime TypeError",
1360+
"source": "app/sync-headers/page.tsx (17:70) @ HeadersReadingComponent
1361+
> 17 | const userAgent = (headers() as unknown as UnsafeUnwrappedHeaders).get(
1362+
| ^",
1363+
"stack": [
1364+
"HeadersReadingComponent app/sync-headers/page.tsx (17:70)",
1365+
],
1366+
},
1367+
]
1368+
`)
13691369
} else {
13701370
await expect(browser).toDisplayRedbox(`
1371-
[
1372-
{
1373-
"description": "Route "/sync-headers" used \`headers().get\`. \`headers()\` should be awaited before using its value. Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis",
1374-
"environmentLabel": "Prerender",
1375-
"label": "Console Error",
1376-
"source": "app/sync-headers/page.tsx (17:21) @ HeadersReadingComponent
1377-
> 17 | const userAgent = (headers() as unknown as UnsafeUnwrappedHeaders).get(
1378-
| ^",
1379-
"stack": [
1380-
"HeadersReadingComponent app/sync-headers/page.tsx (17:21)",
1381-
"Page app/sync-headers/page.tsx (11:7)",
1382-
],
1383-
},
1384-
{
1385-
"description": "(0 , <webpack-module-id>.headers)(...).get is not a function",
1386-
"environmentLabel": "Prerender",
1387-
"label": "Runtime TypeError",
1388-
"source": "app/sync-headers/page.tsx (17:70) @ HeadersReadingComponent
1389-
> 17 | const userAgent = (headers() as unknown as UnsafeUnwrappedHeaders).get(
1390-
| ^",
1391-
"stack": [
1392-
"HeadersReadingComponent app/sync-headers/page.tsx (17:70)",
1393-
],
1394-
},
1395-
]
1396-
`)
1371+
[
1372+
{
1373+
"description": "Route "/sync-headers" used \`headers().get\`. \`headers()\` should be awaited before using its value. Learn more: https://nextjs.org/docs/messages/sync-dynamic-apis",
1374+
"environmentLabel": "Prerender",
1375+
"label": "Console Error",
1376+
"source": "app/sync-headers/page.tsx (17:21) @ HeadersReadingComponent
1377+
> 17 | const userAgent = (headers() as unknown as UnsafeUnwrappedHeaders).get(
1378+
| ^",
1379+
"stack": [
1380+
"HeadersReadingComponent app/sync-headers/page.tsx (17:21)",
1381+
"Page app/sync-headers/page.tsx (11:7)",
1382+
],
1383+
},
1384+
{
1385+
"description": "(0 , <webpack-module-id>.headers)(...).get is not a function",
1386+
"environmentLabel": "Prerender",
1387+
"label": "Runtime TypeError",
1388+
"source": "app/sync-headers/page.tsx (17:70) @ HeadersReadingComponent
1389+
> 17 | const userAgent = (headers() as unknown as UnsafeUnwrappedHeaders).get(
1390+
| ^",
1391+
"stack": [
1392+
"HeadersReadingComponent app/sync-headers/page.tsx (17:70)",
1393+
],
1394+
},
1395+
]
1396+
`)
13971397
}
13981398
})
13991399
} else {

‎test/e2e/app-dir/use-cache-hanging-inputs/use-cache-hanging-inputs.test.ts

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -274,15 +274,15 @@ describe('use-cache-hanging-inputs', () => {
274274

275275
if (isTurbopack) {
276276
expect(errorSource).toMatchInlineSnapshot(`
277-
"app/bound-args/page.tsx (13:15) @ {module evaluation}
277+
"app/bound-args/page.tsx (13:15) @ {module evaluation}
278278
279-
11 | const uncachedDataPromise = fetchUncachedData()
280-
12 |
281-
> 13 | const Foo = async () => {
282-
| ^
283-
14 | 'use cache'
284-
15 |
285-
16 | return ("
279+
11 | const uncachedDataPromise = fetchUncachedData()
280+
12 |
281+
> 13 | const Foo = async () => {
282+
| ^
283+
14 | 'use cache'
284+
15 |
285+
16 | return ("
286286
`)
287287

288288
expect(cliOutput).toContain(`Error: ${expectedTimeoutErrorMessage}

‎turbopack/crates/turbopack-browser/src/ecmascript/content.rs

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ use std::io::Write;
22

33
use anyhow::{Result, bail};
44
use either::Either;
5-
use indoc::writedoc;
65
use turbo_rcstr::RcStr;
76
use turbo_tasks::{ResolvedVc, Vc};
87
use turbo_tasks_fs::{File, rope::RopeBuilder};
@@ -106,24 +105,24 @@ impl EcmascriptBrowserChunkContent {
106105
// When the runtime executes (see the `evaluate` module), it will pick up and
107106
// register all pending chunks, and replace the list of pending chunks
108107
// with itself so later chunks can register directly with it.
109-
writedoc!(
108+
write!(
110109
code,
111-
r#"
112-
(globalThis.TURBOPACK = globalThis.TURBOPACK || []).push([{script_or_path}, {{
113-
"#
110+
// `||=` would be better but we need to be es2020 compatible
111+
//`x || (x = default)` is better than `x = x || default` simply because we avoid _writing_ the property in the common case.
112+
"(globalThis.TURBOPACK || (globalThis.TURBOPACK = [])).push([{script_or_path},"
114113
)?;
115114

116115
let content = this.content.await?;
117116
let chunk_items = content.chunk_item_code_and_ids().await?;
118117
for item in chunk_items {
119118
for (id, item_code) in item {
120-
write!(code, "\n{}: ", StringifyJs(&id))?;
119+
write!(code, "\n{}, ", StringifyJs(&id))?;
121120
code.push_code(item_code);
122121
write!(code, ",")?;
123122
}
124123
}
125124

126-
write!(code, "\n}}]);")?;
125+
write!(code, "\n]);")?;
127126

128127
let mut code = code.build();
129128

0 commit comments

Comments
 (0)