fix: address CodeRabbit review findings

- Add checkout step before local composite action (GitHub Actions
  requires repo on disk to read action.yml)
- Use PR base SHA for path detection in multi-commit PRs
- Fail benchmarks loudly on decode errors (.expect instead of let _ =)
- Document intentional encode error handling (codec may drop frames)

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
Clémence Lesné 2026-04-08 18:58:00 +02:00
parent 3cf8eac163
commit cfb41c1358
6 changed files with 39 additions and 27 deletions

View file

@ -30,11 +30,6 @@ runs:
core.exportVariable('ACTIONS_CACHE_URL', process.env.ACTIONS_CACHE_URL || '');
core.exportVariable('ACTIONS_RUNTIME_TOKEN', process.env.ACTIONS_RUNTIME_TOKEN || '');
- name: Checkout source code
uses: actions/checkout@v4
with:
submodules: recursive
- name: Install prerequisites
shell: bash
run: |

View file

@ -16,6 +16,10 @@ jobs:
name: Performance benchmarks
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
with:
submodules: recursive
- uses: ./.github/actions/setup-linux
- name: Install cargo-codspeed

View file

@ -31,6 +31,10 @@ jobs:
job:
- { target: x86_64-unknown-linux-gnu, os: ubuntu-24.04 }
steps:
- uses: actions/checkout@v4
with:
submodules: recursive
- uses: ./.github/actions/setup-linux
- name: Show version information (Rust, cargo, GCC)
@ -75,10 +79,15 @@ jobs:
steps:
- uses: actions/checkout@v4
with:
fetch-depth: 2
fetch-depth: 0
- id: check
run: |
CHANGED=$(git diff --name-only HEAD~1 HEAD -- libs/scrap/ libs/hbb_common/ 2>/dev/null || echo "")
if [ "${{ github.event_name }}" = "pull_request" ]; then
BASE="${{ github.event.pull_request.base.sha }}"
else
BASE="HEAD~1"
fi
CHANGED=$(git diff --name-only "$BASE" HEAD -- libs/scrap/ libs/hbb_common/ 2>/dev/null || echo "")
echo "run=$( [ -n "$CHANGED" ] && echo true || echo false )" >> "$GITHUB_OUTPUT"
benchmarks:

View file

