Skip to content
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

Use turbo for packing files in test setup #44074

Merged
merged 15 commits into from Dec 22, 2022

Conversation

jankaifer
Copy link
Contributor

There was an issue with turbo not caching properly because package.json is always changing.
There are multiple workarounds like:

  • create a .tgz with all dependencies included
  • copy all those .tgz files to a constant persistent storage

I chose the second option and I'm copying those files back to next.js repo.

@jankaifer
Copy link
Contributor Author

Brb, that turbo code is in GH action so we need to get turbo into github actions.
That will require some ncc magic but I'm not sure how to go about it.

We could probably run it in the pipeline without a turbo since there will be no cache anyway.
We will have different code running on local and in CI, but It might be ok?

WDYT @timneutkens ?

@jankaifer
Copy link
Contributor Author

speed when cached, the average is not great but it decreases maximum significantly:

createNext 4.7s
├─ init next dev instance 53 µs
├─ createTestDir 4.5s
│  ├─ createNextInstall 4.5s
│  │  ├─  enruse swc binary 5.1 ms
│  │  ├─ copy package.json to temp dir 1.5 ms
│  │  ├─ copy packages to temp dir 489 ms
│  │  ├─ linkPackages 734 ms
│  │  │  ├─ prepare packages for packing 38 ms
│  │  │  └─ packing packages 696 ms
│  │  │     ├─ pack create-next-app 604 ms
│  │  │     ├─ pack eslint-config-next 584 ms
│  │  │     ├─ pack @next/eslint-plugin-next 554 ms
│  │  │     ├─ pack @next/font 637 ms
│  │  │     ├─ pack next 632 ms
│  │  │     ├─ pack @next/bundle-analyzer 598 ms
│  │  │     ├─ pack @next/codemod 658 ms
│  │  │     ├─ pack @next/env 594 ms
│  │  │     ├─ pack @next/mdx 641 ms
│  │  │     ├─ pack @next/plugin-storybook 619 ms
│  │  │     ├─ pack @next/polyfill-module 594 ms
│  │  │     ├─ pack @next/polyfill-nomodule 597 ms
│  │  │     ├─ pack @next/swc 593 ms
│  │  │     ├─ pack @next/react-dev-overlay 604 ms
│  │  │     └─ pack @next/react-refresh-utils 615 ms
│  │  └─ run generic install command 3.2s
│  └─ writeInitialFiles 1.1 ms
└─ start next instance 256 ms

@ijjk
Copy link
Member

ijjk commented Dec 20, 2022

Stats from current PR

Default Build (Decrease detected ✓)
General Overall increase ⚠️
vercel/next.js canary JanKaifer/next.js add-turbo-powered-packing Change
buildDuration 34.1s 34.8s ⚠️ +656ms
buildDurationCached 14.5s 10s -4.5s
nodeModulesSize 95 MB 95 MB
nextStartRea..uration (ms) 279ms 283ms ⚠️ +4ms
Client Bundles (main, webpack)
vercel/next.js canary JanKaifer/next.js add-turbo-powered-packing Change
152-HASH.js gzip 64.5 kB 64.5 kB
53.HASH.js gzip 182 B 182 B
main-app-HASH.js gzip 205 B 205 B
main-HASH.js gzip 79.2 kB 79.2 kB
webpack-HASH.js gzip 1.7 kB 1.7 kB
Overall change 146 kB 146 kB
Legacy Client Bundles (polyfills)
vercel/next.js canary JanKaifer/next.js add-turbo-powered-packing Change
polyfills-HASH.js gzip 31 kB 31 kB
Overall change 31 kB 31 kB
Client Pages
vercel/next.js canary JanKaifer/next.js add-turbo-powered-packing Change
_app-HASH.js gzip 192 B 192 B
_error-HASH.js gzip 179 B 179 B
amp-HASH.js gzip 484 B 484 B
css-HASH.js gzip 804 B 804 B
dynamic-HASH.js gzip 2.29 kB 2.29 kB
edge-ssr-HASH.js gzip 255 B 255 B
head-HASH.js gzip 827 B 827 B
hooks-HASH.js gzip 848 B 848 B
image-HASH.js gzip 4.29 kB 4.29 kB
index-HASH.js gzip 252 B 252 B
link-HASH.js gzip 2.69 kB 2.69 kB
routerDirect..HASH.js gzip 782 B 782 B
script-HASH.js gzip 857 B 857 B
withRouter-HASH.js gzip 781 B 781 B
85e02e95b279..7e3.css gzip 107 B 107 B
Overall change 15.6 kB 15.6 kB
Client Build Manifests
vercel/next.js canary JanKaifer/next.js add-turbo-powered-packing Change
_buildManifest.js gzip 482 B 482 B
Overall change 482 B 482 B
Rendered Page Sizes
vercel/next.js canary JanKaifer/next.js add-turbo-powered-packing Change
index.html gzip 489 B 489 B
link.html gzip 506 B 506 B
withRouter.html gzip 485 B 485 B
Overall change 1.48 kB 1.48 kB
Edge SSR bundle Size
vercel/next.js canary JanKaifer/next.js add-turbo-powered-packing Change
edge-ssr.js gzip 110 kB 110 kB
page.js gzip 98.4 kB 98.4 kB
Overall change 209 kB 209 kB
Middleware size Overall decrease ✓
vercel/next.js canary JanKaifer/next.js add-turbo-powered-packing Change
middleware-b..fest.js gzip 586 B 584 B -2 B
middleware-r..fest.js gzip 145 B 145 B
middleware.js gzip 27 kB 27 kB
edge-runtime..pack.js gzip 1.83 kB 1.83 kB
Overall change 29.6 kB 29.6 kB -2 B

