code review 6197045: runtime: fix complex division benchmarks

runtime: fix complex division benchmarks
we can't add the division result to n during iteration, because it might
turn n into NaN or Inf.

[minux1, 2012-05-04 18:31:49]

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

I'd like you to review this change to
https://code.google.com/p/go/

[rsc, 2012-05-04 18:52:41]

LGTM

[iant2, 2012-05-04 18:54:36]

This inserts a global variable reference into the middle of the loop,
which seems less than ideal for a micro-benchmark.  Can you use a local
variable instead, and assign it to result after the loop?

Also, how does this change the benchmark results?

Ian

minux.ma@gmail.com writes:

> Reviewers: golang-dev_googlegroups.com,
>
> Message:
> Hello golang-dev@googlegroups.com (cc: golang-dev@googlegroups.com),
>
> I'd like you to review this change to
> https://code.google.com/p/go/
>
>
> Description:
> runtime: fix complex division benchmarks
> we can't add the division result to n during iteration, because it might
> turn n into NaN or Inf.
>
> Please review this at http://codereview.appspot.com/6197045/
>
> Affected files:
>   M src/pkg/runtime/complex_test.go
>
>
> Index: src/pkg/runtime/complex_test.go
> ===================================================================
> --- a/src/pkg/runtime/complex_test.go
> +++ b/src/pkg/runtime/complex_test.go
> @@ -14,44 +14,49 @@
>  func BenchmarkComplex128DivNormal(b *testing.B) {
>  	d := 15 + 2i
>  	n := 32 + 3i
> +	result = 0
>  	for i := 0; i < b.N; i++ {
> -		n += n / d
> +		n += 0.1i
> +		result += n / d
>  	}
> -	result = n
>  }
>
>  func BenchmarkComplex128DivNisNaN(b *testing.B) {
>  	d := cmplx.NaN()
>  	n := 32 + 3i
> +	result = 0
>  	for i := 0; i < b.N; i++ {
> -		n += n / d
> +		n += 0.1i
> +		result += n / d
>  	}
> -	result = n
>  }
>
>  func BenchmarkComplex128DivDisNaN(b *testing.B) {
>  	d := 15 + 2i
>  	n := cmplx.NaN()
> +	result = 0
>  	for i := 0; i < b.N; i++ {
> -		n += n / d
> +		d += 0.1i
> +		result += n / d
>  	}
> -	result = n
>  }
>
>  func BenchmarkComplex128DivNisInf(b *testing.B) {
>  	d := 15 + 2i
>  	n := cmplx.Inf()
> +	result = 0
>  	for i := 0; i < b.N; i++ {
> -		n += n / d
> +		d += 0.1i
> +		result += n / d
>  	}
> -	result = n
>  }
>
>  func BenchmarkComplex128DivDisInf(b *testing.B) {
>  	d := cmplx.Inf()
>  	n := 32 + 3i
> +	result = 0
>  	for i := 0; i < b.N; i++ {
> -		n += n / d
> +		n += 0.1i
> +		result += n / d
>  	}
> -	result = n
>  }

[minux1, 2012-05-04 19:09:18]

PTAL.

On Sat, May 5, 2012 at 2:54 AM, Ian Lance Taylor <iant@google.com> wrote:

> This inserts a global variable reference into the middle of the loop,
> which seems less than ideal for a micro-benchmark.  Can you use a local
> variable instead, and assign it to result after the loop?
>
Done.

>
> Also, how does this change the benchmark results?
>
Not much. The reason behind this CL is this:

> >  func BenchmarkComplex128DivNisNaN(b *testing.B) {
> >       d := cmplx.NaN()
> >       n := 32 + 3i
> > +     result = 0
> >       for i := 0; i < b.N; i++ {
> > -             n += n / d
> > +             n += 0.1i
> > +             result += n / d
>
n / d is NaN, so the original test will change n to NaN after the first
iteration,
which changes the test to do Nan/NaN afterwards.

>  >       }
> > -     result = n
> >  }
>

[iant, 2012-05-04 20:08:49]

LGTM

[minux1, 2012-05-10 19:09:32]

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

runtime: fix complex division benchmarks
we can't add the division result to n during iteration, because it might
turn n into NaN or Inf.

R=golang-dev, rsc, iant, iant
CC=golang-dev
http://codereview.appspot.com/6197045