perf: use charWidthCache to improve performance by 1.5% - #1978
Conversation
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
🦋 Changeset detectedLatest commit: 4b6b4bd The changes in this PR will be included in the next version bump. This PR includes changesets to release 47 packages
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 |
There was a problem hiding this comment.
Summary of Changes
Hello @Alexzjt, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request primarily focuses on enhancing rendering performance, particularly for text elements, by optimizing how character widths are measured and cached. The core change involves elevating a previously local character width cache to a global, font-aware service-level cache, drastically reducing redundant measureText calls. Additionally, I've improved the efficiency of font string generation and added a minor optimization to scene graph dirty state propagation.
Highlights
- Performance Improvement: Global Character Width Caching: I've introduced a new global
charWidthCachewithin theTextServiceto persist character width measurements across different text objects. Previously, the cache was local to eachwordWrapcall, leading to redundantcontext.measureTextcalls, especially in scenarios like scrolling tables with many text elements. This change aims to significantly reduce these repeated measurements. - Memoization for Font String Generation: I've applied memoization to the
toFontStringutility function inpackages/g-lite/src/utils/text.ts. This caches the generated font strings based on text style attributes, preventing redundant computations when the same font style is used repeatedly, further contributing to performance. - Scene Graph Dirty State Optimization: I've refined the dirty state propagation logic in
DefaultSceneGraphService.ts. Thewhileloop that marks parent elements as dirty now includes a condition to stop traversal if an element is already marked as bothdirtyandboundsDirty. This can prevent unnecessary re-marking of elements that have already been flagged for update.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments or fill out our survey to provide feedback.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a global cache for character widths to improve text rendering performance. The core logic in TextService.ts is sound and directly addresses the performance bottleneck.
However, I've identified a few areas that need attention:
- A critical issue exists in
toFontStringwhere the memoization introduces code duplication and flawed logic for handlingfontSize, potentially leading to incorrect font rendering and caching problems. - The modification in
packages/g-lite/src/services/SceneGraphService.tsis an unrelated optimization and should be submitted in its own PR. - There's an inaccurate comment in
packages/g-lite/src/services/TextService.tsthat needs to be updated.
Addressing the critical issue in toFontString is essential before merging.
|
这个改动风险比较大,字体的测量结果不仅与字体相关,可能还与对齐方式等其它属性相关;还有持久缓存可能带来内存泄露问题 |
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces a global, persistent charWidthCache using an LRU strategy to improve text rendering performance. The implementation also includes a new LRU cache utility and memoization for toFontString.
A critical issue was identified in TextService.ts where the new LRU cache is accessed incorrectly, which will prevent it from working. A suggestion to further improve performance in toFontString by avoiding a redundant function call was also made.
Once the critical issue is addressed, this change should provide the intended performance benefits.
|
G2、G6 本地测试通过 |
* fix: prevent NaN in transform calculations when scale is 0 (#1971) * fix: prevent NaN in transform calculations when scale is 0 - Add MIN_SCALE constant to avoid zero scaling - Update scale transform functions to enforce minimum scale value - Add test case for scale(0) transform - Add demo example with scale(0) transform * chore: add changeset * refactor: extract scale clamping logic into reusable function - Add clampScale helper function to avoid code duplication - Replace repeated Math.max(item, MIN_SCALE) calls with clampScale - Maintain same behavior while improving code maintainability * chore: add changeset * perf: use charWidthCache to improve performance by 1.5% (#1978) * perf: 避免重复dirty父级元素提升2%性能 * perf: accept gemini review Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> * refactor: 修复eslint * perf: use charWidthCache to improve performance by 1.5% * refactor: 修复评审意见 * refactor: 修复评审意见 * refactor: 使用LRU * refactor: 使用LRU * chore: add changeset --------- Co-authored-by: huiyu.zjt <huiyu.zjt@antgroup.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: wang1212 <mrwang1212@126.com> * revert: fix element attribute update exception #1968 * chore(release): bump version (#1977) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --------- Co-authored-by: huiyu.zjt <Alexzjt@users.noreply.github.com> Co-authored-by: huiyu.zjt <huiyu.zjt@antgroup.com> Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
🤔 This is a ...
🔗 Related issue link
💡 Background and solution
原有逻辑:短暂的、局部的字符缓存
瓶颈分析
结论:现有的字符缓存机制,只能对单个长文本的内部重复字符生效(比如一个段落里有多个'A',只测量一次'A'的宽度)。但它无法在多个不同的文本对象之间共享缓存。在表格滚动场景下,成百上千个单元格被创建和销毁,每个单元格的文本都会触发一次独立的、从零开始的字符宽度测量,这导致 context.measureText 被大量重复调用,从而产生了2%的性能开销。
优化方案
将这个局部的、短暂的 cache 提升为持久的、服务级别的全局缓存,就像 fontMetricsCache 一样。需要让这个缓存与字体关联,因为同一个字符在不同字体下的宽度是不同的。一个理想的缓存结构是:
1.当第一个表格单元格渲染时,它会计算并缓存其文本中所有字符在特定字体下的宽度。
2.当滚动到新的单元格时,如果它们使用了相同的字体,wordWrap 在计算字符宽度时将几乎总是命中全局缓存 this.charWidthCache。
3.实际调用 context.measureText 的次数将从“每个单元格的每个字符一次”锐减到“整个应用生命周期内,每种字体的每个字符一次”。
📝 Changelog
☑️ Self Check before Merge