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

Icon resize task blocks the main thread during build #171

Closed
jhnns opened this issue Mar 30, 2019 · 5 comments
Closed

Icon resize task blocks the main thread during build #171

jhnns opened this issue Mar 30, 2019 · 5 comments

Comments

@jhnns
Copy link

jhnns commented Mar 30, 2019

Version

2.6.0

Reproduction link

https://github.com/jhnns/nuxt-pwa-build-perf-demo

Steps to reproduce

  • Create an initial nuxt project with create-nuxt-app and select the PWA module
  • Run node --inspect-brk ./node_modules/.bin/nuxt
  • Open Chrome and go to chrome://inspect/#devices
  • Select Node process and create a profile

What is expected ?

The icon resize task should start as soon as possible in a different process/thread to speed up the build.

What is actually happening?

Here is a flamegraph that demonstrates that the icon resize task takes up more than half of the build time. But the bigger problem is that the task blocks the main thread which means that webpack can't start to do its work in the mean time. Here I've marked the resize task with red:
Bildschirmfoto 2019-03-30 um 17.57.58 Kopie.jpg

You can see that webpack starts its work only after around 16s.

Additional comments?

Spinning up a separate process doesn't come for free. Usually it's better to have a separate thread for this kind of task. We could do this with Node.js worker threads (https://nodejs.org/api/worker_threads.html) which are only available for Node >10.

This bug report is available on Nuxt community (#c116)
@ghost ghost added the cmty:bug-report label Mar 30, 2019
@pi0
Copy link
Member

pi0 commented Apr 30, 2019

Hi @jhnns. Thanks for reporting this issue. Actually, this problem was not visible because normally the original icon is small-sized.

I've tried to make the issue more visible by using a bigger icon in the test fixture:

image

I'm working on an icon module rewrite in a way that resize microtasks running in the background. (Initial read may be still required before build to generate MD5 hash that is used in filenames)

pi0 pushed a commit that referenced this issue Apr 30, 2019
@pi0
Copy link
Member

pi0 commented Apr 30, 2019

Implemented in a4a457e (with some enhancenments including cache in later commits) I used normal cp.fork to support node <10.

image

@jhnns
Copy link
Author

jhnns commented May 3, 2019

this problem was not visible because normally the original icon is small-sized

I never changed the icon. I just used the default icon (see also reproduction link).

Implemented in a4a457e (with some enhancenments including cache in later commits) I used normal cp.fork to support node <10.

Awesome, the flame graph looks good 👍. I just left some comments at the commit :)

@pi0
Copy link
Member

pi0 commented May 3, 2019

Thanks, @jhnns for the comments.

@pi0
Copy link
Member

pi0 commented May 7, 2019

Released in 3.0.0-beta.15

@pi0 pi0 closed this as completed May 7, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants