Skip to content
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

removed min() function for RescalePair() #265

Merged
merged 3 commits into from Jan 15, 2024

Conversation

cbelsole
Copy link
Contributor

Removing the min() func from RescalePair() stops the copy operation for passing the values into min() saving on the bytes per op and marginal decreases in runtime. For fun I implemented the new implementation into a new Add() func and it looks like the effect is doubled.

goos: windows
goarch: amd64
pkg: github.com/shopspring/decimal
cpu: Intel(R) Core(TM) i5-3550 CPU @ 3.30GHz

BenchmarkRescalePairGreaterThan-4      	 3631178	       333.6 ns/op	     200 B/op	       7 allocs/op
BenchmarkRescalePairLessThan-4         	 3669758	       336.1 ns/op	     200 B/op	       7 allocs/op
BenchmarkRescalePairEqual-4            	261132116	         4.714 ns/op	       0 B/op	       0 allocs/op

BenchmarkNewRescalePairGreaterThan-4   	 3847358	       329.0 ns/op	     160 B/op	       7 allocs/op
BenchmarkNewRescalePairLessThan-4      	 3669552	       318.7 ns/op	     160 B/op	       7 allocs/op
BenchmarkNewRescalePairEqual-4         	255000668	         4.692 ns/op	       0 B/op	       0 allocs/op

BenchmarkAddGreaterThan-4              	 2455047	       428.4 ns/op	     280 B/op	       9 allocs/op
BenchmarkAddLessThan-4                 	 2711714	       430.2 ns/op	     280 B/op	       9 allocs/op
BenchmarkAddEqual-4                    	11634004	       100.4 ns/op	      80 B/op	       2 allocs/op

BenchmarkNewAddGreaterThan-4           	 2999233	       400.4 ns/op	     200 B/op	       9 allocs/op
BenchmarkNewAddLessThan-4              	 2975070	       402.1 ns/op	     200 B/op	       9 allocs/op
BenchmarkNewAddEqual-4                 	13229145	        97.82 ns/op	      80 B/op	       2 allocs/op

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 nice optimization, I have not thought about that during the implementation.
Sorry for not working CI, Travis.org is unresponsive. I tried to contact them a few times about open-source credits. Probably I would have to move to another open-source friendly CI service.

decimal.go Outdated Show resolved Hide resolved
@cbelsole
Copy link
Contributor Author

@mwoss I fixed the tests. Thanks for catching that.

As far as CI environments, for public projects CircleCI is pretty easy to setup and has a free tier.

@mwoss mwoss merged commit b79c571 into shopspring:master Jan 15, 2024
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.

None yet

2 participants