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

implement Log10 and CmpUint64 (alternate version) #136

Merged
merged 8 commits into from
Mar 31, 2023
Merged

implement Log10 and CmpUint64 (alternate version) #136

merged 8 commits into from
Mar 31, 2023

Conversation

holiman
Copy link
Owner

@holiman holiman commented Mar 30, 2023

Alternative to #134

This PR implements Log10 in what is probably close to the optimal way. Both PRs handle the majority of cases by one lookup based on the bitlength.

For the remaining cases, #134 does 0, or, 1, or 2 divisions, and then a few uint64-compares and then it's done.

In this PR, for the remaining cases, we do an additional lookup, and then a comparison, and then we're done.

The drawback of this PR is that it has more precalculated 'persistent memory' hog.


pows = [60]Int         // 1920B 
pows64 = [20]uint64 // 20*8 = 160B

So around 2 K. Not much, but still

@holiman holiman changed the title implement Log10 and CmpUint64 (alternate version) implement Log10 and CmpUint64 (alternate version) Mar 30, 2023
@holiman
Copy link
Owner Author

holiman commented Mar 30, 2023

From CI benchmark run

#134 :

BenchmarkLog10/Log10/uint256-36                     	  168109	      6829 ns/op	       0 B/op	       0 allocs/op
BenchmarkLog10/Log10/big-36                         	   36321	     34610 ns/op	   23424 B/op	     510 allocs/op

This PR

BenchmarkLog10/Log10/uint256-36                     	  705776	      1597 ns/op	       0 B/op	       0 allocs/op
BenchmarkLog10/Log10/big-36                         	   27064	     44892 ns/op	   23424 B/op	     510 allocs/op

So this is on the order of 3-4x faster.

@codecov
Copy link

codecov bot commented Mar 31, 2023

Codecov Report

Merging #136 (fc79e38) into master (1c9ad56) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #136   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files            5         5           
  Lines         1628      1646   +18     
=========================================
+ Hits          1628      1646   +18     

@holiman holiman marked this pull request as ready for review March 31, 2023 10:19
uint256.go Outdated Show resolved Hide resolved
@holiman holiman merged commit e42b95a into master Mar 31, 2023
@fyfyrchik fyfyrchik mentioned this pull request Aug 23, 2023
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.

1 participant