Fix shiki perf issue pt. 2 #2870
Conversation
🦋 Changeset detectedLatest commit: 7421609 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
FredKSchott
force-pushed
the
fix-shiki-perf
branch
2 times, most recently
from
8bf3543 to
e89448f
Compare
FredKSchott
force-pushed
the
fix-shiki-perf
branch
from
e89448f to
7421609
Compare
| */ | ||
| const highlighterCache = new Map<string, shiki.Highlighter>(); | ||
| const highlighterCacheAsync = new Map<string, Promise<shiki.Highlighter>>(); |
There was a problem hiding this comment.
Always makes me nervous to cache promises, but this makes sense to me.
There was a problem hiding this comment.
agreed, see note below about wanting to refactor this post v1.0 beta (for perf but also to get rid of some edge case bugs related to this)
| // NOTE: There may be a performance issue here for large sites that use `lang`. | ||
| // Since this will be called on every page load. Unclear how to fix this. |
There was a problem hiding this comment.
Hmm... Happy to push this off for later. Thanks for adding a note!
There was a problem hiding this comment.
I actually lost a ton of time trying to refactor this yesterday to run once and reuse the created plugin across all parsers, but it turns out (as far as I could tell) you need a fresh parser & fresh plugins created for every file. The fact that we're doing so much async work in a plugin instantiation is a bit of an anti-pattern for unified, I think, but more than I could bite off here.
So there's probably some low-hanging perf wins to unlock in how our markdown parser works, but it was more than I could bite off in this PR.
Changes
Testing
Docs