vanilla-extract-css / vanilla-extract

Zero-runtime Stylesheets-in-TypeScript
https://vanilla-extract.style
MIT License
9.64k stars 295 forks source link

[webpack.cache.PackFileCacheStrategy] Serializing big strings (110kiB) impacts deserialization performance #374

Open moroshko opened 3 years ago

moroshko commented 3 years ago

Describe the bug

Strangely enough, adding more sprinkles (see this commit) causing the following warning to appear in Node's console:

<w> [webpack.cache.PackFileCacheStrategy] Serializing big strings (110kiB) impacts deserialization 
performance (consider using Buffer instead and decode when needed)

Link to reproduction

  1. git clone https://github.com/moroshko/webpack-cache-issue.git
  2. cd webpack-cache-issue
  3. yarn
  4. yarn dev
  5. Observe the warning

Note: You might need to rm -rf .next to see the warning.

System Info

  System:
    OS: macOS 11.5.2
    CPU: (12) x64 Intel(R) Core(TM) i7-9750H CPU @ 2.60GHz
    Memory: 558.97 MB / 16.00 GB
    Shell: 5.8 - /bin/zsh
  Binaries:
    Node: 14.17.6 - ~/.nvm/versions/node/v14.17.6/bin/node
    Yarn: 1.22.11 - ~/.nvm/versions/node/v14.17.6/bin/yarn
    npm: 6.14.15 - ~/.nvm/versions/node/v14.17.6/bin/npm
  Browsers:
    Chrome: 93.0.4577.82
    Firefox: 92.0
    Safari: 14.1.2
  npmPackages:
    @vanilla-extract/babel-plugin: 1.1.0 => 1.1.0 
    @vanilla-extract/css: 1.6.0 => 1.6.0 
    @vanilla-extract/next-plugin: 1.0.0 => 1.0.0 
    @vanilla-extract/recipes: 0.1.0 => 0.1.0 
    @vanilla-extract/sprinkles: 1.3.0 => 1.3.0 
moroshko commented 3 years ago

Added an issue in the next repo, as it feels the warning is coming from there. https://github.com/vercel/next.js/issues/29239

mattcompiles commented 3 years ago

I'm assuming this is due to the fact that the whole CSS file gets serialised into the filename hash. Guess we could look into compression when the files grow large enough to keep the filename length down. 🤔

moroshko commented 2 years ago

@mattcompiles Looks like quite a few people are hitting this warning. Any chance to look into it any time soon?

JCMais commented 2 years ago

I've some CSS files with vanilla-extract, and the build log is now full with warnings like this one.

talentlessguy commented 2 years ago

any updates?

ffleandro commented 2 years ago

Any update on this? Hitting this issue on a next.js 12.1 app with 10k+ pages

nayaabkhan commented 2 years ago

I'm having a great time using vanilla-extract and interested in getting this fixed. So I started digging into the source to contribute a fix across.

I've been able to work around this warning by compressing the sources, as @mattcompiles suggested, before they're used by the virtual-resource-loader.

Here's the general approach:

  1. Compress the css before base64 encoding it: https://github.com/seek-oss/vanilla-extract/blob/master/packages/integration/src/processVanillaFile.ts#L112
  2. Decompress the source after base64 decoding it: https://github.com/seek-oss/vanilla-extract/blob/master/packages/integration/src/virtualFile.ts#L10
  3. Repeat the same for the virtual loader: https://github.com/seek-oss/vocab/blob/master/packages/virtual-resource-loader/src/index.ts#L6

ℹ I've left out an improvement to compress only above a certain threshold. If required, I'll include it in the final PR.

Here are the two patch files that you can use with patch-package to fix this locally. Please give it a try and let me know if it fixes the problem for you and how is the performance. Use the zip attached below for downloading the patch files.

🗄 patches.zip

Here's a preview of the files.

patches/@vanilla-extract+integration+2.0.1.patch:

diff --git a/node_modules/@vanilla-extract/integration/dist/vanilla-extract-integration.cjs.dev.js b/node_modules/@vanilla-extract/integration/dist/vanilla-extract-integration.cjs.dev.js
index de0087b..e117205 100644
--- a/node_modules/@vanilla-extract/integration/dist/vanilla-extract-integration.cjs.dev.js
+++ b/node_modules/@vanilla-extract/integration/dist/vanilla-extract-integration.cjs.dev.js
@@ -12,6 +12,7 @@ var path = require('path');
 var findUp = require('find-up');
 var fs = require('fs');
 var esbuild = require('esbuild');
+var zlib = require('zlib');

 function _interopDefault (e) { return e && e.__esModule ? e : { 'default': e }; }

@@ -96,7 +97,10 @@ async function processVanillaFile({
       composedClassLists,
       cssObjs: fileScopeCss
     }).join('\n');
-    const base64Source = Buffer.from(css, 'utf-8').toString('base64');
+
+    const compressedCSS = zlib.gzipSync(css);
+    const base64Source = compressedCSS.toString('base64');
+
     const fileName = `${fileScope.packageName ? `${fileScope.packageName}/${fileScope.filePath}` : fileScope.filePath}.vanilla.css`;
     let virtualCssFilePath;

@@ -202,9 +206,11 @@ function getSourceFromVirtualCssFile(id) {
     throw new Error('No source in vanilla CSS file');
   }

+  let uncompSource = zlib.gunzipSync(Buffer.from(match.groups.source, 'base64'))
+
   return {
     fileName: match.groups.fileName,
-    source: Buffer.from(match.groups.source, 'base64').toString('utf-8')
+    source: uncompSource.toString('utf-8')
   };
 }

patches/virtual-resource-loader+1.0.0.patch

diff --git a/node_modules/virtual-resource-loader/dist/virtual-resource-loader.cjs.dev.js b/node_modules/virtual-resource-loader/dist/virtual-resource-loader.cjs.dev.js
index e1a987c..ceb2a36 100644
--- a/node_modules/virtual-resource-loader/dist/virtual-resource-loader.cjs.dev.js
+++ b/node_modules/virtual-resource-loader/dist/virtual-resource-loader.cjs.dev.js
@@ -3,12 +3,14 @@
 Object.defineProperty(exports, '__esModule', { value: true });

 var loaderUtils = require('loader-utils');
+var zlib = require('zlib');

 function virtualResourceLoader() {
   const {
     source
   } = loaderUtils.getOptions(this);
-  return Buffer.from(source, 'base64').toString('utf-8');
+  let uncompSource = zlib.gunzipSync(Buffer.from(source, 'base64'));
+  return uncompSource.toString('utf-8');
 }

 exports.default = virtualResourceLoader;

Once someone from the team can confirm this is the right solution, I'd send over the PRs.

Cheers!

mattcompiles commented 2 years ago

@nayaabkhan Thanks for taking a look at this. Is compression resolving the issue for you? My main concern is compression just makes the issue take longer to appear and then we're also paying a perf cost to compress as well. Are you seeing large reductions in size of the final base64 string?

Also, if we did add compression then we'd probably need to drop the virtual-resource-loader dep and move that code internally to allow for the compression handling.

nayaabkhan commented 2 years ago

Is compression resolving the issue for you?

If we consider the warnings to be the issue, then yes. I no longer see the issue for the same set of atoms I generate (the majority of our CSS comes from the atoms).

However, the warnings would re-appear after the compressed CSS grows beyond 102400 bytes (Search for "Serializing big strings"). At which point, it might be time to inspect why would your setup generate 100 kiB of compressed CSS.


Are you seeing large reductions in the size of the final base64 string?

In terms of size

A new hot update index.hash.hot-update.js (these have the base64 strings for all modules used) after touching the sprinkles file is:

So in terms of the size of hot-updates, we get a 5x improvement.

In terms of time

The time it takes to compile the client is (average of ~10 runs changing the atoms file and taking time from the NextJS logs):

I find it strange that the compressed version performs faster. Perhaps this compression helps the bundler pipeline down the stream because of the shortened strings?

