Skip to content

Conversation

@Shane32
Copy link
Owner

@Shane32 Shane32 commented Oct 8, 2025

Summary by CodeRabbit

  • New Features

    • Expanded QR code rendering benchmarks with small/medium/large cases and a high-scale (huge) case.
    • Benchmarks can be selected and run via command-line for greater flexibility.
  • Refactor

    • Standardized benchmark class names for consistency.
  • Chores

    • Project updated to target .NET 8 for Windows to support graphics benchmarks.
    • Added .gitignore entry to exclude benchmark artifacts.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Oct 8, 2025

📝 Walkthrough

Walkthrough

Renames several benchmark classes, adds a QRCodeRendererBenchmark with multiple render benchmarks, updates Program to use BenchmarkSwitcher with command-line args, changes project TFM to net8.0-windows, and adds a BenchmarkDotNet.Artifacts/ ignore entry to .gitignore.

Changes

Cohort / File(s) Summary
Git ignore updates
.../.gitignore
Add “# Benchmarks” comment and ignore BenchmarkDotNet.Artifacts/.
Benchmark class renames
QRCoderBenchmarks/BitmapByteQRCodeRendererBenchmark.cs, QRCoderBenchmarks/PngByteQRCodeRendererBenchmark.cs, QRCoderBenchmarks/QRCodeGeneratorBenchmark.cs
Rename public classes and their constructors to include RendererBenchmark or Benchmark suffix; no logic changes.
New renderer benchmark
QRCoderBenchmarks/QRCodeRendererBenchmark.cs
Add new BenchmarkDotNet class that precomputes QRCodeData samples and exposes four render benchmark methods (Small/Medium/Big/Huge).
Benchmark entrypoint change
QRCoderBenchmarks/Program.cs
Replace BenchmarkRunner.Run(typeof(Program).Assembly) with BenchmarkSwitcher.FromAssembly(Assembly.GetExecutingAssembly()).Run(args); and add using System.Reflection.
Project target framework
QRCoderBenchmarks/QRCoderBenchmarks.csproj
Change TargetFramework from net8.0 to net8.0-windows.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor User as CLI User
  participant Program as Program.Main
  participant Switcher as BenchmarkSwitcher
  participant BDN as BenchmarkDotNet
  participant Benches as Benchmarks (Assembly)

  User->>Program: run with args
  Program->>Switcher: FromAssembly(GetExecutingAssembly)
  Program->>Switcher: Run(args)
  Switcher->>BDN: select & invoke benchmarks
  BDN->>Benches: execute selected benchmarks
  Benches-->>BDN: results
  BDN-->>User: reports/output
Loading
sequenceDiagram
  autonumber
  participant Bench as QRCodeRendererBenchmark
  participant Gen as QRCodeGenerator
  participant Data as QRCodeData (precomputed)
  participant QR as QRCode
  participant Renderer as Bitmap renderer

  Note over Bench: Constructor precomputes samples
  Bench->>Gen: create samples (small/medium/big)
  Gen-->>Bench: QRCodeData stored

  Note over Bench: Render methods (Small/Medium/Big/Huge)
  Bench->>Data: retrieve sample
  Bench->>QR: using var qr = new QRCode(data)
  QR->>Renderer: GetGraphic(scale)
  Renderer-->>Bench: Bitmap (disposed)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

  • Add PNG benchmarks #616 — Renames/aligns PngByteQRCodeBenchmark to PngByteQRCodeRendererBenchmark, directly related to benchmark renames in this change.

Suggested labels

performance

Suggested reviewers

  • gfoidl

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title accurately references the addition of the new QRCode renderer benchmark class and the introduction of a menu-style benchmark selection in Program.cs, which are the core changes in this PR. It is concise, clear, and directly reflects the primary enhancements without unnecessary detail.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch benchmarks

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9a7dc3e and e5944c7.

