Skip to content

chore: optimize string manipulation functions and improve error handling#167

Merged
ReneWerner87 merged 10 commits intomasterfrom
optimize_fns
Dec 5, 2025
Merged

chore: optimize string manipulation functions and improve error handling#167
ReneWerner87 merged 10 commits intomasterfrom
optimize_fns

Conversation

@ReneWerner87
Copy link
Copy Markdown
Member

@ReneWerner87 ReneWerner87 commented Dec 5, 2025

                                                      │    old.txt    │               new.txt                │
                                                      │    sec/op     │    sec/op      vs base               │
_ConvertToBytes/fiber-12                                10.075n ±  3%    7.393n ±  0%  -26.62% (p=0.002 n=6)
_ToString/int-12                                         2.599n ±  1%    2.663n ±  0%   +2.50% (p=0.002 n=6)
_ToString/int8-12                                        2.597n ±  1%    2.668n ±  1%   +2.71% (p=0.002 n=6)
_ToString/int16-12                                       2.599n ±  1%    2.667n ±  1%   +2.62% (p=0.002 n=6)
_ToString/int32-12                                       2.598n ±  1%    2.665n ±  1%   +2.58% (p=0.002 n=6)
_ToString/int64-12                                       2.594n ±  0%    2.667n ±  1%   +2.79% (p=0.002 n=6)
_ToString/uint-12                                        2.592n ±  1%    2.074n ±  0%  -20.02% (p=0.002 n=6)
_ToString/uint8-12                                       2.591n ±  1%    2.075n ±  0%  -19.88% (p=0.002 n=6)
_ToString/uint16-12                                      2.592n ±  1%    2.073n ±  1%  -20.02% (p=0.002 n=6)
_ToString/uint32-12                                      2.592n ±  1%    2.075n ±  1%  -19.96% (p=0.002 n=6)
_ToString/uint64-12                                      2.594n ±  0%    2.074n ±  1%  -20.07% (p=0.002 n=6)
_ToString/string-12                                      1.936n ±  0%    1.944n ±  0%   +0.39% (p=0.002 n=6)
_ToString/[]uint8-12                                     12.02n ±  1%    12.11n ±  0%   +0.83% (p=0.050 n=6)
_ToString/time.Time-12                                   91.41n ±  1%    91.67n ±  1%   +0.29% (p=0.045 n=6)
_ToString/time.Time#01-12                                90.74n ±  0%    91.27n ±  1%   +0.59% (p=0.009 n=6)
_ToString/[]string-12                                    82.20n ±  1%    22.31n ±  1%  -72.85% (p=0.002 n=6)
_ToString/[]int-12                                       57.88n ±  0%    23.58n ±  1%  -59.26% (p=0.002 n=6)
_ToString/[2]int-12                                      41.93n ±  2%    37.62n ±  1%  -10.29% (p=0.002 n=6)
_ToString/[][]int-12                                     228.7n ±  2%    133.2n ±  1%  -41.74% (p=0.002 n=6)
_ToString/[]interface_{}-12                              261.8n ±  1%    118.0n ±  0%  -54.92% (p=0.002 n=6)
_ToString/utils.MyStringer-12                            3.883n ±  2%    2.963n ±  1%  -23.69% (p=0.002 n=6)
_ByteSize/1-12                                           17.88n ±  1%    17.37n ±  1%   -2.85% (p=0.002 n=6)
_ByteSize/500-12                                         22.49n ±  2%    19.62n ±  1%  -12.76% (p=0.002 n=6)
_ByteSize/1024-12                                        19.34n ±  1%    18.96n ±  0%   -1.96% (p=0.002 n=6)
_ByteSize/1126-12                                        22.54n ±  1%    21.96n ±  1%   -2.57% (p=0.002 n=6)
_ByteSize/1048576-12                                     19.02n ±  1%    18.75n ±  1%   -1.45% (p=0.002 n=6)
_ByteSize/1153024-12                                     22.21n ±  0%    21.65n ±  1%   -2.50% (p=0.002 n=6)
_ByteSize/1073741824-12                                  18.82n ±  1%    18.36n ±  0%   -2.50% (p=0.002 n=6)
_ByteSize/1180696576-12                                  21.93n ±  1%    21.36n ±  1%   -2.60% (p=0.002 n=6)
_ByteSize/1099511627776-12                               18.41n ±  1%    18.07n ±  1%   -1.82% (p=0.002 n=6)
_ByteSize/1209033293824-12                               21.64n ±  2%    21.03n ±  1%   -2.84% (p=0.002 n=6)
_ByteSize/1125899906842624-12                            18.17n ±  1%    17.77n ±  1%   -2.17% (p=0.002 n=6)
_ByteSize/1238050092875776-12                            21.28n ±  0%    20.79n ±  1%   -2.30% (p=0.002 n=6)
_ByteSize/1267763295104794624-12                         20.97n ±  1%    20.74n ±  2%   -1.07% (p=0.035 n=6)
_GetMIME/fiber-12                                        40.44n ±  1%    39.98n ±  1%   -1.14% (p=0.004 n=6)
_ParseVendorSpecificContentType/vendorContentType-12     26.79n ±  1%    25.99n ±  1%   -2.99% (p=0.002 n=6)
_ParseUint/fiber_bytes-12                                7.171n ±  1%    7.087n ±  2%   -1.18% (p=0.026 n=6)
_ParseInt/fiber-12                                       7.095n ±  1%    7.004n ±  1%   -1.28% (p=0.015 n=6)
_ParseInt/fiber_bytes-12                                 7.470n ±  1%    7.333n ±  1%   -1.83% (p=0.002 n=6)
_ParseInt16/fiber_bytes-12                               5.183n ±  2%    5.064n ±  3%   -2.31% (p=0.045 n=6)
_ParseInt8/fiber-12                                      4.429n ±  1%    4.638n ±  2%   +4.72% (p=0.002 n=6)
_ParseInt8/fiber_bytes-12                                4.417n ±  1%    4.551n ±  1%   +3.05% (p=0.002 n=6)
_ParseUint32/fiber-12                                    6.843n ±  0%    6.978n ±  1%   +1.97% (p=0.002 n=6)
_ParseUint32/fiber_bytes-12                              7.128n ±  1%    7.282n ±  1%   +2.15% (p=0.004 n=6)
_ParseUint16/fiber-12                                    4.566n ±  1%    4.685n ±  1%   +2.61% (p=0.002 n=6)
_ParseUint16/fiber_bytes-12                              4.704n ±  1%    4.785n ±  1%   +1.73% (p=0.009 n=6)
_ParseUint8/fiber-12                                     3.657n ±  1%    3.588n ±  1%   -1.87% (p=0.002 n=6)
Benchmark_FormatUint/small/fiber-12                     	1000000000	         0.2871 ns/op	       0 B/op	       0 allocs/op
Benchmark_FormatUint/small/fiber-12                     	1000000000	         0.2902 ns/op	       0 B/op	       0 allocs/op
Benchmark_FormatUint/small/strconv-12                   	599065209	         2.010 ns/op	       0 B/op	       0 allocs/op
Benchmark_FormatUint/small/strconv-12                   	595841398	         2.029 ns/op	       0 B/op	       0 allocs/op
Benchmark_FormatUint/medium/fiber-12                    	130085965	         9.240 ns/op	       0 B/op	       0 allocs/op
Benchmark_FormatUint/medium/fiber-12                    	129899154	         9.309 ns/op	       0 B/op	       0 allocs/op
Benchmark_FormatUint/medium/strconv-12                  	61112242	        19.82 ns/op	      16 B/op	       1 allocs/op
Benchmark_FormatUint/medium/strconv-12                  	60156405	        20.10 ns/op	      16 B/op	       1 allocs/op
Benchmark_FormatUint/large/fiber-12                     	67980163	        18.22 ns/op	       0 B/op	       0 allocs/op
Benchmark_FormatUint/large/fiber-12                     	65608428	        18.17 ns/op	       0 B/op	       0 allocs/op
Benchmark_FormatUint/large/strconv-12                   	47258048	        25.55 ns/op	      24 B/op	       1 allocs/op
Benchmark_FormatUint/large/strconv-12                   	47346849	        25.56 ns/op	      24 B/op	       1 allocs/op
Benchmark_FormatInt/small_pos/fiber-12                  	600536476	         2.002 ns/op	       0 B/op	       0 allocs/op
Benchmark_FormatInt/small_pos/fiber-12                  	602514745	         1.990 ns/op	       0 B/op	       0 allocs/op
Benchmark_FormatInt/small_pos/strconv-12                	598452873	         2.008 ns/op	       0 B/op	       0 allocs/op
Benchmark_FormatInt/small_pos/strconv-12                	600354334	         2.028 ns/op	       0 B/op	       0 allocs/op
Benchmark_FormatInt/small_neg/fiber-12                  	100000000	        11.76 ns/op	       3 B/op	       1 allocs/op
Benchmark_FormatInt/small_neg/fiber-12                  	100000000	        11.77 ns/op	       3 B/op	       1 allocs/op
Benchmark_FormatInt/small_neg/strconv-12                	89750936	        13.47 ns/op	       3 B/op	       1 allocs/op
Benchmark_FormatInt/small_neg/strconv-12                	89387971	        13.45 ns/op	       3 B/op	       1 allocs/op
Benchmark_FormatInt/medium_pos/fiber-12                 	63039834	        18.84 ns/op	      16 B/op	       1 allocs/op
Benchmark_FormatInt/medium_pos/fiber-12                 	65529009	        18.45 ns/op	      16 B/op	       1 allocs/op
Benchmark_FormatInt/medium_pos/strconv-12               	60842155	        19.58 ns/op	      16 B/op	       1 allocs/op
Benchmark_FormatInt/medium_pos/strconv-12               	58988349	        19.66 ns/op	      16 B/op	       1 allocs/op
Benchmark_FormatInt/medium_neg/fiber-12                 	65295759	        18.43 ns/op	      16 B/op	       1 allocs/op
Benchmark_FormatInt/medium_neg/fiber-12                 	64023048	        18.84 ns/op	      16 B/op	       1 allocs/op
Benchmark_FormatInt/medium_neg/strconv-12               	63172580	        19.21 ns/op	      16 B/op	       1 allocs/op
Benchmark_FormatInt/medium_neg/strconv-12               	62150941	        19.10 ns/op	      16 B/op	       1 allocs/op
Benchmark_FormatInt/large_pos/fiber-12                  	49343880	        24.12 ns/op	      24 B/op	       1 allocs/op
Benchmark_FormatInt/large_pos/fiber-12                  	49661936	        24.31 ns/op	      24 B/op	       1 allocs/op
Benchmark_FormatInt/large_pos/strconv-12                	47172282	        25.26 ns/op	      24 B/op	       1 allocs/op
Benchmark_FormatInt/large_pos/strconv-12                	46397208	        25.21 ns/op	      24 B/op	       1 allocs/op
Benchmark_FormatInt/large_neg/fiber-12                  	49492189	        24.34 ns/op	      24 B/op	       1 allocs/op
Benchmark_FormatInt/large_neg/fiber-12                  	48588241	        24.44 ns/op	      24 B/op	       1 allocs/op
Benchmark_FormatInt/large_neg/strconv-12                	47608340	        25.14 ns/op	      24 B/op	       1 allocs/op
Benchmark_FormatInt/large_neg/strconv-12                	44964800	        25.67 ns/op	      24 B/op	       1 allocs/op
Benchmark_FormatUint32/fiber-12                         	130348042	         9.196 ns/op	       0 B/op	       0 allocs/op
Benchmark_FormatUint32/fiber-12                         	130270428	         9.176 ns/op	       0 B/op	       0 allocs/op
Benchmark_FormatUint32/strconv-12                       	60917570	        19.55 ns/op	      16 B/op	       1 allocs/op
Benchmark_FormatUint32/strconv-12                       	60743475	        19.84 ns/op	      16 B/op	       1 allocs/op
Benchmark_FormatInt32/fiber-12                          	65395689	        18.56 ns/op	      16 B/op	       1 allocs/op
Benchmark_FormatInt32/fiber-12                          	65174740	        18.57 ns/op	      16 B/op	       1 allocs/op
Benchmark_FormatInt32/strconv-12                        	63959209	        19.16 ns/op	      16 B/op	       1 allocs/op
Benchmark_FormatInt32/strconv-12                        	62956185	        19.12 ns/op	      16 B/op	       1 allocs/op
Benchmark_FormatUint16/fiber-12                         	165817112	         7.337 ns/op	       0 B/op	       0 allocs/op
Benchmark_FormatUint16/fiber-12                         	165319921	         7.227 ns/op	       0 B/op	       0 allocs/op
Benchmark_FormatUint16/strconv-12                       	76576396	        15.58 ns/op	       5 B/op	       1 allocs/op
Benchmark_FormatUint16/strconv-12                       	78519046	        15.32 ns/op	       5 B/op	       1 allocs/op
Benchmark_FormatInt16/fiber-12                          	80419747	        14.99 ns/op	       8 B/op	       1 allocs/op
Benchmark_FormatInt16/fiber-12                          	79379295	        15.01 ns/op	       8 B/op	       1 allocs/op
Benchmark_FormatInt16/strconv-12                        	79775299	        15.05 ns/op	       8 B/op	       1 allocs/op
Benchmark_FormatInt16/strconv-12                        	80716579	        15.05 ns/op	       8 B/op	       1 allocs/op
Benchmark_FormatUint8/fiber-12                          	239991619	         4.960 ns/op	       0 B/op	       0 allocs/op
Benchmark_FormatUint8/fiber-12                          	242351924	         5.000 ns/op	       0 B/op	       0 allocs/op
Benchmark_FormatUint8/strconv-12                        	85645719	        13.87 ns/op	       3 B/op	       1 allocs/op
Benchmark_FormatUint8/strconv-12                        	87952087	        13.92 ns/op	       3 B/op	       1 allocs/op
Benchmark_FormatInt8/fiber-12                           	99749586	        11.88 ns/op	       4 B/op	       1 allocs/op
Benchmark_FormatInt8/fiber-12                           	99660878	        11.86 ns/op	       4 B/op	       1 allocs/op
Benchmark_FormatInt8/strconv-12                         	85782709	        14.06 ns/op	       4 B/op	       1 allocs/op
Benchmark_FormatInt8/strconv-12                         	84831132	        14.03 ns/op	       4 B/op	       1 allocs/op
Benchmark_AppendUint/fiber-12                           	142098361	         8.447 ns/op	       0 B/op	       0 allocs/op
Benchmark_AppendUint/fiber-12                           	142081795	         8.452 ns/op	       0 B/op	       0 allocs/op
Benchmark_AppendUint/strconv-12                         	100000000	        10.03 ns/op	       0 B/op	       0 allocs/op
Benchmark_AppendUint/strconv-12                         	120097623	        10.02 ns/op	       0 B/op	       0 allocs/op
Benchmark_AppendInt/fiber-12                            	129486499	         9.313 ns/op	       0 B/op	       0 allocs/op
Benchmark_AppendInt/fiber-12                            	129404218	         9.266 ns/op	       0 B/op	       0 allocs/op
Benchmark_AppendInt/strconv-12                          	123666277	         9.715 ns/op	       0 B/op	       0 allocs/op
Benchmark_AppendInt/strconv-12                          	123572066	         9.700 ns/op	       0 B/op	       0 allocs/op
  • Bug Fixes

    • Fixed edge-case handling for empty input in numeric parsing, returning consistent empty-number errors.
    • Improved MIME type detection reliability.
  • Performance Improvements

    • Faster, lower-allocation string/byte formatting and conversion paths.
    • Reduced reflection and memory allocations for common types; optimized integer formatting with caching.
  • New Features

    • Expanded direct handling for common pointer and slice types to improve conversion speed.
  • Tests

    • Added comprehensive unit tests and detailed benchmarks for formatting and conversion utilities.

