Skip to content

UnmarshalJSON, Scan and NewFromString performance improvements #403

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Jun 24, 2025

Conversation

PauliusLozys
Copy link
Contributor

@PauliusLozys PauliusLozys commented Jun 9, 2025

This PR improves UnmarshalJSON performance by reducing unnecessary allocation caused by unquoteIfQuoted function. This also touches on Scan method to split default case into string and []byte cases.

This PR also slightly touches NewFromString function by making so scientific notation and dots are checked in a single loop. Also, adds an additional check (and tests) for multiple scientific notations in the string.

Benchmarks:
Old:

goos: linux
goarch: amd64
pkg: github.com/shopspring/decimal
cpu: AMD Ryzen 7 7700X 8-Core Processor             
BenchmarkDecimal_UnmarshalJSON-16    	10973582	       105.5 ns/op	      96 B/op	       5 allocs/op
BenchmarkDecimal_UnmarshalJSON-16    	11399134	       103.7 ns/op	      96 B/op	       5 allocs/op
BenchmarkDecimal_UnmarshalJSON-16    	11480460	       103.4 ns/op	      96 B/op	       5 allocs/op
BenchmarkDecimal_UnmarshalJSON-16    	11440952	       103.6 ns/op	      96 B/op	       5 allocs/op
BenchmarkDecimal_UnmarshalJSON-16    	11305298	       103.2 ns/op	      96 B/op	       5 allocs/op
BenchmarkDecimal_UnmarshalJSON-16    	11353047	       102.5 ns/op	      96 B/op	       5 allocs/op
BenchmarkDecimal_UnmarshalJSON-16    	11419255	       103.4 ns/op	      96 B/op	       5 allocs/op
BenchmarkDecimal_UnmarshalJSON-16    	11392092	       103.3 ns/op	      96 B/op	       5 allocs/op
BenchmarkDecimal_UnmarshalJSON-16    	11341843	       104.6 ns/op	      96 B/op	       5 allocs/op
BenchmarkDecimal_UnmarshalJSON-16    	11605540	       103.0 ns/op	      96 B/op	       5 allocs/op
PASS
ok  	github.com/shopspring/decimal	12.861s

New:

goos: linux
goarch: amd64
pkg: github.com/shopspring/decimal
cpu: AMD Ryzen 7 7700X 8-Core Processor             
BenchmarkDecimal_UnmarshalJSON-16    	13650522	        83.84 ns/op	      72 B/op	       4 allocs/op
BenchmarkDecimal_UnmarshalJSON-16    	14697308	        80.16 ns/op	      72 B/op	       4 allocs/op
BenchmarkDecimal_UnmarshalJSON-16    	14545791	        80.81 ns/op	      72 B/op	       4 allocs/op
BenchmarkDecimal_UnmarshalJSON-16    	14889534	        79.66 ns/op	      72 B/op	       4 allocs/op
BenchmarkDecimal_UnmarshalJSON-16    	15066388	        79.93 ns/op	      72 B/op	       4 allocs/op
BenchmarkDecimal_UnmarshalJSON-16    	14768164	        80.08 ns/op	      72 B/op	       4 allocs/op
BenchmarkDecimal_UnmarshalJSON-16    	14964182	        80.74 ns/op	      72 B/op	       4 allocs/op
BenchmarkDecimal_UnmarshalJSON-16    	15048375	        80.77 ns/op	      72 B/op	       4 allocs/op
BenchmarkDecimal_UnmarshalJSON-16    	14638641	        80.14 ns/op	      72 B/op	       4 allocs/op
BenchmarkDecimal_UnmarshalJSON-16    	14878336	        80.23 ns/op	      72 B/op	       4 allocs/op
PASS
ok  	github.com/shopspring/decimal	12.701s

Benchstat comparison:

goos: linux
goarch: amd64
pkg: github.com/shopspring/decimal
cpu: AMD Ryzen 7 7700X 8-Core Processor             
                         │   old.txt    │               new.txt               │
                         │    sec/op    │   sec/op     vs base                │
Decimal_UnmarshalJSON-16   103.40n ± 1%   80.20n ± 1%  -22.44% (p=0.000 n=10)

                         │  old.txt   │              new.txt               │
                         │    B/op    │    B/op     vs base                │
Decimal_UnmarshalJSON-16   96.00 ± 0%   72.00 ± 0%  -25.00% (p=0.000 n=10)

                         │  old.txt   │              new.txt               │
                         │ allocs/op  │ allocs/op   vs base                │
Decimal_UnmarshalJSON-16   5.000 ± 0%   4.000 ± 0%  -20.00% (p=0.000 n=10)

This PR improves `UnmarshalJSON`  performance by reducing unnecessary allocation caused by `unquoteIfQuoted` function. This also touches on `Scan` method to split `default` case into `string` and `[]byte` cases.

This PR also slightly touches `NewFromString` function by making so scientific notation and dots are checked in a single loop.
Copy link
Member

@mwoss mwoss left a comment

Choose a reason for hiding this comment

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

Overall looks good! Thanks for the contribution!

decimal.go Outdated
Comment on lines 189 to 192
if eIndex == -1 && (r == 'E' || r == 'e') {
eIndex = i
continue
}
Copy link
Member

Choose a reason for hiding this comment

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

Just for the sake of consistency, can we do a similar check as below? I mean, check for e or E and if value already set, return an error. What do you think?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hi, yes, no problem 👍. Added the check and also a small test for it 🛠️.

@mwoss mwoss merged commit 08afb35 into shopspring:master Jun 24, 2025
6 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