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

feat: add build concurrency control (fix #1819) #2953

Merged
merged 3 commits into from Jan 15, 2022

Conversation

troyeagle
Copy link
Contributor

@troyeagle troyeagle commented Nov 24, 2021

Summary

What kind of change does this PR introduce? (check at least one)

  • Bugfix
  • Feature
  • Code style update
  • Refactor
  • Docs
  • Build-related changes
  • Other, please describe:

If changing the UI of default theme, please provide the before/after screenshot:

Does this PR introduce a breaking change? (check one)

  • Yes
  • No

If yes, please describe the impact and migration path for existing applications:

The PR fulfills these requirements:

  • When resolving a specific issue, it's referenced in the PR's title (e.g. fix #xxx[,#xxx], where "xxx" is the issue number)

You have tested in the following browsers: (Providing a detailed version will be better.)

  • Chrome
  • Firefox
  • Safari
  • Edge
  • IE

This PR is all about build phase. No need to check browser compability.

If adding a new feature, the PR's description includes:

  • A convincing reason for adding this feature
  • Related documents have been updated
  • Related tests have been updated

To avoid wasting your time, it's best to open a feature request issue first and wait for approval before working on it.

Other information:

To fix #1819. Vuepress build would fail for many docs, for Promise.all push thousands of files to VueServer to render concurrently and cause OOM.
#2454 tried to fix this problem. However, with pLimit fixed to 50, the build process may be significantly slowed down. To fix this, this PR makes it optional as a vuepress build param.

@troyeagle
Copy link
Contributor Author

troyeagle commented Nov 24, 2021

Why not p-limit

I did not dig through it, but in my case, it seems p-limit will cause the whole process turn into serial, which is actually much slower.
This is the case manually grouped by 50:
manual

This is the case using p-limit, single task time cost is printed, the max concurrency is set to 50, then t/50 maybe the actual time cost.
pLimit

Copy link
Member

@ulivz ulivz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you address the comment and update the documentation?

@troyeagle troyeagle requested a review from ulivz December 24, 2021 07:56
@ulivz ulivz merged commit 2f9a394 into vuejs:master Jan 15, 2022
@sr2ds
Copy link

sr2ds commented Apr 13, 2023

Hello, @troyeagle first congrats by this job, its really cool.

I'm trying to optimize this flow or trying understand better.
I have a blog with >2K posts and the memory use on build is so big.
I'm trying setting up some different max concurrency but without success yet.

I don't know if do you remember but, on my tests, the memory is never cleanuped while the loop run.
I'm trying cleanup the memory while the process is running but I can´t do it.
In other words, I can to limit the async process with the max concurrency, but the memory usage is more big each time.
This is a dump of memory after each promise call.

using max-concurrency 5 for try: the memory growth slow but never go back
image

My tests there are around this place:
image
I tried set as null the variables to 'clean-up' memory, but I guess the GC do it alone. BUT if do, is not doing.

Thanks for your attention, if you have some idea to share with me, will be great.

@troyeagle
Copy link
Contributor Author

As I remember, what magic happens may be in the method this.renderPage, or your processAsyncRenderPage which uses vue-server-renderer and use its renderString utility.

The server-renderer may be stateful, it not just accepts an object and uses template to parse it, but run with a context and then create a bunch of async rendering tasks. The context may expand during the rendering process to save building cache for optimizing following tasks' rendering performance.

I think it's not the results' not being GC-ed but the context growing that causes your memory growing. It seems vuepress always uses one context, when a group of tasks ends, just some of the spaces are set free. @sr2ds You could check whether the total size of 5 pages' html output string is the same as your memory growth per time, or use chrome://inspect to see what object in the memory is really growing as the task goes on, then you may have idea of what objects or attributes you should set null to trigger garbage collection.

In all, the memory problem is the tradeoff of server-side rendering optimization. Vuepress maybe did not expect such huge size of pages, and in vuepress build case, building speed may be the first concern.

@sr2ds
Copy link

sr2ds commented Apr 20, 2023

Thanks so much for your time and answer.

To be honest I don't know how do a good review on inspect mode, but looks that the string type of data is the most used, I don't sure if can be the returns of renderString stored on context.

I made some tests but without success yet.

I saw this other issue with some comparatives between vuepress and vitepress, I'll try a little bit more but looks more prudent to migrate to vitepres on this specific blog case.

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

Successfully merging this pull request may close these issues.

FATAL ERROR: CALL_AND_RETRY_LAST Allocation failed - JavaScript heap out of memory
3 participants