Diffs

Diff for page.js
@@ -8618,23 +8618,23 @@ Learn More: https://nextjs.org/docs/messages/node-module-in-edge-runtime`;
       /***/
     },
 
-    /***/ 1036: /***/ (
+    /***/ 1028: /***/ (
       __unused_webpack_module,
       __unused_webpack_exports,
       __webpack_require__
     ) => {
-      Promise.resolve(/* import() eager */).then(
-        __webpack_require__.bind(__webpack_require__, 1210)
-      );
       Promise.resolve(/* import() eager */).then(
         __webpack_require__.bind(__webpack_require__, 2973)
       );
       Promise.resolve(/* import() eager */).then(
-        __webpack_require__.bind(__webpack_require__, 6448)
+        __webpack_require__.bind(__webpack_require__, 1210)
       );
       Promise.resolve(/* import() eager */).then(
         __webpack_require__.bind(__webpack_require__, 9916)
       );
+      Promise.resolve(/* import() eager */).then(
+        __webpack_require__.bind(__webpack_require__, 6448)
+      );
 
       /***/
     },
Diff for middleware-b..-manifest.js
@@ -9,7 +9,7 @@ self.__BUILD_MANIFEST = {
   rootMainFiles: [
     "static/chunks/webpack-c452a3e31b73f504.js",
     "static/chunks/152-170afc3f7613f941.js",
-    "static/chunks/main-app-c9a0aaff35eafcb5.js"
+    "static/chunks/main-app-ef0948581cabd144.js"
   ],
   pages: {
     "/": [
Diff for main-app-HASH.js
@@ -1,23 +1,23 @@
 (self["webpackChunk_N_E"] = self["webpackChunk_N_E"] || []).push([
   [744],
   {
-    /***/ 9915: /***/ function(
+    /***/ 5764: /***/ function(
       __unused_webpack_module,
       __unused_webpack_exports,
       __webpack_require__
     ) {
-      Promise.resolve(/* import() eager */).then(
-        __webpack_require__.t.bind(__webpack_require__, 429, 23)
-      );
       Promise.resolve(/* import() eager */).then(
         __webpack_require__.t.bind(__webpack_require__, 1161, 23)
       );
       Promise.resolve(/* import() eager */).then(
-        __webpack_require__.t.bind(__webpack_require__, 8138, 23)
+        __webpack_require__.t.bind(__webpack_require__, 429, 23)
       );
       Promise.resolve(/* import() eager */).then(
         __webpack_require__.t.bind(__webpack_require__, 9379, 23)
       );
+      Promise.resolve(/* import() eager */).then(
+        __webpack_require__.t.bind(__webpack_require__, 8138, 23)
+      );
 
       /***/
     }
@@ -28,7 +28,7 @@
       return __webpack_require__((__webpack_require__.s = moduleId));
     };
     /******/ __webpack_require__.O(0, [152], function() {
-      return __webpack_exec__(7070), __webpack_exec__(9915);
+      return __webpack_exec__(7070), __webpack_exec__(5764);
     });
     /******/ var __webpack_exports__ = __webpack_require__.O();
     /******/ _N_E = __webpack_exports__;
Commit: db6b71b

@ijjk ijjk added the created-by: Next.js team PRs by the Next.js team label Dec 22, 2022
@jankaifer
Copy link
Contributor Author

@timneutkens the only action that is failing is PR stats but I think that is a flake - it's an issue with the playwright download timing out.

I'll merge canary but it should be good to go.

@timneutkens
Copy link
Member

All green! Great work Jan 🙏

@timneutkens timneutkens reopened this Dec 22, 2022
@timneutkens timneutkens merged commit fd9ec64 into vercel:canary Dec 22, 2022
@jankaifer
Copy link
Contributor Author

Cool this unblocks caching pnpm-lock.yaml for faster installs 🎉

jankaifer added a commit to jankaifer/next.js that referenced this pull request Jan 4, 2023
jankaifer added a commit to jankaifer/next.js that referenced this pull request Jan 4, 2023
jankaifer added a commit to jankaifer/next.js that referenced this pull request Jan 4, 2023
jankaifer added a commit to jankaifer/next.js that referenced this pull request Jan 4, 2023
ijjk pushed a commit that referenced this pull request Jan 4, 2023
This reverts commit fd9ec64.

It seems that there is an issue with caching of inputs which uses stale
files.

We also had to disable caching of `pnpm-lock.yaml` because it requires
stable package locations chich we removed.
jankaifer added a commit to jankaifer/next.js that referenced this pull request Jan 5, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jan 22, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
created-by: Next.js team PRs by the Next.js team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants