-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
feat($core): Improve VuePress build time #2163
Conversation
Tip: You can use "Create draft PR" for WIP PRs |
@meteorlxy Yes I forgot about this one 😅 👍 |
Hi @kefranabg. I have a project where the build was gradually slowing down until it fails (blows the stack) and I just tried your new code. It now renders all the pages in less than a second! |
@TribeWeb I'd be curious to know how many docs in your project and if you ran this code on top of 1.2 or 1.3. I've got a project with 2k+ docs and dropping this code onto 1.3, I'm still seeing lengthy build times (24 minutes). I also was getting "JavaScript heap out of memory" errors mid-build until I changed the build script to set node's Keen to hear a bit more as I was working on a separate approach to pull in node's worker_threads which I had working under 1.2 and had dropped my build time from 1 hour to 7.5 minutes, though the move to 1.3 has created some issues. |
Hi @itsxallwater. It’s a much smaller project about 100 pages. I’m using it with Vuetify and moving from v1.5 to 2.2. Build time was seconds previous but now hitting out of memory errors until I tried the fix above. VuePress is 1.3. I keep hitting this problem sometimes for weird reasons. I isolated the build time problem on the previous version of Vuetify to one button and fixed by wrapping it in ‘’. |
e86e470
to
59b20bb
Compare
Great work on this @kefranabg! |
About improvement 1, #315 (comment) discuessed long time ago (in v0.x 😅). Let's check if it would break the cli output in current version. |
Oh, so you removed the cli output in the PR. Have you made some benchmark for these improvements? @kefranabg |
@meteorlxy I'll update the description later to add the benchmark 😄On small projects like the vuepress doc, these changes are not that effective. But on projects that have like 600pages to render, it has an interesting impact. There is still some improvements to do to improve the build time, probably on webpack config. |
With more than 600 pages to render Without optimization: |
} | ||
|
||
const pagePaths = await Promise.all(pagePathsPromises) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
From 91 to 97:
const pagePaths = await Promise.all(pagePathsPromises) | |
const pagePaths = await Promise.all( | |
this.context.pages.map(page => this.renderPage(page)) | |
) |
Co-Authored-By: meteorlxy <meteor.lxy@foxmail.com>
Have you checked #2221? I am afraid this change will make this issue even worse.@kefranabg |
FWIW these updates took our build time down to ~10 minutes, and we're talking ~1500 docs with these plugins: @Mister-Hope might be noteworthy but we're overloading the build command in the package.json to run as |
I knew node can use more memory. But I am talking about memory leak. Before changing, I don't think it's normal that the memory keep rising during build. |
I don't disagree, just wanted to add context from our build experience. |
Let me explain my idea more carefully, I agree that the build process of pages should be parrell intead of await each other. But there must be some GC issues while building. Your FR is nice, since the offcial accepted it, I want to remind the offical that this may make the GC issue even worse. At least I am using even more memory in 1.4.0. You can't just talk that it not an issue since node can take more memory in use. I think we should optimize the memory usage while building. |
I'm not arguing :) More context for the thread at large--we've moved our build to a GitHub action and it's reliably coming in at 20 minutes. |
Summary
These optimisations make sense on projects that contains a huge amount of pages to render.
Related to #1560
What kind of change does this PR introduce? (check at least one)
Performance enhancement
Does this PR introduce a breaking change? (check one)
Steps:
Improvement #1 : Render static pages in parallel instead of rendering one page after the other.
Improvement #2 : Run extendPageData enhancers in parallel