code review 4527089: big: ~8x faster number scanning

big: ~8x faster number scanning

- better number scanning algorithm
- fixed a couple of bugs related to base interpretation
- added scan benchmark
- added more test cases and made tests more precise
- introduced Int.scan method matching nat.scan
- refactored Int.Scan; now uses int.scan
- refactored Int.SetString; now uses int.scan

There is more potential, this was a fairly simple change.

gotest -test.bench="ScanPi" before/after (best of 3 runs):
big.BenchmarkScanPi	   1000	    2024900 ns/op
big.BenchmarkScanPi       10000      257540 ns/op

[gri, 2011-06-01 00:52:22]

Hello chickencha@gmail.com (cc: golang-dev@googlegroups.com, rsc@golang.org),

I'd like you to review this change to
https://go.googlecode.com/hg/

[eds, 2011-06-01 01:33:40]

Nice, LGTM. A couple suggestions.

http://codereview.appspot.com/4527089/diff/8001/src/pkg/big/int.go
File src/pkg/big/int.go (right):

http://codereview.appspot.com/4527089/diff/8001/src/pkg/big/int.go#newcode465
src/pkg/big/int.go:465: func (z *Int) SetString(s string, base int) (*Int, bool)
{
You could refactor this method to call Int.scan.

http://codereview.appspot.com/4527089/diff/8001/src/pkg/big/nat.go
File src/pkg/big/nat.go (right):

http://codereview.appspot.com/4527089/diff/8001/src/pkg/big/nat.go#newcode695
src/pkg/big/nat.go:695: bb = Word(base)
base and d are used as Words more often than ints in this method. Maybe it would
make sense to just make them both Words instead of converting all the time?

[gri, 2011-06-01 18:19:05]

PTAL.

Also fixed a couple of bugs related to the returned base in special cases,
simplified the code as suggested, added better tests, and made comments more
precise.

http://codereview.appspot.com/4527089/diff/8001/src/pkg/big/int.go
File src/pkg/big/int.go (right):

http://codereview.appspot.com/4527089/diff/8001/src/pkg/big/int.go#newcode465
src/pkg/big/int.go:465: func (z *Int) SetString(s string, base int) (*Int, bool)
{
On 2011/06/01 01:33:40, eds wrote:
> You could refactor this method to call Int.scan.

Done.

http://codereview.appspot.com/4527089/diff/8001/src/pkg/big/nat.go
File src/pkg/big/nat.go (right):

http://codereview.appspot.com/4527089/diff/8001/src/pkg/big/nat.go#newcode695
src/pkg/big/nat.go:695: bb = Word(base)
On 2011/06/01 01:33:40, eds wrote:
> base and d are used as Words more often than ints in this method. Maybe it
would
> make sense to just make them both Words instead of converting all the time?

Done.

[eds, 2011-06-01 21:02:23]

http://codereview.appspot.com/4527089/diff/12002/src/pkg/big/int.go
File src/pkg/big/int.go (right):

http://codereview.appspot.com/4527089/diff/12002/src/pkg/big/int.go#newcode457
src/pkg/big/int.go:457: // SetString sets z to the value of s, interpreted in
the given base,
Mention somewhere here that base must be <= MaxBase.

[gri, 2011-06-01 21:05:18]

PTAL.

http://codereview.appspot.com/4527089/diff/12002/src/pkg/big/int.go
File src/pkg/big/int.go (right):

http://codereview.appspot.com/4527089/diff/12002/src/pkg/big/int.go#newcode457
src/pkg/big/int.go:457: // SetString sets z to the value of s, interpreted in
the given base,
On 2011/06/01 21:02:23, eds wrote:
> Mention somewhere here that base must be <= MaxBase.

Done.

[eds, 2011-06-01 21:07:45]

LGTM

[gri, 2011-06-01 21:17:06]

*** Submitted as http://code.google.com/p/go/source/detail?r=73d526d94d1d ***

big: ~8x faster number scanning

- better number scanning algorithm
- fixed a couple of bugs related to base interpretation
- added scan benchmark
- added more test cases and made tests more precise
- introduced Int.scan method matching nat.scan
- refactored Int.Scan; now uses int.scan
- refactored Int.SetString; now uses int.scan

There is more potential, this was a fairly simple change.

gotest -test.bench="ScanPi" before/after (best of 3 runs):
big.BenchmarkScanPi	   1000	    2024900 ns/op
big.BenchmarkScanPi       10000      257540 ns/op

R=chickencha
CC=golang-dev, rsc
http://codereview.appspot.com/4527089