✏️ Tip: You can customize this high-level summary in your review settings.

Summary by CodeRabbit

  • New Features

    • Added formatting functions for efficient integer-to-string conversion across multiple data types.
  • Bug Fixes

    • Improved error reporting for edge cases in number parsing.
    • Enhanced nil-pointer handling in type conversions.
  • Tests

    • Added comprehensive test coverage for formatting utilities and nil-pointer scenarios.
  • Refactor

    • Optimized internal number parsing and string conversion logic.

✏️ Tip: You can customize this high-level summary in your review settings.

Copilot AI review requested due to automatic review settings December 5, 2025 07:56
@ReneWerner87 ReneWerner87 requested a review from a team as a code owner December 5, 2025 07:56
@ReneWerner87 ReneWerner87 requested review from efectn, gaby and sixcolors and removed request for a team December 5, 2025 07:56
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Summary of Changes

Hello @ReneWerner87, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request focuses on enhancing the performance and robustness of various utility functions related to string manipulation and type conversion. The changes aim to minimize memory allocations, streamline string processing, and improve the accuracy of error reporting in parsing routines, leading to a more efficient and reliable codebase.

Highlights

  • Performance Optimization: Significantly optimized string manipulation functions like ToLower, ToUpper, ByteSize, ToString, GetMIME, and ParseVendorSpecificContentType to reduce memory allocations and improve execution speed. This includes using sync.Pool for buffer reuse, pre-allocating strings.Builder capacity, and avoiding redundant string operations.
  • Improved Error Handling: Enhanced error reporting in custom parsing functions (parseSigned, parseUnsigned, parseFloat) by ensuring strconv.NumError provides consistent empty Num fields for empty input strings, aligning with standard library behavior.
  • Refactored Type Conversions: The ToString utility now directly handles common pointer and slice types, bypassing reflection for better performance. It also uses strconv.FormatUint for unsigned integer types for more accurate and consistent formatting.
  • Linter Configuration Update: Updated the .golangci.yml configuration to remove exclusions for bytebufferpool and add exclusions for strings.Builder write operations, reflecting a shift in string handling strategy. The unhandled-error linter has also been temporarily disabled.
Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description
Code Review /gemini review Performs a code review for the current pull request in its current state.
Pull Request Summary /gemini summary Provides a summary of the current pull request in its current state.
Comment @gemini-code-assist Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help /gemini help Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

  1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution.

@codecov
Copy link
Copy Markdown

codecov bot commented Dec 5, 2025

Codecov Report

❌ Patch coverage is 91.96429% with 18 lines in your changes missing coverage. Please review.
✅ Project coverage is 84.54%. Comparing base (71424ca) to head (483fccc).
⚠️ Report is 31 commits behind head on master.

Files with missing lines Patch % Lines
convert.go 75.34% 8 Missing and 10 partials ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #167      +/-   ##
==========================================
+ Coverage   83.31%   84.54%   +1.23%     
==========================================
  Files          11       12       +1     
  Lines         755      938     +183     
==========================================
+ Hits          629      793     +164     
- Misses        115      124       +9     
- Partials       11       21      +10     
Flag Coverage Δ
unittests 84.54% <91.96%> (+1.23%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request introduces several significant performance optimizations and a critical bug fix across various utility functions. The optimizations in functions like ToLower, ToUpper, and ByteSize effectively reduce allocations. The ToString function is also optimized to handle common types without reflection, and more importantly, it fixes a critical bug where unsigned integers were incorrectly converted to strings. While most changes are positive, I have a concern about disabling the unhandled-error linter in .golangci.yml, which could compromise code quality by hiding potential bugs.

Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR aims to optimize string manipulation functions and improve error handling across the utils package. However, several optimization attempts introduce bugs or fail to provide actual performance improvements, and some changes reduce code quality.

Key changes:

  • Refactored ToLower and ToUpper functions with loop unrolling (but with performance regressions)
  • Added a sync.Pool for buffer allocation in ByteSize (incorrectly implemented)
  • Expanded ToString with direct handling of pointer and slice types
  • Simplified error message construction in parsing functions
  • Updated linter configuration to accommodate strings.Builder usage

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
strings.go Refactored ToLower/ToUpper with loop unrolling, but changes allocate full string upfront causing performance regression
strings_test.go Removed t.Parallel() calls, eliminating concurrent test execution and slowing down test suite
parse.go Improved error messages by avoiding unnecessary string conversion for empty inputs
http.go Refactored GetMIME for clearer control flow; attempted optimization in ParseVendorSpecificContentType that provides no benefit
convert.go Added sync.Pool for ByteSize (with critical bug in type assertion), expanded ToString with pointer/slice type handling, fixed uint conversion functions
common.go Refactored ConvertToBytes with manual byte scanning that's slower than standard library
.golangci.yml Updated linter config for strings.Builder, but disabled unhandled-error rule entirely instead of targeted exclusions

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Dec 5, 2025

Note

Other AI code review bot(s) detected

CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.

Walkthrough

Refactors numeric parsing/formatting and MIME helpers: adds a zero-allocation integer formatter and tests, replaces strconv usage with custom Format/Append helpers across conversions, rewrites byte-size digit/unit detection and parse error semantics, and restructures string tests/benchmarks and MIME normalization.

Changes

Cohort / File(s) Summary
Formatting utilities
format.go, format_test.go
New high-performance integer formatting module with small-int caches (0–99), buffer-based non-allocating formatters, public Format*/Append* APIs, comprehensive unit tests and benchmarks validating parity with strconv.
Conversion / ToString updates
convert.go
Replaces strconv conversions and pooled appends with the new Format*/Append* helpers; adds nil-safe branches for common pointer types, explicit slice handling to reduce reflection, and optimized string-building paths.
Byte-size parsing
common.go
Reworked digit/unit detection into a single backward scan, removed strconv import, switched parsing calls to ParseFloat64/ParseUint, added early return on no-digits, preserved unit suffix multiplier handling.
Parsing behavior
parse.go
Changed empty-input handling so parse errors carry empty Num fields; simplified digit parsing entry points (use parseDigits(s, 0)).
HTTP/MIME helpers
http.go
Normalized extension handling in GetMIME with a single map lookup and fallback to mime.TypeByExtension; minor slice index adjustment in ParseVendorSpecificContentType.
String tests & benchmarks
strings_test.go
Removed some t.Parallel() usages, restructured tests to serial subtests and benchmarks into paired "fiber" vs "default" sub-benchmarks with per-path assertions.
Conversion tests
convert_test.go
Added nil-pointer handling tests verifying ToString returns type-appropriate zero-values for common nil pointer types.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

  • Focus areas:
    • format.go / format_test.go: correctness across full numeric ranges, cache/init correctness, and benchmark validity.
    • convert.go: nil-pointer branches, explicit type paths, and reflection fallbacks.
    • common.go: backward-scan digit/unit detection edge cases and interaction with ParseFloat64/ParseUint.
    • parse.go: callers that previously relied on non-empty Num for empty-input errors.

Possibly related PRs

Suggested labels

✏️ Feature

Suggested reviewers

  • gaby
  • sixcolors
  • efectn

Poem

🐰
I hopped through bytes and numbers bright,
Cached tiny twos and zeros tight,
Replaced the old strconv way,
Now formatting hops light as day,
🥕✨

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 44.90% 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 PR title broadly references optimization and error handling improvements but does not convey the main focus or scale of changes (new formatting functions, reflection reduction, pointer/slice handling, parsing fixes, MIME improvements).
✨ 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 optimize_fns

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.

Copy link
Copy Markdown
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: 2

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 71424ca and 266de52.

⛔ Files ignored due to path filters (1)
  • .golangci.yml is excluded by !**/*.yml
📒 Files selected for processing (6)
  • common.go (1 hunks)
  • convert.go (5 hunks)
  • http.go (2 hunks)
  • parse.go (3 hunks)
  • strings.go (1 hunks)
  • strings_test.go (0 hunks)
💤 Files with no reviewable changes (1)
  • strings_test.go
🧰 Additional context used
🧬 Code graph analysis (1)
common.go (1)
parse.go (2)
  • ParseFloat64 (252-254)
  • ParseUint (20-22)
🪛 GitHub Actions: Modernize Lint
convert.go

[error] 254-254: for loop can be modernized using range over int

🪛 GitHub Check: modernize
convert.go

[failure] 254-254:
for loop can be modernized using range over int

⏰ 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). (2)
  • GitHub Check: Agent
  • GitHub Check: Compare
🔇 Additional comments (12)
parse.go (2)

82-82: Consistent empty string optimization for empty input errors.

When the input s is empty, string(s) would produce "" anyway, so setting Num: "" directly avoids an unnecessary conversion. This is a correct micro-optimization applied consistently across all three parsing functions.

Also applies to: 125-125, 145-145


128-129: Simplified parseDigits call.

Removing the separate index variable and calling parseDigits(s, 0) directly is a cleaner approach that reduces local state.

common.go (2)

138-157: Good refactor: combined backward scan for digit and unit detection.

The single backward pass to find both the last digit position and unit prefix position is efficient. The early return for missing digits (line 155-157) is a good defensive check.


162-183: Replaced stdlib calls with internal parsing functions.

Using ParseFloat64 and ParseUint instead of strconv.ParseFloat and strconv.ParseUint aligns with the PR's goal of reducing external dependencies. The manual dot detection loop is simple and avoids the strings import.

http.go (2)

19-41: Clean normalization of extension handling.

Computing both extWithoutDot and extWithDot once upfront eliminates repeated checks and makes the control flow clearer. The fallback chain (map lookup → mime package → default) is well-structured.


77-82: Reduced allocation with pre-sized buffer.

Using a pre-allocated byte slice and UnsafeString avoids the allocation from string concatenation. This is a valid optimization for hot paths.

strings.go (2)

8-45: Effective single-allocation optimization with early-out for no-op cases.

The approach of detecting the first character needing conversion and returning early if none is found eliminates allocations for already-lowercase strings. The loop unrolling at lines 34-39 can improve throughput for longer strings.


48-85: ToUpper mirrors ToLower structure consistently.

Same optimization pattern applied symmetrically. The implementation is correct and consistent.

convert.go (4)

18-24: Good use of sync.Pool for buffer reuse.

The pool correctly stores a pointer to a slice (*[]byte) to avoid interface allocation overhead. The initial capacity of 16 bytes is reasonable for the typical output sizes.


96-131: Correct pool usage with copy before return.

The string(buf) calls at lines 107 and 128 correctly create a copy of the buffer contents before returning the buffer to the pool. This ensures the returned string doesn't alias the pooled memory.


148-156: Direct FormatUint for unsigned types.

Using strconv.FormatUint directly for unsigned types is more appropriate than going through Itoa which is designed for signed integers.


208-237: Direct slice handling reduces reflection overhead.

Handling []string and []int explicitly avoids the reflection path for these common types. The space-separated format matches the reflection-based behavior.

Copy link
Copy Markdown
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: 0

♻️ Duplicate comments (2)
convert.go (2)

147-156: Unsigned integer conversion now uses FormatUint – correctness fix confirmed.

Switching all unsigned cases (uint, uint8, uint16, uint32, uint64) to strconv.FormatUint (Lines 147–156) avoids the overflow/incorrect string issues that occur when routing through signed types. This matches the intended domain of these values and fixes the prior bug without changing the public API surface.


176-207: Pointer fast paths are efficient, but nil semantics change for basic pointer types.

  • The specialized pointer cases for *string, *int, *int64, *uint64, *float64, and *bool (Lines 176–207) are efficient and avoid reflection for common cases.
  • However, they intentionally change nil behavior: e.g. ToString((*int)(nil)) now returns "0" instead of the previous "<nil>" fallback via fmt.Sprint, and similarly (*string)(nil)"", (*bool)(nil)"false".

If this semantic change is intended, it should be explicitly documented and covered by tests; otherwise, consider returning "<nil>" (or delegating to fmt.Sprint) for the nil branches to preserve prior behavior.

🧹 Nitpick comments (2)
convert.go (2)

13-24: ByteSize’s sync.Pool usage and buffer lifecycle look correct.

  • The byteSizePool setup and Get/Put pattern are sound: bufPtr is always initialized (*bufPtr = make(...)) in the fallback path before slicing (Line 100), so there’s no risk of slicing a nil slice.
  • Converting with result := string(buf) before returning the buffer to the pool (Lines 107 and 128) correctly avoids aliasing; using UnsafeString here would be unsafe if the buffer is reused.
  • You’re getting reduced allocations on the []byte backing store while still paying the unavoidable string copy, which is a reasonable trade-off for this API.

If you want stricter safety, you could replace the if !ok branch with a panic, since any wrong type coming from this pool would be a programming error, but the current defensive fallback is still correct.

Also applies to: 96-103, 107-111, 128-131


208-237: Direct []string and []int formatting is correct and allocation‑aware.

  • The specialized handling for []string and []int (Lines 208–237) matches fmt.Sprint’s bracketed, space‑separated format ([a b], [] for empty) while avoiding reflection in the hot path.
  • Using strings.Builder with a rough Grow heuristic is fine here; it trades a modest over-approximation for fewer reallocations, and behaves reasonably for nil vs empty slices (len(v) == 0 hits the early "[]" return).

Looks good as-is; no functional issues spotted.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 266de52 and 5b6a783.

📒 Files selected for processing (1)
  • convert.go (5 hunks)
⏰ 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). (1)
  • GitHub Check: Compare
🔇 Additional comments (1)
convert.go (1)

241-252: The review comment contains an incorrect primary concern regarding reflect.ValueOf(nil) behavior. Calling rv.Kind() on the zero reflect.Value returned by reflect.ValueOf(nil) does not panic—it safely returns reflect.Invalid. The current code at line 241 is safe.

For nil arguments, the function already handles them correctly:

  1. reflect.ValueOf(nil) returns a zero reflect.Value
  2. rv.Kind() returns reflect.Invalid without panicking
  3. Neither the pointer check (line 242) nor the slice/array check (line 245) matches
  4. Execution falls through to fmt.Sprint(arg) at line 265, which returns "<nil>"—the same behavior as before the refactor

The other observations are accurate: the for i := range n syntax correctly uses Go 1.22+ semantics, and the recursive formatting of slices/arrays could theoretically loop on self-referential structures (though this is a low-probability edge case). Adding an early nil guard is not necessary to prevent a panic and would be purely stylistic.

Copy link
Copy Markdown
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: 0

♻️ Duplicate comments (2)
convert.go (1)

155-185: Nil pointer behavior still needs test coverage.

This comment echoes a previous review concern: the explicit nil handling for common pointer types (*string, *int, *int64, etc.) returns type-specific defaults ("", "0", "false") rather than "<nil>", which differs from how bare nil or unhandled pointer types are treated (they fall through to fmt.Sprint and return "<nil>").

The code comment indicates this is an intentional optimization to avoid reflection. However, test coverage documenting this behavior is still missing. Consider adding test cases such as:

  • require.Equal(t, "0", ToString((*int)(nil)))
  • require.Equal(t, "", ToString((*string)(nil)))

Based on past reviews, this behavioral change was previously flagged and remains unaddressed.

strings.go (1)

9-15: Consider keeping a zero-allocation fast path for unchanged ASCII input

This version always allocates and copies len(b) bytes before mapping through toLowerTable, whereas the previous implementation (per summary / earlier feedback) avoided allocation when no bytes changed and only copied once a differing byte was found. For common already-lowercase ASCII inputs this can be a GC and CPU regression. A hybrid approach could scan for the first byte where toLowerTable[c] != c, and only then allocate/copy and apply the table from that index onward, returning the original b if no such byte is found. That preserves the optimization while still using the lookup table on the hot path.

🧹 Nitpick comments (1)
strings.go (1)

20-26: Mirror the ToLower strategy and consider de-duplicating the case-mapping logic

The same “always allocate/copy and map” pattern applies here, with the same trade-offs as in ToLower. Whatever strategy you choose there (e.g., reintroducing a no-change fast path) should be mirrored here for consistency. Given the bodies are identical except for toUpperTable vs toLowerTable, you might also extract a small helper like func mapASCII(b string, tbl *[256]byte) string to avoid copy-paste and keep behavior in sync.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between c366f6d and 6564354.

📒 Files selected for processing (4)
  • common.go (1 hunks)
  • convert.go (4 hunks)
  • http.go (2 hunks)
  • strings.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • common.go
🧰 Additional context used
🧬 Code graph analysis (2)
strings.go (1)
convert.go (1)
  • UnsafeString (18-23)
convert.go (1)
format.go (10)
  • AppendUint (88-95)
  • FormatInt (46-55)
  • FormatInt8 (83-85)
  • FormatInt16 (73-75)
  • FormatInt32 (63-65)
  • FormatUint (35-42)
  • FormatUint8 (78-80)
  • FormatUint16 (68-70)
  • FormatUint32 (58-60)
  • FormatBool (110-115)
⏰ 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). (3)
  • GitHub Check: Build (1.23.x, windows-latest)
  • GitHub Check: Build (1.24.x, macos-13)
  • GitHub Check: Compare
🔇 Additional comments (6)
convert.go (4)

87-111: LGTM! ByteSize optimization using new AppendUint helper.

The switch from strconv.AppendUint to the new AppendUint helper is consistent with the PR's optimization goals. The logic correctly handles both the integer-only case (when div == 1) and the fractional case, and the UnsafeString usage is safe since buf is a local variable.


117-141: LGTM! Critical bug fix and performance optimization for numeric types.

The switch to dedicated Format* helpers (FormatInt, FormatUint, FormatInt8, etc.) correctly addresses the critical overflow bug in the previous implementation where unsigned integers were incorrectly cast to signed types. The new implementation properly uses FormatUint for unsigned types and FormatInt for signed types.


187-216: LGTM! Efficient optimization for common slice types.

The direct handling of []string and []int avoids reflection overhead and uses strings.Builder with sensible pre-allocation. The space-separated format (e.g., "[hello world]") is consistent with the reflection-based slice handling below.


220-240: LGTM! Reflection path optimized with modern Go syntax.

The improvements correctly:

  • Cache the Kind() result to avoid redundant calls
  • Use strings.Builder with pre-allocation for slice/array formatting
  • Employ the modern for i := range n syntax (Go 1.22+), which addresses the previously flagged lint issue

The logic correctly handles pointers (dereferencing non-nil), slices/arrays (formatted with space-separated elements), and falls back to fmt.Sprint for other types.

http.go (2)

20-42: GetMIME normalization and fallback chain look correct

The revised flow is tight and preserves expected behavior:

  • Empty-string guard avoids any indexing panics.
  • Normalizing into extWithoutDot / extWithDot once removes redundant checks while handling both "ext" and ".ext" forms consistently.
  • Prioritizing mimeExtensions[extWithoutDot] before mime.TypeByExtension(extWithDot) keeps your custom MIME overrides (like js, xml, etc.) in control and only falls back to the stdlib when not covered by your table.
  • The final MIMEOctetStream fallback gives a consistent default for unknown extensions rather than leaking an empty string for non-empty input.

This matches the intended “custom table → stdlib → application/octet-stream” resolution described in the library context.


77-77: Idiomatic slice expression in ParseVendorSpecificContentType

Switching from working[0:slashIndex+1] to working[:slashIndex+1] is purely stylistic and idiomatic Go with no behavioral change; the function still correctly reconstructs type/subtype for vendor-specific content types.

Copy link
Copy Markdown
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: 0

♻️ Duplicate comments (1)
convert.go (1)

155-185: Nil pointer behavior change should be documented and tested.

The nil pointer handling returns type-specific zero values ("", "0", "false") rather than "<nil>" which would be produced by the fmt.Sprint fallback. This is a behavioral change that should be explicitly covered by tests.

Consider adding test cases to document this intentional behavior:

// Example test cases for nil pointer handling
require.Equal(t, "", ToString((*string)(nil)))
require.Equal(t, "0", ToString((*int)(nil)))
require.Equal(t, "0", ToString((*int64)(nil)))
require.Equal(t, "0", ToString((*uint64)(nil)))
require.Equal(t, "0", ToString((*float64)(nil)))
require.Equal(t, "false", ToString((*bool)(nil)))

Based on past review comment noting this behavioral change needs documentation.

🧹 Nitpick comments (1)
convert.go (1)

186-216: Consider documenting the space-separated slice format.

The slice formatting uses space-separated elements ([a b c]) rather than the comma-separated format typical in Go ([a, b, c]). This is consistent between the direct type cases and the reflection fallback, but the format choice may surprise users expecting standard Go representation.

If this is intentional, consider adding a code comment or documentation noting the format.

📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 6564354 and 413b58f.

📒 Files selected for processing (3)
  • convert.go (4 hunks)
  • format.go (1 hunks)
  • format_test.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • format_test.go
🧰 Additional context used
🧬 Code graph analysis (1)
convert.go (1)
format.go (9)
  • AppendUint (88-95)
  • FormatInt (46-55)
  • FormatInt8 (83-85)
  • FormatInt16 (73-75)
  • FormatInt32 (63-65)
  • FormatUint (35-42)
  • FormatUint8 (78-80)
  • FormatUint16 (68-70)
  • FormatUint32 (58-60)
⏰ 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). (1)
  • GitHub Check: Compare
🔇 Additional comments (8)
convert.go (3)

88-111: LGTM - ByteSize optimization is correct.

The switch from strconv.AppendUint to the custom AppendUint helper is consistent with the performance-oriented approach introduced in format.go. The logic remains sound, and UnsafeString is appropriate here since the buffer is not reused after conversion.


116-135: LGTM - Integer formatting conversions are correct.

All integer types are properly converted using the new formatting helpers. The widening conversions (e.g., int to int64, uint to uint64) are safe and correct for all value ranges.


217-244: LGTM - Reflection fallback is well-structured.

The early kind extraction and the use of range n (Go 1.22+ syntax) are correct. The slice/array handling via reflection is consistent with the direct type cases. Nil pointers correctly fall through to fmt.Sprint which produces "<nil>".

format.go (5)

1-17: LGTM - Small integer cache is well-designed.

The precomputed smallInts cache for values 0-99 is a good optimization that avoids allocations for the most common integer values. The init() function correctly populates the cache at startup.


19-31: LGTM - Buffer formatting is correct and efficient.

The formatUintBuf function correctly handles all uint64 values. The 20-byte buffer is appropriately sized for the maximum uint64 value (20 digits), and the algorithm correctly extracts digits from right to left.


44-55: LGTM - FormatInt correctly handles all int64 values including MinInt64.

The implementation correctly handles the edge case of math.MinInt64 (-9223372036854775808) due to two's complement arithmetic. When -n overflows in int64, the subsequent uint64() conversion produces the correct unsigned magnitude (9223372036854775808).


57-85: LGTM - Type-specific wrappers are correct.

All wrappers correctly delegate to FormatUint or FormatInt with safe widening conversions. This provides a clean API while maintaining the performance benefits of the underlying implementation.


87-107: LGTM - Append functions are correct and consistent.

The AppendUint and AppendInt functions mirror the logic of their Format* counterparts while efficiently appending to a destination slice. The small integer cache optimization is correctly applied here as well.

…ng custom implementations with standard library functions
@gaby gaby changed the title optimize string manipulation functions and improve error handling chore: optimize string manipulation functions and improve error handling Dec 5, 2025
@gaby
Copy link
Copy Markdown
Member

gaby commented Dec 5, 2025

@ReneWerner87

Comment from CodeRabbit:

155-185: Nil pointer behavior change should be documented and tested.

The nil pointer handling returns type-specific zero values ("", "0", "false") rather than "<nil>" which would be produced by the fmt.Sprint fallback. This is a behavioral change that should be explicitly covered by tests.

Copy link
Copy Markdown
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: 0

🧹 Nitpick comments (2)
convert_test.go (1)

167-192: Good coverage for the documented nil-pointer behavior change.

These tests correctly verify the new type-specific default values for nil pointers, which is a behavioral change from the previous <nil> fallback. The tests are well-structured and run in parallel.

Consider adding similar tests for other pointer types that might be handled by ToString (e.g., *float32, *uint, *int32) to ensure comprehensive coverage of nil-pointer edge cases.

format.go (1)

194-204: Consider caching small negative integers in AppendInt for consistency.

AppendInt doesn't use smallNegInts for values in the range -99 to -1, unlike FormatInt. This means small negative integers incur a buffer allocation and formatting overhead.

 // AppendInt appends the decimal string representation of n to dst.
 func AppendInt(dst []byte, n int64) []byte {
 	if n >= 0 {
 		return AppendUint(dst, uint64(n))
 	}
+	if n > -100 {
+		return append(dst, smallNegInts[-n]...)
+	}
 	var buf [20]byte
 	i := formatUintBuf(&buf, uint64(-n))
 	i--
 	buf[i] = '-'
 	return append(dst, buf[i:]...)
 }
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 7f10c3e and 483fccc.

📒 Files selected for processing (2)
  • convert_test.go (1 hunks)
  • format.go (1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
convert_test.go (1)
convert.go (1)
  • ToString (114-246)
⏰ 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). (2)
  • GitHub Check: Build (1.23.x, windows-latest)
  • GitHub Check: Compare
🔇 Additional comments (7)
format.go (7)

9-16: LGTM: Cache initialization is correct.

The cache initialization correctly populates both positive and negative small integer caches, and appropriately skips index 0 for negative numbers.


18-23: LGTM: Small integer formatting helper is correct.

The helper correctly handles single and double-digit numbers using efficient byte arithmetic.


25-37: LGTM: Buffer formatting is efficient and correct.

The 20-byte buffer is sufficient for the maximum uint64 value (20 digits), and the digit extraction loop is implemented correctly.


50-69: FormatInt correctly handles math.MinInt64.

The conversion uint64(-n) when n = math.MinInt64 works correctly in Go due to two's complement representation—the overflow wraps to produce the correct unsigned magnitude (9223372036854775808).


71-112: LGTM: 32-bit formatting functions are correct.

Buffer sizes are appropriate (10 for uint32, 11 for int32 with sign), and the edge case of math.MinInt32 is handled correctly through unsigned conversion.


114-155: LGTM: 16-bit formatting functions are correct.

Buffer sizes and logic are appropriate for the value ranges.


157-182: LGTM: 8-bit formatting handles edge cases correctly.

The special case of math.MinInt8 (-128) is handled correctly—the unsigned conversion via uint8(-n) produces 128 due to Go's two's complement bit reinterpretation.

@ReneWerner87
Copy link
Copy Markdown
Member Author

@gaby fixed

@ReneWerner87 ReneWerner87 merged commit f24eca3 into master Dec 5, 2025
21 checks passed
@ReneWerner87 ReneWerner87 deleted the optimize_fns branch December 5, 2025 20:48
@gaby
Copy link
Copy Markdown
Member

gaby commented Dec 5, 2025

@ReneWerner87 One more small thing:


format.go (1)
194-204: Consider caching small negative integers in AppendInt for consistency.

AppendInt doesn't use smallNegInts for values in the range -99 to -1, unlike FormatInt. This means small negative integers incur a buffer allocation and formatting overhead.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants