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

perf: cache shiki #4321

Merged
merged 4 commits into from
Oct 29, 2024
Merged

perf: cache shiki #4321

merged 4 commits into from
Oct 29, 2024

Conversation

Barbapapazes
Copy link
Contributor

@Barbapapazes Barbapapazes commented Oct 29, 2024

Description

Hello 👋,

I'm building my website, https://soubiran.dev, with VitePress.

At first, it was fast but as I added more content and more data loaders, it became slower and slower to build. Today, it takes around 4 minutes to build the website and each time I add a new page, it adds 10 seconds to the build time.

So I decided to investigate to find the annoying issue. At the same time, I had this warning: Shiki is supposed to be used as a singleton, consider refactoring your code to cache your highlighter instance;.

I moved all my content to a fresh VitePress project and I found that the speed build was back to normal. In my project, soubiran.dev, I started to play with DEBUG=vitepress:md and I discover that some pages took 27s to build while some was under 100ms. This led me to the conclusion that the issue was related to the markdown content and potentially the parser: MarkdownIt.

With the warning in mind, I quickly understand the highlighter creation was the issue: https://github.com/vuejs/vitepress/blob/main/src/node/markdown/markdown.ts#L208

I also discover that every time a createContentLoader is used, a new instance of MarkdownIt is created: https://github.com/vuejs/vitepress/blob/main/src/node/contentLoader.ts#L129

So I try to patch VitePress in my project by caching the highlighter instance.

Before, in the Cloudflare Pages CI: more than 4 minutes to build the website.
After, in the Cloudflare Pages CI: 47 seconds to build the website.

related to https://x.com/soubiran_/status/1851189188634288614

This also improve significantly the dev startup of the project.

Unfortunately, this project is private but I can happily invite maintainers if needed.

Linked Issues

Additional Context


Tip

The author of this PR can publish a preview release by commenting /publish below.

@Barbapapazes
Copy link
Contributor Author

Barbapapazes commented Oct 29, 2024

While I was writing this PR, I was wondering if we couldn't go further and cache the MardownIt instance?

So I try but it did no improve perf.

@brc-dd
Copy link
Member

brc-dd commented Oct 29, 2024

Actually the issue is we need to reload the highlighter instance and destroy the old one too in certain cases like reloading config file (because a user might change shiki's theme and expect it to reload). But yeah I'm seeing perf degradation with both cold start (and config reload).

If possible, can you add me temporarily with read-only access on the repo? You can dm me on discord if you've any concerns (I'm in https://chat.vuejs.org/ - #vitepress channel).

But from what you've told, I believe the issue in your case is because of using createMarkdownRenderer in data loader (similar to note in #4300 (comment)). We probably should provide better API instead of createMarkdownRenderer, or maybe an option to re-use the instance (enabled by default, but disabled inside our vite plugin so that on config reload it still creates and caches a new instance but inside data loaders, etc. it can re-use the cached one).


Ok looks like Anthony added the dispose function - shikijs/shiki#707 - this should be used in case of clearing cache (config reloads). It wasn't there when that memory leak was initially fixed (and that singleton warning was added).

@brc-dd
Copy link
Member

brc-dd commented Oct 29, 2024

/publish

Copy link

pkg-pr-new bot commented Oct 29, 2024

pnpm add https://pkg.pr.new/vitepress@4321

commit: 4c510b4

@Barbapapazes
Copy link
Contributor Author

/publish

I've updated soubiran.dev and this PR is in production! (build time: 30 seconds on CF, 13 seconds on my computer and a way better dev start time)

@brc-dd brc-dd marked this pull request as draft October 29, 2024 12:52
@brc-dd brc-dd marked this pull request as ready for review October 29, 2024 13:28
@brc-dd brc-dd merged commit 45968cd into vuejs:main Oct 29, 2024
9 checks passed
@Barbapapazes
Copy link
Contributor Author

I just update my dependencies, and now, it's under 10 seconds 🥳

Screenshot 2024-11-02 at 19 34 24

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 10, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants