Skip to content

Comments

fix(C++): Fix reversed parameters and incorrect calls in string_util benchmark#2246

Merged
pandalee99 merged 5 commits intoapache:mainfrom
lzaeh:feat/improve-benchmark-uti
May 19, 2025
Merged

fix(C++): Fix reversed parameters and incorrect calls in string_util benchmark#2246
pandalee99 merged 5 commits intoapache:mainfrom
lzaeh:feat/improve-benchmark-uti

Conversation

@lzaeh
Copy link
Contributor

@lzaeh lzaeh commented May 19, 2025

What does this PR do?

This PR fixes several minor issues and improves the implementation of string utility benchmarks:

  • Corrected reversed function parameters.
  • Fixed an incorrect function call.
  • Replaced some usages of auto with explicit types for better readability and maintainability.
  • Refactored random number generation by encapsulating std::mt19937 in a thread-local helper function.

One part of the original code used default random generation methods, which could result in uneven value distributions and non-deterministic behavior across platforms. To improve randomness quality and thread-safety, I replaced it with a thread_local encapsulated std::mt19937 generator. This ensures better reproducibility and consistency during benchmarking.

After these changes, the code compiles and runs correctly. Benchmark results are stable and as expected. (See image below.)

Screenshot 2025-05-19 at 17 24 58

Related issues

Does this PR introduce any user-facing change?

  • Does this PR introduce any public API change?
  • Does this PR introduce any binary protocol compatibility change?

Benchmark

… replace auto with explicit types; optimize implementation
@lzaeh lzaeh changed the title Fix reversed parameters and incorrect calls in string_util benchmark fix(C++): Fix reversed parameters and incorrect calls in string_util benchmark May 19, 2025
@pandalee99 pandalee99 self-requested a review May 19, 2025 09:34
@lzaeh lzaeh requested a review from theweipeng as a code owner May 19, 2025 10:12
Copy link
Contributor

@pandalee99 pandalee99 left a comment

Choose a reason for hiding this comment

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

LGTM

@pandalee99 pandalee99 merged commit 308ea2c into apache:main May 19, 2025
50 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.

2 participants