Hacker Newsnew | past | comments | ask | show | jobs | submitlogin

I'm surprised it doesn't eliminate the BenchmarkAtan2 entirely:

* math.Atan2 is in the standard library, so it should know it has no side effects and can be replaced with an empty statement. Unless Go has a way to mark a function as not having side effects, no user-defined function would have that property.

* The only observable impact of the loop if math.Atan2 is removed is that b.N could be a larger size type than n, so it could loop forever depending on the values. If they're known to be the same type, it could eliminate the loop entirely.

Are you sure you're compiling with optimizations enabled?

I also wonder if that DIVSD instruction in yours might cause a floating point exception, which would be a different behavior compared to atan2.



Some optimizations are disabled when Golang is running benchmarks.


I don't think that is correct. Do you have a link to more information about it.

When writing benchmarks in Go you always need to take steps to circumvent the optimizer detecting functions with no side effects. Assigning the result of a benchmark to a public package level variable is usually enough.


As in Rust, which has a test::black_box, which is unintelligible to the optimizer.


Author here.

> I'm surprised it doesn't eliminate the BenchmarkAtan2 entirely:

I'm sorry, could you clarify what you mean by "eliminate" here. Are you saying I shouldn't benchmark atan2 but something else ?

> Are you sure you're compiling with optimizations enabled?

I just did - `go test -bench=. -benchmem -cpu=1`. Did I miss something ?

> I also wonder if that DIVSD instruction in yours might cause a floating point exception, which would be a different behavior compared to atan2.

I checked with the assembly output of math.Atan2. It also uses DIVSD.


> I'm sorry, could you clarify what you mean by "eliminate" here.

Most compilers will eliminate code if they can tell the results of a calculation are never used. Benchmarks typically call some function and discard the result. If you aren't careful, the compiler can often optimize away your entire benchmark down to nothing.

This is why you often see benchmarks that look like:

    int average(a, b) {
      return (a + b) / 2;
    }

    main() {
      int sum;
      for (int i = 0; i < 100000; i++) {
        for (int j = 0; j < 100000; j++) {
          sum += average(i, j);
        }
      }

      if (sum != 123) printf("!");
    }
Instead of:

    int average(a, b) {
      return (a + b) / 2;
    }

    main() {
      for (int i = 0; i < 100000; i++) {
        for (int j = 0; j < 100000; j++) {
          average(i, j);
        }
      }
    }


Ah I see ! Gotcha :)

Yes, it seems Go is smart enough not to eliminate those values entirely.

I think this might be a better way to be sure that the function will be computed entirely.

`_ = myatan2(-479, 123)`

The result is same though. I will update the post.


Even better is to make a global var sink interface{}, and assign to it. https://github.com/golang/go/issues/14813


I don't know about go specifically, but in general if a compiler sees that the result of an expression is thrown away (like here: you don't use it anywhere), and there are no side effects to the expression, then it's okay to replace it with a noop. Usually it's better to write benchmarks in a way that the result of the mathematical expressions is used, like if you kept a running sum of `atan2(rand(), rand())` and then printed it.


>could you clarify what you mean by "eliminate" here.

He's talking about a common compiler optimization: https://en.wikipedia.org/wiki/Dead_code_elimination

Some compilers are smart enough to not run code that doesn't have any side effects. (Running a function a million times in a row but never doing anything with the results)

Hopefully golang bench is smart enough to not be that smart.


Got it. Thanks !

> Hopefully golang bench is smart enough to not be that smart.

Yes, that seems to be true.




Guidelines | FAQ | Lists | API | Security | Legal | Apply to YC | Contact

Search: