-
Notifications
You must be signed in to change notification settings - Fork 262
perf(core): cache measureFunc results in TextBufferRenderable #456
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
base: main
Are you sure you want to change the base?
Conversation
Add measure cache to avoid redundant measureForDimensions calls during Yoga layout passes. Yoga probes nodes with different width constraints (Undefined, AtMost, Exactly) which caused 2-3x redundant text wrapping calculations per layout pass. - Add _version counter to TextBuffer incremented on content changes - Cache measure results keyed by width in TextBufferRenderable - Invalidate cache on version change or wrapMode change - Add benchmark showing 5.3x speedup when content unchanged
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.
Pull request overview
This PR implements a measure cache in TextBufferRenderable to avoid redundant text measurement calculations during Yoga layout passes. During flexbox layout, Yoga often calls the measure function 2-3 times with different width constraints to determine optimal sizing, causing wasteful recomputation of text wrapping. The optimization adds version tracking to detect content changes and caches measurement results by width, providing a 5.3x speedup for unchanged content.
Key changes:
- Added version counter to
TextBufferthat increments on all content mutations - Implemented measure cache in
TextBufferRenderablekeyed by floored width with version-based invalidation - Added benchmark to validate the performance improvement
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/core/src/text-buffer.ts | Adds _version counter incremented by all content-mutating methods (setText, append, loadFile, setStyledText, clear, reset) and exposes it via a getter |
| packages/core/src/renderables/TextBufferRenderable.ts | Implements measure cache with Map keyed by floored width, invalidated on version changes and wrap mode changes |
| packages/core/src/benchmark/measure-cache-benchmark.ts | Adds benchmark comparing cached vs uncached render performance with 144 text renderables in a grid layout |
| .gitignore | Adds *.cpuprofile to ignore CPU profiling output files |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
The Also, the measure function caused a lot of issues in the past, I am very vary. I think there are some invalidations missing, like when wrapMode or viewport changes for example. |
- Move cache from TextBufferRenderable to TextBufferView so it works for both TextBufferRenderable and EditorView - Add cache invalidation on setWrapMode, setWrapWidth, setViewport, setViewportSize - Add version increment on setTabWidth since it affects measurement
|
Ok I moved the cache inside TextBufferView. Benchmark still shows 5x faster Added also more invalidations |
Summary
Adds a measure cache to
TextBufferRenderableto avoid redundantmeasureForDimensionscalls during Yoga layout passes.Why Yoga calls measureFunc multiple times:
During flexbox layout, Yoga probes nodes with different width constraints to determine optimal sizing:
Undefinedmode to get intrinsic sizeAtMostto constrain to available spaceExactlyafter resolving flex grow/shrinkThis means
measureForDimensions(which does text wrapping calculations) was being called 2-3x per layout pass per text node.Solution:
_versioncounter toTextBufferthat increments on content changesTextBufferRenderablekeyed by widthtextBuffer.versionchanges orwrapModechangesBenchmark results (144 text renderables in 8x6 grid):
References #433 (comment)