@ -91,13 +91,13 @@ fn bench_decode_single(c: &mut Criterion) {
let mut idx = 0;
b.iter(|| {
let union = &unions[idx % unions.len()];
let _ = decoder.handle_video_frame(
decoder.handle_video_frame(
black_box(union),
&mut rgb,
&mut texture,
&mut pixelbuffer,
&mut chroma,
);
).expect("decode failed");
idx += 1;
});
});
@ -118,13 +118,13 @@ fn bench_decode_single(c: &mut Criterion) {
let mut idx = 0;
b.iter(|| {
let union = &unions[idx % unions.len()];
let _ = decoder.handle_video_frame(
decoder.handle_video_frame(
black_box(union),
&mut rgb,
&mut texture,
&mut pixelbuffer,
&mut chroma,
);
).expect("decode failed");
idx += 1;
});
});
@ -145,13 +145,13 @@ fn bench_decode_single(c: &mut Criterion) {
let mut idx = 0;
b.iter(|| {
let union = &unions[idx % unions.len()];
let _ = decoder.handle_video_frame(
decoder.handle_video_frame(
black_box(union),
&mut rgb,
&mut texture,
&mut pixelbuffer,
&mut chroma,
);
).expect("decode failed");
idx += 1;
});
});
@ -181,13 +181,13 @@ fn bench_vp9_decode_sequence(c: &mut Criterion) {
group.bench_function(BenchmarkId::from_parameter("100frames_1080p"), |b| {
b.iter(|| {
for union in &unions {
let _ = decoder.handle_video_frame(
decoder.handle_video_frame(
black_box(union),
&mut rgb,
&mut texture,
&mut pixelbuffer,
&mut chroma,
);
).expect("decode failed");
}
});
});
@ -216,7 +216,7 @@ fn bench_vp9_decode_4k(c: &mut Criterion) {
let mut idx = 0;
b.iter(|| {
let union = &unions[idx % unions.len()];
let _ = decoder.handle_video_frame(
decoder.handle_video_frame(
black_box(union),
&mut rgb,
&mut texture,

View file

@ -49,7 +49,8 @@ fn bench_vpx_encode_single(c: &mut Criterion) {
let mut pts = 0i64;
b.iter(|| {
let input = EncodeInput::YUV(&yuv);
let _ = encoder.encode_to_message(input, pts);
// encode_to_message may return Err("no valid frame") when the codec drops a frame — this is normal
drop(encoder.encode_to_message(input, pts));
pts += 1;
});
});
@ -71,7 +72,8 @@ fn bench_vpx_encode_single(c: &mut Criterion) {
let mut pts = 0i64;
b.iter(|| {
let input = EncodeInput::YUV(&yuv);
let _ = encoder.encode_to_message(input, pts);
// encode_to_message may return Err("no valid frame") when the codec drops a frame — this is normal
drop(encoder.encode_to_message(input, pts));
pts += 1;
});
});
@ -109,7 +111,8 @@ fn bench_encode_4k(c: &mut Criterion) {
let mut pts = 0i64;
b.iter(|| {
let input = EncodeInput::YUV(black_box(&yuv));
let _ = encoder.encode_to_message(input, pts);
// encode_to_message may return Err("no valid frame") when the codec drops a frame — this is normal
drop(encoder.encode_to_message(input, pts));
pts += 1;
});
});
@ -141,7 +144,7 @@ fn bench_vp9_encode_sequence_static(c: &mut Criterion) {
b.iter(|| {
for i in 0..100 {
let input = EncodeInput::YUV(black_box(&yuv));
let _ = encoder.encode_to_message(input, i);
drop(encoder.encode_to_message(input, i));
}
});
});
@ -174,7 +177,7 @@ fn bench_vp9_encode_sequence_movement(c: &mut Criterion) {
b.iter(|| {
for (i, yuv) in frames.iter().enumerate() {
let input = EncodeInput::YUV(black_box(yuv));
let _ = encoder.encode_to_message(input, i as i64);
drop(encoder.encode_to_message(input, i as i64));
}
});
});
@ -211,7 +214,8 @@ fn bench_vp9_encode_quality(c: &mut Criterion) {
let mut pts = 0i64;
b.iter(|| {
let input = EncodeInput::YUV(black_box(&yuv));
let _ = encoder.encode_to_message(input, pts);
// encode_to_message may return Err("no valid frame") when the codec drops a frame — this is normal
drop(encoder.encode_to_message(input, pts));
pts += 1;
});
});

View file

@ -83,13 +83,13 @@ fn bench_pipeline_decode_1080p(c: &mut Criterion) {
// Step 1: Protobuf deserialize
// Step 2+3: Decode + YUV→RGB via real Decoder::handle_video_frame
if let Some(union) = extract_union(black_box(msg_bytes)) {
let _ = decoder.handle_video_frame(
decoder.handle_video_frame(
&union,
&mut rgb,
&mut texture,
&mut pixelbuffer,
&mut chroma,
);
).expect("decode failed");
}
idx += 1;
@ -121,13 +121,13 @@ fn bench_pipeline_decode_4k(c: &mut Criterion) {
b.iter(|| {
let msg_bytes = &messages[idx % messages.len()];
if let Some(union) = extract_union(black_box(msg_bytes)) {
let _ = decoder.handle_video_frame(
decoder.handle_video_frame(
&union,
&mut rgb,
&mut texture,
&mut pixelbuffer,
&mut chroma,
);
).expect("decode failed");
}
idx += 1;
black_box(rgb.raw.len())
@ -160,7 +160,7 @@ fn bench_pipeline_decode_sequence(c: &mut Criterion) {
b.iter(|| {
for msg_bytes in &messages {
if let Some(union) = extract_union(msg_bytes) {
let _ = decoder.handle_video_frame(
decoder.handle_video_frame(
&union,
&mut rgb,
&mut texture,