📒 Files selected for processing (1)
  • QRCoderBenchmarks/QRCodeRendererBenchmark.cs (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • QRCoderBenchmarks/QRCodeRendererBenchmark.cs
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Test .NET 5.0 Windows
  • GitHub Check: Test .NET Core 2.1
  • GitHub Check: Test .NET 6.0 Windows
  • GitHub Check: Test .NET Framework 4.6.2
  • GitHub Check: additional-tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Shane32 Shane32 requested a review from gfoidl October 8, 2025 17:41
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
QRCoderBenchmarks/Program.cs (1)

1-4: LGTM!

The switch to BenchmarkSwitcher enables command-line driven benchmark selection, as discussed in previous review comments.

🧹 Nitpick comments (1)
QRCoderBenchmarks/QRCodeRendererBenchmark.cs (1)

12-21: Consider extracting shared sample initialization.

The constructor initializes the same samples as BitmapByteQRCodeRendererBenchmark and PngByteQRCodeRendererBenchmark. Consider extracting this to a shared base class or helper to reduce duplication.

Example approach:

public abstract class QRCodeRendererBenchmarkBase
{
    protected readonly Dictionary<string, QRCodeData> _samples;

    protected QRCodeRendererBenchmarkBase()
    {
        var eccLvl = QRCoder.QRCodeGenerator.ECCLevel.L;
        _samples = new Dictionary<string, QRCodeData>()
        {
            { "small", QRCoder.QRCodeGenerator.GenerateQrCode("ABCD", eccLvl) },
            { "medium", QRCoder.QRCodeGenerator.GenerateQrCode("https://github.com/Shane32/QRCoder/blob/f89aa90081f369983a9ba114e49cc6ebf0b2a7b1/QRCoder/Framework4.0Methods/Stream4Methods.cs", eccLvl) },
            { "big", QRCoder.QRCodeGenerator.GenerateQrCode(new string('a', 2600), eccLvl) }
        };
    }
}

Then each benchmark class inherits from this base.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 47d2c2d and 9a7dc3e.

📒 Files selected for processing (7)
  • .gitignore (1 hunks)
  • QRCoderBenchmarks/BitmapByteQRCodeRendererBenchmark.cs (1 hunks)
  • QRCoderBenchmarks/PngByteQRCodeRendererBenchmark.cs (1 hunks)
  • QRCoderBenchmarks/Program.cs (1 hunks)
  • QRCoderBenchmarks/QRCodeGeneratorBenchmark.cs (1 hunks)
  • QRCoderBenchmarks/QRCodeRendererBenchmark.cs (1 hunks)
  • QRCoderBenchmarks/QRCoderBenchmarks.csproj (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
QRCoderBenchmarks/QRCodeRendererBenchmark.cs (3)
QRCoderBenchmarks/BitmapByteQRCodeRendererBenchmark.cs (5)
  • MemoryDiagnoser (6-50)
  • Benchmark (23-28)
  • Benchmark (30-35)
  • Benchmark (37-42)
  • Benchmark (44-49)
QRCoderBenchmarks/PngByteQRCodeRendererBenchmark.cs (5)
  • MemoryDiagnoser (6-50)
  • Benchmark (23-28)
  • Benchmark (30-35)
  • Benchmark (37-42)
  • Benchmark (44-49)
QRCoderBenchmarks/QRCodeGeneratorBenchmark.cs (4)
  • MemoryDiagnoser (5-31)
  • Benchmark (8-14)
  • Benchmark (16-22)
  • Benchmark (24-30)
🪛 GitHub Actions: Format (Pull Request)
QRCoderBenchmarks/QRCoderBenchmarks.csproj

[error] 1-1: Required references did not load for QRCoder(netstandard1.3) or referenced project. Run dotnet restore prior to formatting.

QRCoderBenchmarks/QRCodeRendererBenchmark.cs

[error] 1-1: IMPORTS: Fix imports ordering.

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
  • GitHub Check: Test .NET 6.0 Windows
  • GitHub Check: Test .NET Core 3.1
  • GitHub Check: Test .NET Core 2.1
  • GitHub Check: Test .NET 5.0 Windows
  • GitHub Check: additional-tests
🔇 Additional comments (6)
QRCoderBenchmarks/QRCoderBenchmarks.csproj (1)

5-5: Appropriate framework change for Windows-specific APIs.

The change to net8.0-windows is correct since QRCodeRendererBenchmark.cs uses System.Drawing, which requires Windows. The pipeline failure about dotnet restore appears to be an environmental issue rather than a problem with this framework change.

.gitignore (1)

226-228: LGTM!

Standard ignore pattern for BenchmarkDotNet artifacts with a clear comment.

QRCoderBenchmarks/QRCodeGeneratorBenchmark.cs (1)

6-6: LGTM!

The rename to QRCodeGeneratorBenchmark appropriately distinguishes this benchmark class from the actual QRCoder.QRCodeGenerator class being tested.

QRCoderBenchmarks/PngByteQRCodeRendererBenchmark.cs (1)

7-11: LGTM!

The class and constructor rename to include "Renderer" is consistent with the naming pattern used in BitmapByteQRCodeRendererBenchmark.

QRCoderBenchmarks/BitmapByteQRCodeRendererBenchmark.cs (1)

7-11: LGTM!

The rename clarifies that this benchmark focuses on the BitmapByte renderer implementation.

QRCoderBenchmarks/QRCodeRendererBenchmark.cs (1)

23-49: LGTM!

The benchmark methods appropriately test QR code rendering at different sizes and scales, correctly disposing of bitmaps via using statements.

@Shane32 Shane32 requested a review from gfoidl October 8, 2025 20:12
@Shane32 Shane32 merged commit 161aa5f into master Oct 8, 2025
8 checks passed
@Shane32 Shane32 deleted the benchmarks branch October 8, 2025 20:18
@Shane32 Shane32 added the performance Performance related enhancements or benchmarks label Oct 8, 2025
@Shane32 Shane32 self-assigned this Oct 8, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

performance Performance related enhancements or benchmarks

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants