perf: rebuild Multimap canonicalization without intermediate vectors/maps by jiuker · Pull Request #231 · minio/minio-cpp · GitHub

…maps

perf: rebuild Multimap canonicalization without intermediate vectors/maps
fix #220

Summary by CodeRabbit

Release Notes

Refactor
- Optimized internal header and query string processing to improve efficiency.

…maps perf: rebuild Multimap canonicalization without intermediate vectors/maps

📝 Walkthrough

Walkthrough

The PR refactors Multimap canonicalization methods in src/utils.cc to eliminate intermediate container allocations. A new helper function collapses consecutive whitespace. GetCanonicalHeaders() and GetCanonicalQueryString() are rewritten to build results directly during map iteration. ToQueryString() is reintroduced as a delegation wrapper.

Changes

Multimap Canonicalization Optimization

Layer / File(s)	Summary
Whitespace normalization helper src/utils.cc	New internal removeExtraSpaces() function collapses consecutive spaces and trims boundaries by iterating the input once.
Canonical headers optimization src/utils.cc	GetCanonicalHeaders() rewritten to build signed_headers and canonical_headers directly during map_ iteration, using lowercased header names, skipping authorization and user-agent, and normalizing values via removeExtraSpaces() instead of regex+trim and separate sorting.
Canonical query string and ToQueryString refactoring src/utils.cc	GetCanonicalQueryString() rewritten to concatenate escaped key=value pairs in map iteration order without explicit sorting. ToQueryString() removed from original location and reintroduced as a thin wrapper that delegates to GetCanonicalQueryString().

Sequence Diagram(s)

No sequence diagrams needed; the changes are internal optimizations to individual methods without new cross-component interactions or control flow alterations.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Suggested reviewers

harshavardhana

Poem

🐰 A rabbit hops through canonicalized strings,
No extra spaces, no intermediate things—
One pass, no vectors, the map leads the way,
Query and headers, lighter today! ✨

🚥 Pre-merge checks | ✅ 5 ✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The PR title accurately and concisely describes the main objective: performance optimization by refactoring Multimap canonicalization to eliminate intermediate vectors/maps.
Linked Issues check	✅ Passed	The PR implementation directly addresses all coding requirements from issue #220: eliminating intermediate containers, using single-pass construction with reserve(), leveraging std::map iteration order, and removing heavy operations like regex_replace.
Out of Scope Changes check	✅ Passed	All changes in src/utils.cc are scoped to the three canonicalization functions identified in issue #220; no unrelated modifications or refactoring outside these targets were introduced.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ 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/utils.cc

src/utils.cc:18:10: fatal error: 'miniocpp/utils.h' file not found
18 | #include "miniocpp/utils.h"
| ^~~~~~~~~~~~~~~~~~
1 error generated.
src/utils.cc:301:3-10: ERROR translating statement 'ReturnStmt'
Aborting translation of method 'minio::utils::Base64Encode' in file 'src/utils.cc': "Assert_failure src/clang/cAst_utils.ml:249:53"
Uncaught Internal Error: "Assert_failure src/clang/cAst_utils.ml:249:53"
Error backtrace:
Raised at ClangFrontend__CAst_utils.get_decl_from_typ_ptr in file "src/clang/cAst_utils.ml", line 249, characters 53-65
Called from ClangFrontend__CTrans.CTrans_funct.get_destructor_decl_ref in file "src/clang/cTrans.ml", line 658, characters 12-59
Called from ClangFrontend__CTrans.CTrans_funct.destructor_calls.(fun) in file "src/clang/cTrans.ml", line 2048, characters 12-69
Called from Base__List.rev_filter_map.loop in file "src/list.ml", line 944, characters 13-17
Called from Base__List.filter_map in file "src/list.ml" (inlined), line 951, characte

... [truncated 2200 characters] ...

m ClangFrontend__CTrans.CTrans_funct.instruction in file "src/clang/cTrans.ml" (inlined), line 4765, characters 38-71
Called from ClangFrontend__CTrans.CTrans_funct.exec_with_node_creation in file "src/clang/cTrans.ml" (inlined), line 104, characters 20-38
Called from ClangFrontend__CTrans.CTrans_funct.get_clang_stmt_trans in file "src/clang/cTrans.ml" (inlined), line 5395, characters 4-69
Called from ClangFrontend__CTrans.CTrans_funct.get_custom_stmt_trans in file "src/clang/cTrans.ml", line 5401, characters 8-55
Called from ClangFrontend__CTrans.CTrans_funct.exec_trans_instrs.exec_trans_instrs_rev in file "src/clang/cTrans.ml" (inlined), line 5365, characters 28-54
Called from ClangFrontend__CTrans.CTrans_funct.exec_trans_instrs in file "src/clang/cTrans.ml" (inlined), line 5389, charact

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

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/utils.cc`:
- Around line 583-599: The loop currently iterates map_ and lowercases each key
later, causing mixed-case duplicates and non-deterministic ordering; instead
iterate keys_ (which should contain the canonical/lowercased header names) and
for each key k append it to signed_headers/canonical_headers without calling
ToLower on map keys, then gather and join all values from map_ entries whose
original key lowercased equals k (so case-variants are merged) using
removeExtraSpaces before joining with commas; update the logic that builds
signed_headers and canonical_headers (symbols: keys_, map_, ToLower,
signed_headers, canonical_headers, removeExtraSpaces) so the header order and
value aggregation are stable and case-insensitive.

🪄 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: 257b6fe7-2de7-4ef2-a407-913ad084f9e8

📥 Commits

Reviewing files that changed from the base of the PR and between a6f124b and 507f064.

📒 Files selected for processing (1)

src/utils.cc

perf: rebuild Multimap canonicalization without intermediate vectors/…

507f064

…maps perf: rebuild Multimap canonicalization without intermediate vectors/maps

jiuker changed the title perf: rebuild Multimap canonicalization without intermediate vectors/… perf: rebuild Multimap canonicalization without intermediate vectors/maps Jun 10, 2026

jiuker requested a review from harshavardhana June 10, 2026 10:37

coderabbitai Bot reviewed Jun 10, 2026

View reviewed changes

Comment thread src/utils.cc

harshavardhana approved these changes Jun 10, 2026

View reviewed changes

harshavardhana merged commit 7c503ae into minio:main Jun 10, 2026
8 checks passed

perf: rebuild Multimap canonicalization without intermediate vectors/maps by jiuker · Pull Request #231 · minio/minio-cpp · GitHub

Uh oh!

perf: rebuild Multimap canonicalization without intermediate vectors/maps - #231

perf: rebuild Multimap canonicalization without intermediate vectors/maps#231
harshavardhana merged 1 commit into
minio:mainfrom
jiuker:fix-issue-220

jiuker commented Jun 10, 2026 •
edited by coderabbitai Bot
Loading

Uh oh!

Uh oh!

coderabbitai Bot commented Jun 10, 2026 •
edited
Loading

Uh oh!

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

perf: rebuild Multimap canonicalization without intermediate vectors/maps by jiuker · Pull Request #231 · minio/minio-cpp · GitHub

Uh oh!

perf: rebuild Multimap canonicalization without intermediate vectors/maps - #231

perf: rebuild Multimap canonicalization without intermediate vectors/maps#231harshavardhana merged 1 commit intominio:mainminio/minio-cpp:mainfrom jiuker:fix-issue-220jiuker/minio-cpp:fix-issue-220Copy head branch name to clipboard

Conversation

jiuker commented Jun 10, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented Jun 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Sequence Diagram(s)

Estimated code review effort

Suggested reviewers

Poem

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

perf: rebuild Multimap canonicalization without intermediate vectors/maps#231
harshavardhana merged 1 commit into
minio:mainfrom
jiuker:fix-issue-220

jiuker commented Jun 10, 2026 •
edited by coderabbitai Bot
Loading

coderabbitai Bot commented Jun 10, 2026 •
edited
Loading