Repository navigation
Conversation
Contributor
|
@kyzer1023 is attempting to deploy a commit to the Cap Software Inc Team on Vercel. A member of the Team first needs to authorize it. |
Comment on lines
+21
to
+26
| black_box(f( | ||
| black_box(&src), | ||
| black_box(width), | ||
| black_box(stride), | ||
| black_box(height), | ||
| )); |
Contributor
There was a problem hiding this comment.
Benchmark silently discards its result
black_box(f(...)) returns Option<Vec<u8>>, but the benchmark silently discards it. AGENTS.md requires every Option result to be explicitly handled. Use let _ = black_box(f(...)); to mark the discard as intentional and avoid the unused_must_use warning. This repository requirement must be satisfied before merging.
Suggested change
| black_box(f( | |
| black_box(&src), | |
| black_box(width), | |
| black_box(stride), | |
| black_box(height), | |
| )); | |
| let _ = black_box(f( | |
| black_box(&src), | |
| black_box(width), | |
| black_box(stride), | |
| black_box(height), | |
| )); |
Context Used: AGENTS.md (source)
Prompt To Fix With AI
This is a comment left during a code review.
Path: apps/desktop-gpui/kernels/benches/frame_unpad.rs
Line: 21-26
Comment:
**Benchmark silently discards its result**
`black_box(f(...))` returns `Option<Vec<u8>>`, but the benchmark silently discards it. `AGENTS.md` requires every `Option` result to be explicitly handled. Use `let _ = black_box(f(...));` to mark the discard as intentional and avoid the `unused_must_use` warning. This repository requirement must be satisfied before merging.
```suggestion
let _ = black_box(f(
black_box(&src),
black_box(width),
black_box(stride),
black_box(height),
));
```
**Context Used:** AGENTS.md ([source](https://github.com/capsoftware/cap/blob/main/AGENTS.md))
---
For each issue above, determine whether it is valid and should be fixed. If so, fix it directly.
Author
There was a problem hiding this comment.
Fixed in cb5d9d7: the benchmark now uses let _ = black_box(f(...)); to make the discard explicit, per AGENTS.md.
cursor
Bot
force-pushed
the
cursor/perf-gpui-unpad-v2-squashed
branch
from
October 15, 2026 08:00
10cab80 to
cb5d9d7
Compare
Author
|
@greptileai review |
unpad_rgba_to_bgra was vec![0; len] then overwriting every pixel on the Linux/Windows playback path. Write through with_capacity + *mut u8 + set_len instead. spare_capacity_mut/MaybeUninit::write skips calloc but on rustc 1.88 (the Cap pin) the inner stores stay four byte moves and regress vs the zero-filled baseline.
cursor
Bot
force-pushed
the
cursor/perf-gpui-unpad-v2-squashed
branch
from
October 19, 2026 18:06
5368e51 to
69287d9
Compare
Author
|
@greptileai review |
This branch has not been deployed
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
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.
Summary
unpad_rgba_to_bgra(Linux/Windows editor playback viaframe_image, and the portable camera-blur readback) allocated withvec![0; len]and then overwrote every byte. This writes throughVec::with_capacity+*mut u8+set_leninstead: same skip-zero allocation, tight nested loop, no iterator zip.An earlier
spare_capacity_mut/MaybeUninit::writeversion was faster on rustc 1.99 but ~0.78x slower on rustc 1.88 (the pinned toolchain), because 1.88 doesn't inlineIterator::zipover theMaybeUninitchunks. The raw-pointer loop is a win on both.Change
apps/desktop-gpui/kernels/src/frame.rs: raw-pointer unpad, no zero fill. Onlyunpad_rgba_to_bgrachanges; main'sreorder_32bit_row/convert_32bit_rowswork is untouched.apps/desktop-gpui/kernels/benches/frame_unpad.rs: before/after harness with the previous implementation inlined (results consumed withlet _ = black_box(...)per AGENTS.md)apps/desktop-gpui/kernels/Cargo.toml: keeps all of main's benches (includingoverlay_image_inspection,frame_export,screenshot_alpha) and appendsframe_unpadRebased onto
main(0bc36e0d). Single commit.Relationship to #2513
#2513 edits the same file (
kernels/src/frame.rs) but different functions (swap_red_blue_into,convert_32bit_rows). The changes are complementary; landing both is a same-file merge, not overlapping edits.Benchmarks
On the rebased commit. Intel Xeon, 4 cores, KVM; Ubuntu 24.04.5 LTS, Linux 6.12.94+. GPUI-like release profile (
opt-level=3,lto=thin,codegen-units=1), 5 warmup, median of 30. Baseline is main'svec![0]path.Earlier runs on previous bases matched (1.29–1.32× on both toolchains), with byte-identical output to the previous implementation and Miri clean on 11 tests.
Tests
Full
cap-gpui-kernelssuite: 40 passed, 0 failed, 1 ignored (benchmark_waveform_peaks) on both rustc 1.88.0 and 1.99.1.The full GPUI app was not built: the
[patch."https://github.com/wingleeio/zed"]entry points at/zed-cap, which isn't available in the test VM, so the kernels crate was extracted into a standalone workspace.Repro
With
/zed-capchecked out:Without it, extract
apps/desktop-gpui/kernelsinto a one-crate workspace and run:

Confidence Score: 5/5
The PR appears safe to merge; no blocking issues remain.Summary
Rewrites
unpad_rgba_to_bgrato fill allocated space through a raw pointer instead of zeroing it first.Reviews (3) · Last reviewed commit: "improve(desktop-gpui): write BGRA unpad ..." · Reviewed by Greptile