Skip to content

perf: replace O(n²) hex-encoding loops with std::to_chars - #230

Merged
harshavardhana merged 2 commits into
minio:mainfrom
jiuker:fix-issue-219
Jun 10, 2026
Merged

perf: replace O(n²) hex-encoding loops with std::to_chars#230
harshavardhana merged 2 commits into
minio:mainfrom
jiuker:fix-issue-219

Conversation

@jiuker

@jiuker jiuker commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

perf: replace O(n²) hex-encoding loops with std::to_chars
fix #219

Summary by CodeRabbit

  • Refactor
    • Optimized internal cryptographic routines for signature and hash generation by improving hex-encoding performance and reducing temporary allocations. These changes preserve existing outputs and public interfaces while improving speed and reliability of signing and hashing operations.

perf: replace O(n²) hex-encoding loops with std::to_chars
@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 102a2521-9652-46b9-8309-f6d3b1210d0e

📥 Commits

Reviewing files that changed from the base of the PR and between 1e53dbd and 38a2a3f.

📒 Files selected for processing (1)
  • src/signer.cc
🚧 Files skipped from review as they are similar to previous changes (1)
  • src/signer.cc

📝 Walkthrough

Walkthrough

This PR replaces O(n²) hex-encoding loops in two hot signing paths (GetSignature and Sha256Hash) with O(n) implementations using std::to_chars, eliminating repeated string reallocations by pre-allocating output buffers once and writing hex digits directly.

Changes

Hex encoding optimization

Layer / File(s) Summary
GetSignature hex encoding
src/signer.cc
Adds <charconv> and refactors the signature hex formatting loop from snprintf("%02x") with incremental += to a pre-sized string buffer with std::to_chars direct writes and manual zero-padding when needed.
Sha256Hash hex encoding
src/utils.cc
Adds <charconv> and refactors the hash hex string construction from per-byte snprintf appends to direct buffer writes via std::to_chars, with zero-padding normalization to handle single-character outputs.

🎯 2 (Simple) | ⏱️ ~10 minutes

A rabbit hops through the code so fast,
No more allocations lurking in the past,
Two strings now write in one swift pass,
Performance gains—so snappy, so crass! 🐰✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ 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%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the main change: replacing O(n²) hex-encoding loops with std::to_chars in hot signing paths.
Linked Issues check ✅ Passed The PR implementation matches all coding requirements: both hot paths (GetSignature in src/signer.cc and Sha256Hash in src/utils.cc) now use std::to_chars with pre-sized buffers instead of snprintf+concatenation loops.
Out of Scope Changes check ✅ Passed All changes are directly scoped to the linked issue #219: modifications target only the two identified hex-encoding paths with no unrelated alterations.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Warning

There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure.

🔧 Infer (1.2.0)
src/signer.cc

src/signer.cc:18:10: fatal error: 'miniocpp/signer.h' file not found
18 | #include "miniocpp/signer.h"
| ^~~~~~~~~~~~~~~~~~~
1 error generated.
Error: the following clang command did not run successfully:
/opt/infer-linux-x86_64-v1.2.0/lib/infer/facebook-clang-plugins/clang/install/bin/clang-18
@/tmp/coderabbit-infer/38a2a3f3458888b53174d8cc1d48ba0b4552a9c5-e7aff5a48cec001f/tmp/clang_command_.tmp.d56d74.txt
++Contents of '/tmp/coderabbit-infer/38a2a3f3458888b53174d8cc1d48ba0b4552a9c5-e7aff5a48cec001f/tmp/clang_command_.tmp.d56d74.txt':
"-cc1" "-load"
"/opt/infer-linux-x86_64-v1.2.0/lib/infer/infer/bin/../../facebook-clang-plugins/libtooling/build/FacebookClangPlugin.dylib"
"-add-plugin" "BiniouASTExporter" "-plugin-arg-BiniouASTExporter" "-"
"-plugin-arg-BiniouASTExporter" "PREPEND_CURRENT_DIR=1"
"-plugin-arg-BiniouASTExporter" "MAX_STRING_SIZE=65535" "-cc1" "-triple"
"x86_64-unknown-linux-gnu" "-emit-obj" "-mrelax-all" "-disable-free

... [truncated 1045 characters] ...

plugins/clang/install/lib/clang/18/include"
"-internal-isystem" "/usr/local/include" "-internal-isystem"
"/usr/lib/gcc/x86_64-linux-gnu/12/../../../../x86_64-linux-gnu/include"
"-internal-externc-isystem" "/usr/include/x86_64-linux-gnu"
"-internal-externc-isystem" "/include" "-internal-externc-isystem"
"/usr/include" "-Wno-ignored-optimization-argument" "-Wno-everything"
"-fdeprecated-macro" "-ferror-limit" "19" "-fgnuc-version=4.2.1"
"-fskip-odr-check-in-gmf" "-fcxx-exceptions" "-fexceptions"
"-D__GCC_HAVE_DWARF2_CFI_ASM=1" "-o"
"/tmp/coderabbit-infer/e7aff5a48cec001f/file.o" "-x" "c++"
"src/signer.cc" "-O0" "-fno-builtin" "-include"
"/opt/infer-linux-x86_64-v1.2.0/lib/infer/infer/bin/../lib/clang_wrappers/global_defines.h"
"-Wno-everything"


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

@jiuker
jiuker requested a review from harshavardhana June 10, 2026 04:51

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@src/signer.cc`:
- Around line 93-97: The loop converting HMAC bytes to hex uses
std::to_chars(..., hash[i], 16) which can treat char as signed and produce
negative values; fix by casting each byte to an unsigned type before conversion
(e.g., convert hash[i] to unsigned char or uint8_t and then to an integer type)
so std::to_chars sees 0..255 values. Update the call site around std::to_chars
and the hash[i] argument in signer.cc (the to_chars invocation and the
surrounding byte-to-hex logic) to use the unsigned-cast value, leaving the rest
of the hex-padding logic unchanged.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 02f84aef-c08c-41ea-b889-6d901a8a5b20

📥 Commits

Reviewing files that changed from the base of the PR and between f35873b and 1e53dbd.

📒 Files selected for processing (2)
  • src/signer.cc
  • src/utils.cc

Comment thread src/signer.cc Outdated
@harshavardhana
harshavardhana merged commit a6f124b into minio:main Jun 10, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

perf: replace O(n²) hex-encoding loops with std::to_chars

2 participants