code review 6303087: strconv: extend Grisu3 algorithm to float32.

strconv: extend Grisu3 algorithm to float32.

Also improve extfloat.Normalize to obtain a modest performance
gain in parsing, and add a shortcut path for exact integers.

benchmark                              old ns/op    new ns/op    delta
BenchmarkAtof64Decimal                        73           73   -0.54%
BenchmarkAtof64Float                          91           91   -0.54%
BenchmarkAtof64FloatExp                      198          180   -9.09%
BenchmarkAtof64Big                           307          308   +0.33%

BenchmarkAtof32Decimal                        72           72   +0.42%
BenchmarkAtof32Float                          83           83   -0.72%
BenchmarkAtof32FloatExp                      212          186  -12.26%
BenchmarkAtof32Random                        262          250   -4.58%

BenchmarkAppendFloatDecimal                  474          305  -35.65%
BenchmarkAppendFloat                         497          489   -1.61%
BenchmarkAppendFloatExp                      493          483   -2.03%
BenchmarkAppendFloatNegExp                   481          481   +0.00%
BenchmarkAppendFloatBig                      667          652   -2.25%

BenchmarkAppendFloat32Integer                338          307   -9.17%
BenchmarkAppendFloat32ExactFraction          364          439  +20.60%
BenchmarkAppendFloat32Point                 1299          490  -62.28%
BenchmarkAppendFloat32Exp                   2593          489  -81.14%
BenchmarkAppendFloat32NegExp                5116          481  -90.60%

[remyoudompheng, 2012-06-15 21:29:41]

Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com,
remy@archlinux.org),

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

[rsc, 2012-06-19 04:05:15]

Why did BenchmarkAppendFloat32Decimal get slower? Is the grisu slower than the
old case for the case of an odd integer?

http://codereview.appspot.com/6303087/diff/4001/src/pkg/strconv/extfloat.go
File src/pkg/strconv/extfloat.go (right):

http://codereview.appspot.com/6303087/diff/4001/src/pkg/strconv/extfloat.go#n...
src/pkg/strconv/extfloat.go:218: if mant>>32 == 0 {
Could you write these >> constants as (64-32), (64-16) and so on, to match the
constants used in the if bodies?

[rsc, 2012-06-19 04:05:20]

LGTM

[remyoudompheng, 2012-06-19 06:46:36]

Hello rsc@golang.org (cc: golang-dev@googlegroups.com, remy@archlinux.org),

Please take another look.

[rsc, 2012-06-19 15:11:26]

LGTM

http://codereview.appspot.com/6303087/diff/4002/src/pkg/strconv/extfloat.go
File src/pkg/strconv/extfloat.go (right):

http://codereview.appspot.com/6303087/diff/4002/src/pkg/strconv/extfloat.go#n...
src/pkg/strconv/extfloat.go:201: if f.exp < 0 && mant ==
(mant>>uint(-f.exp))<<uint(-f.exp) {
Should this be <= 0 ?

[remyoudompheng, 2012-06-28 10:14:21]

Hello rsc@golang.org (cc: golang-dev@googlegroups.com, remy@archlinux.org),

Please take another look.

[remyoudompheng, 2012-06-28 10:18:09]

The refactoring introduced a bug I discovered while doing more extensive testing
(printing of float32(1<<92)). I have added a test.

http://codereview.appspot.com/6303087/diff/4002/src/pkg/strconv/extfloat.go
File src/pkg/strconv/extfloat.go (right):

http://codereview.appspot.com/6303087/diff/4002/src/pkg/strconv/extfloat.go#n...
src/pkg/strconv/extfloat.go:201: if f.exp < 0 && mant ==
(mant>>uint(-f.exp))<<uint(-f.exp) {
On 2012/06/19 15:11:26, rsc wrote:
> Should this be <= 0 ?
> 

Done.

[remyoudompheng, 2012-07-06 17:58:49]

Any more thoughts on this?

[r, 2012-07-09 22:28:46]

LGTM

[remyoudompheng, 2012-07-10 05:44:30]

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

strconv: extend Grisu3 algorithm to float32.

Also improve extfloat.Normalize to obtain a modest performance
gain in parsing, and add a shortcut path for exact integers.

benchmark                              old ns/op    new ns/op    delta
BenchmarkAtof64Decimal                        73           73   -0.54%
BenchmarkAtof64Float                          91           91   -0.54%
BenchmarkAtof64FloatExp                      198          180   -9.09%
BenchmarkAtof64Big                           307          308   +0.33%

BenchmarkAtof32Decimal                        72           72   +0.42%
BenchmarkAtof32Float                          83           83   -0.72%
BenchmarkAtof32FloatExp                      212          186  -12.26%
BenchmarkAtof32Random                        262          250   -4.58%

BenchmarkAppendFloatDecimal                  474          305  -35.65%
BenchmarkAppendFloat                         497          489   -1.61%
BenchmarkAppendFloatExp                      493          483   -2.03%
BenchmarkAppendFloatNegExp                   481          481   +0.00%
BenchmarkAppendFloatBig                      667          652   -2.25%

BenchmarkAppendFloat32Integer                338          307   -9.17%
BenchmarkAppendFloat32ExactFraction          364          439  +20.60%
BenchmarkAppendFloat32Point                 1299          490  -62.28%
BenchmarkAppendFloat32Exp                   2593          489  -81.14%
BenchmarkAppendFloat32NegExp                5116          481  -90.60%

R=rsc, r
CC=golang-dev, remy
http://codereview.appspot.com/6303087

[remyoudompheng, 2012-07-10 05:51:30]

Ah, for some reason I missed the new test in atof_test.go, I will include it in
another patch.