Optimize Height Estimation#718
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d60fd50625
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const item: CodeViewItem<CommentMetadata> = { | ||
| id, | ||
| type: 'diff', | ||
| collapsed: fileDiff.type === 'deleted', | ||
| fileDiff, | ||
| version: 0, | ||
| }; |
There was a problem hiding this comment.
This constructor no longer sets collapsed for deleted files, so type: 'deleted' entries now render expanded by default in Diffshub. On large patches with big deleted files, that forces full deleted-content layout/render work up front (instead of deferring it behind collapse), which is a user-visible performance regression compared to prior behavior.
Useful? React with 👍 / 👎.
amadeus
force-pushed
the
amadeus/cache-estimated-heights
branch
from
d60fd50 to
088df82
Compare
amadeus
force-pushed
the
amadeus/cache-estimated-heights
branch
from
088df82 to
45a508d
Compare
Create small reusable utilities for the estimate helper and existing virtual layout code. - Extract or share the `getExpandedRegion` logic currently inside `iterateOverDiff` so expansion math cannot drift. - Add a separator estimate helper that mirrors renderer/CSS behavior. - Keep these helpers pure and independent of `VirtualizedFileDiff` instance state. Separator helper rules: - `simple`: render only when `hunkIndex > 0`; height is `metrics.hunkSeparatorHeight ?? 4`. - `metadata`: render only when `hunkSpecs != null`; height is `metrics.hunkSeparatorHeight ?? 32`. - `line-info-basic`: height is `metrics.hunkSeparatorHeight ?? 32`. - `line-info`: height is `metrics.hunkSeparatorHeight ?? 32`, plus top/bottom spacing based on first/last position. - `line-info`: omit top gap for `isFirstHunk`. - `line-info`: omit bottom gap for `isLastHunk`. - `custom`: estimate like `line-info` unless we decide to force custom separators to be measurement-only later.
amadeus
force-pushed
the
amadeus/cache-estimated-heights
branch
from
45a508d to
e730f08
Compare
1 participant
This is a pass at optimizing how we estimate height, yet another piece of the puzzle that can potentially improve performance in certain scenarios.