vercel / next.js

The React Framework
https://nextjs.org
MIT License
123.45k stars 26.33k forks source link

optimizeCss incorrect behavior in Next.js 12 (Performance decrease) #38561

Closed aralroca closed 7 months ago

aralroca commented 2 years ago

Verify canary release

Provide environment information

Operating System: Platform: darwin Arch: x64 Version: Darwin Kernel Version 21.2.0: Sun Nov 28 20:28:54 PST 2021; root:xnu-8019.61.5~1/RELEASE_X86_64 Binaries: Node: 16.10.0 npm: 7.24.0 Yarn: N/A pnpm: N/A Relevant packages: next: 12.1.6 react: 17.0.2 react-dom: 17.0.2 critters: 0.0.7

What browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

Describe the Bug

Here is the official documentation about how to use a CDN with Asset Prefix:

https://nextjs.org/docs/api-reference/next.config.js/cdn-support-with-asset-prefix

However, in Next.js 12 the publicPath of critters changed to use assetPrefix:

v10.2.3 vs v12.2.2

const Critters = require('critters')
const cssOptimizer = new Critters({
  ssrMode: true,
  reduceInlineStyles: false,
 path: renderOpts.distDir,
- publicPath: '/_next/',
+ publicPath: `${renderOpts.assetPrefix}/_next/`,
  preload: 'media',
  fonts: false,
  ...renderOpts.optimizeCss,
})

Now every page request is downloading the CSS from the CDN before calculate the critical CSS to inline, and this has a significant impact on page latency:

