cmd/compile: avoid copying if copied value isn't modified #19817

valyala · 2017-04-01T14:57:02Z

Go 1.8 emits unnesessary value copyings in the following code (didn't test go tip yet, but it is likely it has the same behaviour):

func sum(p *[1000][1000]int) int {
  s := 0
  a := *p // copy a million of ints
  for i := range a { // possible copy
      b := a[i] // copy a thousand of ints
      for j := range b { // possible copy
          s += b[j]
      }
  }
  return s
}

All these copyings could be substituted by pointers, which would result in direct memory access pointed by p. This looks like safe optimization, since both a and b values are read-only in this code.
This optimization also could save stack space used for storing a and b values.

Someone could argue this optimization can't be applied since the memory pointed by p could be modified by concurrently running goroutines, which will result in data races. But this code is already vulnerable to the same data races when compiled without the optimization, since value copying isn't atomic.

The text was updated successfully, but these errors were encountered:

valyala · 2017-04-01T15:05:06Z

Issues similar to #18625 could be resolved with this optimization

randall77 · 2017-04-01T16:10:12Z

A copy is required for the a := *p statement.
The reason is the same as for this code:

   var p *int = ...
   x := *p
   return x - x

We want this code to always return 0, even in the presence of data races.
(This isn't in the spec AFAICT but we have code that relies on it. I accidentally did this optimization before and it broke mutexes. See the assignment old := m.state in sync/mutex.go:86. We can't load m.state twice.)

The internal assignments, like b := a[i], could be optimized away.

zigo101 · 2017-04-02T09:01:36Z

@valyala, you can optimize it man 8000 ually:

func sum(p *[1000][1000]int) int {
  s := 0
  a := p // just a pointer copy
  for i := range a { // just a pointer copy
      b := &a[i] // just a pointer copy
      for j := range b { // just a pointer copy
	   s += b[j]
      }
  }
  return s
}

or, even simpler

func sum(p *[1000][1000]int) int {
  s := 0
  for i := range p {
      for _, x := range &p[i] {
	   s += x
      }
  }
  return s
}

valyala · 2017-04-06T13:06:03Z

@go101 , programmers frequently forget about implicit copying and stack usage, which results to issues like #18625 . See also this tweet. So it would be great if compiler could assist the programmer in such cases.

@randall77 , probably the optimization could be applied only to arrays and structs?

randall77 · 2017-04-06T14:45:27Z

That would help the mutex case. But I still don't think it is advisable.

var p *[10]int = ...
a := *p
println(a[5])
println(a[5])

It would be very surprising if it printed different values, even in the presence of data races.

valyala · 2017-05-09T21:33:41Z

@bradfitz , could you add "performance" label to this issue?

valyala mentioned this issue Apr 18, 2017

cmd/compile: big array not allocated on heap, which make program panic. #20021

Open

randall77 added the Performance label May 9, 2017

bradfitz added this to the Unplanned milestone May 19, 2017

valyala mentioned this issue Jun 2, 2017

cmd/compile: improve escape analysis of make([]T, n) where n is non-constant #20533

Open

gopherbot added the compiler/runtime Issues related to the Go compiler and/or runtime. label Jul 13, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmd/compile: avoid copying if copied value isn't modified #19817

cmd/compile: avoid copying if copied value isn't modified #19817

cmd/compile: avoid copying if copied value isn't modified #19817

cmd/compile: avoid copying if copied value isn't modified #19817

Comments