Repository navigation
fix(native): bump godemirror to v0.7.1 for shared fonts and CJK fallback - #819
Merged
Merged
Conversation
godemirror's DiffView, one per file card in the Review and in a pull request's Files, built a glyph rasterizer of its own that read the font files again, and asked fontconfig for a fallback once per distinct character the family lacks, two subprocesses inside the frame. v0.7.1 reads font files once per process and tries the fallbacks already loaded first: 50 DiffViews hold about 1 MB of heap where they held 61 MB. Found in a heap profile of a real window (LICH_NATIVE_PPROF): opening one pull request's Files added 70 MB in os.ReadFile under godemirror's glyphs.New.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
A heap profile of a real window (
LICH_NATIVE_PPROF, from #798) showed the heap growing 117 MB in its first five minutes, 70 MB of it inos.ReadFileunder godemirror'sinternal/glyphs.New, reached fromfileCard.ensureViewin a pull request's Files tab.godemirror carried the same two problems #795 and #798 fixed in
native/glyphs:DiffView, one per file card in the Review and in Pulls, built its own rasterizer that read the font files again.fc-listandfc-match -sas subprocesses inside the frame, so a diff full of CJK text stalled.What changes
native/go.modmoves to godemirror v0.7.1. It reads font files once per process and shares them between views; only the sized face stays per view. It also tries the fallbacks already loaded before looking up again, and reads the monospace ranking once. 50DiffViews hold about 1 MB of heap where they held 61 MB, with the monospace family alone; a CJK fallback multiplied that per file.Tests
The tests live in godemirror:
TestALoadedFallbackServesTheCharactersItCovers(seen failing before the fix: 20 lookups for 20 characters),TestRasterizersShareAFontFile,TestFallbackLookupsRankAgainstMonospaceOnce; its suite is green with-race. Here, thenative/gate is green with the new version.