[32mInlined 5.76 kB (38% of original 15 kB) of https://cdn.vinissimus.com/front/_next/static/css/d9c4e8c06d2c316e.css.
Time 465.162456

image

Expected Behavior

Even if I use a CDN in my project, to access the CSS you would have to access the /_next directly as before, it doesn't make sense to fetch it from a CDN.

Link to reproduction

https://stackblitz.com/edit/vercel-next-js-qqua8l?file=next.config.js

To Reproduce

In the link to the reproduction you need to change assetPrefix inside next.config.js to a correct CDN (hard to reproduce it without it). Then yarn build && yarn start and check the logs and performance.

Without the link to reproduction (all the process):

  1. Activate optimizeCss on next.config.js and add a CDN inside assetPrefix:
{
  assetPrefix: cdnUrl,
  experimental: { optimizeCss: true }
}
  1. yarn add critters@0.0.7
  2. Add some CSS in some page
  3. yarn build && yarn start
  4. Check the logs and performance.
SukkaW commented 2 years ago

@aralroca Critter is added in Next.js as a Webpack plugin and it would only be executed during the build. It is not used when serving a page (When the build has already been finished), so it shouldn't cause a slowdown.

aralroca commented 2 years ago

@SukkaW yes, is executed as Webpack Plugin, but also in runtime (the Next.js code). If I remove the optimizeCss (what I have done for the moment), instead of worsening the performance, now it improves it.

image

SukkaW commented 2 years ago

@aralroca Would you mind providing a flamegraph to support your theory though?

npx 0x -P 'npx autocannon http://localhost:3000 -d 60'  -- node node_modules/.bin/next start
aralroca commented 2 years ago

@SukkaW I didn't know about flamegraph, although it seems that it doesn't do it very well... (all 0 in the table..)

Next 12 with experimental.optimizeCss + CDN:

Running 60s test @ http://localhost:3000
10 connections

┌─────────┬──────┬──────┬───────┬──────┬──────┬───────┬──────┐
│ Stat    │ 2.5% │ 50%  │ 97.5% │ 99%  │ Avg  │ Stdev │ Max  │
├─────────┼──────┼──────┼───────┼──────┼──────┼───────┼──────┤
│ Latency │ 0 ms │ 0 ms │ 0 ms  │ 0 ms │ 0 ms │ 0 ms  │ 0 ms │
└─────────┴──────┴──────┴───────┴──────┴──────┴───────┴──────┘
┌───────────┬─────┬──────┬─────┬───────┬─────┬───────┬─────┐
│ Stat      │ 1%  │ 2.5% │ 50% │ 97.5% │ Avg │ Stdev │ Min │
├───────────┼─────┼──────┼─────┼───────┼─────┼───────┼─────┤
│ Req/Sec   │ 0   │ 0    │ 0   │ 0     │ 0   │ 0     │ 0   │
├───────────┼─────┼──────┼─────┼───────┼─────┼───────┼─────┤
│ Bytes/Sec │ 0 B │ 0 B  │ 0 B │ 0 B   │ 0 B │ 0 B   │ 0 B │
└───────────┴─────┴──────┴─────┴───────┴─────┴───────┴─────┘

Req/Bytes counts sampled once per second.
# of samples: 60

146k requests in 60.05s, 0 B read

Next 12 without experimental.optimizeCss:

Running 60s test @ http://localhost:3000
10 connections

┌─────────┬──────┬──────┬───────┬──────┬──────┬───────┬──────┐
│ Stat    │ 2.5% │ 50%  │ 97.5% │ 99%  │ Avg  │ Stdev │ Max  │
├─────────┼──────┼──────┼───────┼──────┼──────┼───────┼──────┤
│ Latency │ 0 ms │ 0 ms │ 0 ms  │ 0 ms │ 0 ms │ 0 ms  │ 0 ms │
└─────────┴──────┴──────┴───────┴──────┴──────┴───────┴──────┘
┌───────────┬─────┬──────┬─────┬───────┬─────┬───────┬─────┐
│ Stat      │ 1%  │ 2.5% │ 50% │ 97.5% │ Avg │ Stdev │ Min │
├───────────┼─────┼──────┼─────┼───────┼─────┼───────┼─────┤
│ Req/Sec   │ 0   │ 0    │ 0   │ 0     │ 0   │ 0     │ 0   │
├───────────┼─────┼──────┼─────┼───────┼─────┼───────┼─────┤
│ Bytes/Sec │ 0 B │ 0 B  │ 0 B │ 0 B   │ 0 B │ 0 B   │ 0 B │
└───────────┴─────┴──────┴─────┴───────┴─────┴───────┴─────┘

Req/Bytes counts sampled once per second.
# of samples: 60

153k requests in 60.03s, 0 B read

The only thing I can see from this graph is that with optimizeCss, in one minute you can do fewer requests (146k vs 153k). But I guess it looks like something that didn't run correctly because of the strange results in the table.

fraitag commented 1 year ago

Are you shure? I check code of next 12.3.1 and critters and i found place when assetPrefix is replaced. https://github.com/GoogleChromeLabs/critters/blob/main/packages/critters/src/index.js#L242

    if (normalizedPath.indexOf(pathPrefix) === 0) {
      normalizedPath = normalizedPath
        .substring(pathPrefix.length)
        .replace(/^\//, '');
    }

on normalizedPath i have https://cos/_next/static/css/5749cf13b9f9d25a.css on pathPrefix i have https://cos/_next/

on result in normalizedPath i have static/css/5749cf13b9f9d25a.css

I think i your case is not problem with using files from cdn becouse is getted from disk.

baraeb92 commented 1 year ago

We have experienced this as well.

There is a spike in the CPU usage when using optimizeCSS. As it keep on requesting the CSS file on every load.

screen_shot_2022-10-04_at_11.54.09.pngscreen_shot_2022-10-04_at_13.10.35.pngscreen_shot_2022-10-05_at_13.53.11-1.png

@SukkaW @janis

galaxynova1999 commented 1 year ago

facing same issue here

SukkaW commented 1 year ago

The issue may be irrelevant now. The optimizeCss is an experimental feature and may cause unexpected or broken application behavior. Also, it is impossible to get critical CSS from Next.js App Router + Stream SSR.

leerob commented 7 months ago

https://github.com/vercel/next.js/discussions/59989#discussioncomment-7962270

dmgawel commented 6 months ago

@leerob Why this issue was closed? This one is not about getting optimizeCss for App Router, as your linked comment suggests, but about an unexpected behaviour when optimizeCss and assetsPrefix are used together.

github-actions[bot] commented 6 months ago

This closed issue has been automatically locked because it had no new activity for 2 weeks. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.