My atoms setup

Here's my sprinkles file for reference, I could not share the whole repo as it is private. atoms.css.ts.zip

Stats using https://www.projectwallace.com/analyze-css:


if we did add compression then we'd probably need to drop the virtual-resource-loader dep and move that code internally to allow for the compression handling

Agreed, I see the base64 set up inside vanilla integration and this loader have a co-relation in the code. But they live in a separate repo. Even a colocated setup would work better.

mattcompiles commented 2 years ago

@nayaabkhan Thanks for the detailed response, your solution sounds promising. Feel free to send a PR through if you're keen. If you get stuck on anything feel free to open the PR early and ask questions there.

nayaabkhan commented 2 years ago

Sure, I will send over a PR in a few days. Currently have hands full at work.

Meanwhile it’d be great if the requesters could help me test the patches.

cc @moroshko @JCMais @talentlessguy @ffleandro

JCMais commented 2 years ago

I will definitely test it this week, thanks for working on this @nayaabkhan!

moroshko commented 2 years ago

@nayaabkhan I can confirm that the patches make the warnings to disappear. Thanks for taking the lead on this!

JCMais commented 2 years ago

Everything is working great, thanks for all your work @nayaabkhan!

talentlessguy commented 2 years ago

it's still happening on latest vanilla-extract with next.js:

<w> [webpack.cache.PackFileCacheStrategy] Serializing big strings (102kiB) impacts deserialization performance (consider using Buffer instead and decode when needed)
<w> [webpack.cache.PackFileCacheStrategy] Serializing big strings (102kiB) impacts deserialization performance (consider using Buffer instead and decode when needed)
<w> [webpack.cache.PackFileCacheStrategy] Serializing big strings (102kiB) impacts deserialization performance (consider using Buffer instead and decode when needed)
wait  - compiling...
event - compiled client and server successfully in 779 ms (1280 modules)
wait  - compiling /asset/[contractAddress]/[tokenId] (client and server)...
event - compiled client and server successfully in 540 ms (1293 modules)
wait  - compiling...
event - compiled client and server successfully in 436 ms (1280 modules)

deps:

"@vanilla-extract/babel-plugin": "^1.1.5",
"@vanilla-extract/css": "^1.6.8",
"@vanilla-extract/css-utils": "^0.1.2",
"@vanilla-extract/dynamic": "^2.0.2",
"@vanilla-extract/next-plugin": "^2.0.2",
"@vanilla-extract/sprinkles": "^1.4.0",
"next": "12.1.4"

node version: 16.14.2

hiradary commented 2 years ago

Same here, It's not fixed.

callumflack commented 2 years ago

Can this be re-opened please? V-E is the answer to all my woes but rethinking it due to this constraint with Next.js

josephcsoti commented 2 years ago

Just did some testing with my team and it looks like this issue happens only Macs (M1's running 12.4, not sure about intel) and not on linux machines. Changing the node version between 16.x and 18.x did not help either.

Would love to see this issue fixed ❤️

hiradary commented 2 years ago

@josephcsoti I can confirm. Also, switching from the default terminal (or ohmyzsh) to iTerm2 decreased compile time significantly, but still, something is very off.

R-Bower commented 2 years ago

Just did some testing with my team and it looks like this issue happens only Macs (M1's running 12.4, not sure about intel) and not on linux machines. Changing the node version between 16.x and 18.x did not help either.

Would love to see this issue fixed ❤️

Can confirm this is happening on Intel Macs as well (12.3.1), running node 16.x

callumflack commented 2 years ago

@R-Bower what is your build time like? We regularly had 15 min initial localhost builds on Mac but instant on Linux

R-Bower commented 2 years ago

@R-Bower what is your build time like? We regularly had 15 min initial localhost builds on Mac but instant on Linux

Build time is very quick, but I'm only doing PoC work for a small project (only at ~250 dev modules so far).

hiradary commented 2 years ago

@callumflack For my last project, it took me 20-30 mins to build.

R-Bower commented 2 years ago

Hopefully this will be solved with https://nextjs.org/docs/advanced-features/turbopack, but we're still a while away from a production release.

HuynhBrian commented 2 years ago

I have this issue too

sillyleo commented 1 year ago

same here

khashayarghajar commented 1 year ago

same !

kasuta96 commented 1 year ago

same! :((

tomocrafter commented 1 year ago

I have exact same error on nextjs without vanilla-extract-css, just module scss. I have no idea to solve this problem, any idea?

sohaibMan commented 1 year ago

up up same error

tariqs26 commented 1 year ago

still getting this error

mkmrcodes commented 1 year ago

same error [webpack.cache.PackFileCacheStrategy] Serializing big strings (659kiB) impacts deserialization performance (consider using Buffer instead and decode when needed)

Adnanear commented 1 year ago

It's 2023-06-08 and I do still get the same warning: [webpack.cache.PackFileCacheStrategy] Serializing big strings (659kiB) impacts deserialization performance (consider using Buffer instead and decode when needed). I'm using NextJs 13.4 on Node 18.x

clacladev commented 1 year ago

Some here. M1, Next.js 13.4 on Node 18.x and I have this issue

el-frontend commented 1 year ago

This warning keeps happening, using Next.js version 13.4.4.

marco910 commented 1 year ago

I'm facing the same issue. And the using SVGs as Base64 is also broken. Maybe it's related to this?

writtencommissions commented 1 year ago

was facing the same issue in Next13.3.0, The warning showed on my build after I introduced a route middleware, I solved it by enabling compression in my nextConfig and running the build again:

const nextConfig = {
  reactStrictMode: true,
  compress: true,
}
module.exports = nextConfig
guanyu-y commented 1 year ago

Thank you I got rid of this warning. :) But is the method described above a fundamental solution?

jamierpond commented 1 year ago

I have exact same error on nextjs without vanilla-extract-css, just module scss. I have no idea to solve this problem, any idea?

same here.

sctgraham commented 1 year ago

Same here on next@13.5.6. Any progress on this front ? Thnx

Elvincth commented 1 year ago

same in next v14

askoufis commented 1 year ago

I believe this comment is correct in that the compression fix we did just makes the issue take longer to appear.

That being said, are people that have commented on this thread recently actually experiencing any drastic performance issues as a result of this, or is the issue just "I'm getting a warning and I don't like it"? I can trivially recreate it by generating some bloated sprinkles definitions, but even then I don't really see it impacting page load performance that much.

This warning should really be taken as an indicator that your app is generating a large amount of CSS (more than 110kiB, after compression), and that perhaps it could be worth investigating the cause of that.

Some more notes about this warning:

Elvincth commented 1 year ago

same in next v14

After some investigation, I have found that it is not related to vanilla extract at my side. Instead it is related to Mantine by adding the following to my next.config.mjs fixed my problem.

  experimental: {
    optimizePackageImports: ['@mantine/core', '@mantine/hooks'],
  }, 
Digitl-Alchemyst commented 1 year ago

o.O allow me to join the party on a 2 year long on going issue..

dmitriy-belkin commented 10 months ago

I have same problem(

JeonB commented 10 months ago

Same here ..

askoufis commented 10 months ago

@Digitl-Alchemyst @dmitriy-belkin @JeonB Would appreciate some extra information about whether this issue is actually affecting build/runtime performance of your app, or if you're just experiencing warnings in your console. See https://github.com/vanilla-extract-css/vanilla-extract/issues/374#issuecomment-1798002525.

JeonB commented 10 months ago

I tried again and this problem was solved. Thank you.

@Digitl-Alchemyst @dmitriy-belkin @JeonB Would appreciate some extra information about whether this issue is actually affecting build/runtime performance of your app, or if you're just experiencing warnings in your console. See #374 (comment).

I tried again and this problem was solved. Thank you.

songkeys commented 8 months ago

Is there a way to debug what exactly makes the big strings?

dgd03146 commented 8 months ago

I have the same problem I am using nextjs14