headers
remyoudompheng | 15 Jun 2012 23:29
Picon

code review 6303087: strconv: extend Grisu3 algorithm to float32. (issue 6303087)

Reviewers: golang-dev_googlegroups.com,

Message:
Hello golang-dev@... (cc: golang-dev@...,
remy@...),

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

Description:
strconv: extend Grisu3 algorithm to float32.

Also improve extfloat.Normalize to obtain a modest performance
gain in parsing.

benchmark                        old ns/op    new ns/op    delta
BenchmarkAppendFloat32Decimal          353          467  +32.29%
BenchmarkAppendFloat32                1292          487  -62.31%
BenchmarkAppendFloat32Exp             2572          480  -81.34%
BenchmarkAppendFloat32NegExp          5092          480  -90.57%

BenchmarkAtof64Decimal                72           72   -0.96%
BenchmarkAtof64Float                  92           92   +0.43%
BenchmarkAtof64FloatExp              199          182   -8.54%
BenchmarkAtof64Big                   314          314   +0.00%
BenchmarkAtof64RandomBits            528          417  -21.02%
BenchmarkAtof64RandomFloats          291          296   +1.72%
BenchmarkAtof32Decimal                71           71   -0.56%
BenchmarkAtof32Float                  83           83   +0.00%
BenchmarkAtof32FloatExp              211          187  -11.37%
(Continue reading)

Permalink | Reply |
headers
rsc | 19 Jun 2012 06:05
Favicon

Re: code review 6303087: strconv: extend Grisu3 algorithm to float32. (issue 6303087)

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#newcode218
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?

http://codereview.appspot.com/6303087/
Permalink | Reply |
headers
rsc | 19 Jun 2012 06:05
Favicon

Re: code review 6303087: strconv: extend Grisu3 algorithm to float32. (issue 6303087)

LGTM

http://codereview.appspot.com/6303087/
Permalink | Reply |
headers
remyoudompheng | 19 Jun 2012 08:46
Picon

Re: code review 6303087: strconv: extend Grisu3 algorithm to float32. (issue 6303087)

Hello rsc@... (cc: golang-dev@...,
remy@...),

Please take another look.

http://codereview.appspot.com/6303087/
Permalink | Reply |
headers
rsc | 19 Jun 2012 17:11
Favicon

Re: code review 6303087: strconv: extend Grisu3 algorithm to float32. (issue 6303087)

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#newcode201
src/pkg/strconv/extfloat.go:201: if f.exp < 0 && mant ==
(mant>>uint(-f.exp))<<uint(-f.exp) {
Should this be <= 0 ?

http://codereview.appspot.com/6303087/
Permalink | Reply |

Gmane