cmd/compile: constant string -> []byte and []byte -> string conversions aren't constant folded

[FMNSSun]

What did you see?

Conversions like

string([]byte{...})
[]byte("...")

are not constant folded.

What did you expect?

Conversions of const literals should probably be constant folded because it significantly slows down
things like

for .... {
  p.Write([]byte(":=")
}

This is related to #26264 but the scope of that issues should probably be the difference between Write, WriteByte, WriteString as some conversions not being constant folded is an issue on a different level that has little to do with the performance of these functions per se.

I can't quite tell whether []rune(...) is also not constant folded because I'm not sure what type.[]int32 actually does but for []byte(...) and string(...) there are calls to runtime.stringtoslicebyte(SB) and runtime.slicebytetostring(SB).

What version of Go are you using (go version)?

1.10.3

[mvdan]

Could be a duplicate of #10170, although I'm not sure.

cc @dr2chase @josharian

[martisch]

Note that p.Write could change the byte slice given to it. So a new byteslice needs to be passed each time as byte slices are not constant. That is what stringtoslicebyte does by copying the contents of the string to a new slice.

The slicebytetostring call in the below example could be avoided by the compiler by referencing a constant string.

func mystring() string {
    return string([]byte{'a','b','c'})
}

[FMNSSun]

Note that p.Write could change the byte slice given to it. So a new byteslice needs to be passed each time as byte slices are not constant.

That's certainly true yes. But []byte("abc") should essentially be the same as []byte{'a','b','c'} but []byte("abc") has additional runtime costs which []byte{'a','b','c'} doesn't have. In another issue #6643 it mentions that this is disabled due to each byte literal being a node in the (A)ST.
Equally, string([]byte{'a','b','c'}) could be turned into "abc" as strings are immutable - at least spec-wise. But to be fair string([]byte{'a,',b','c'}) (and string([]byte("abc")) is essentially non-sense) is probably rather rare - but []byte("foo") is much more frequent because it's more concise:

[]byte("foo")
[]byte{'f','o','o'}

Once there's a fix/improvement for #6643 this will probably be doable with such a fix. Maybe by adding a special syntax for raw []byte literals who knows.

I think there was a list of common mistakes (or similar). Maybe one can add this to the list to avoid []byte(...) in hot loops and either use

s := []byte(...)
for ... {
  f(s)
}

or

for ... {
  f([]byte{...})
}

Technically with const analysis/escape analysis the compiler could also determine whether the function modifies the slice it receives/or leaks references/pointers and automatically transform

for ... {
 f([]byte(...))
}

to

<temp> := []byte(..)
for ... {
  f(<temp>)
}

(one should pull this out of function scope completely to avoid creating a new slice on each function call.)

but I'm sensing issues with plugins/dynamic libraries there.

[martisch]

Thanks for the clarification.

The problem of detecting if a function modifies or leaks []byte array pointers and not to make a copy is i think captured in #6714.

Its seems true that []byte("abc") has more runtime cost than []byte{'a','b','c'} since the later is just a new object call followed by a memcopy (that may be inlined if short since the length is known at compile time) while the former has a bit more work in a generic runtime conversion function that does not know that the length is known at compile time.

One action for this issue might be to make []byte("abc") as efficient in that regard.

Another that occurs to me while looking at the generated code is that newobject clears the allocated memory that is overwritten immediately. There are no pointers in the backing array so this seems unnecessary.

[FMNSSun]

There might also be return ... cases that might profit from this:

func f(what int) []byte {
 switch what {
  case 0:
   return []byte("abc")
  ....
  default:
   return []byte("cde")
 }

for ... {
  p.Write(f(j))
}

which is probably easier to detect if the function is inlinable because otherwise the analysis/code gen becomes more complicated as you'd probably almost need two versions of the function depending on whether it's used in a context where it's safe to reuse the returned value because it's only used in a read-only fashion and luckily you can't compare slices so nobody can expect that f(0) != f(0).

[quasilyte]

For benchmark like this:

package foo

import "testing"

//go:noinline
func string2bytes() []byte {
	return []byte("hello, world")
}

//go:noinline
func bytesLit() []byte {
	return []byte{'h', 'e', 'l', 'l', 'o', ',', ' ', 'w', 'o', 'r', 'l', 'd'}
}


func BenchmarkOSTRARRAYBYTE(b *testing.B) {
	for i := 0; i < b.N; i++ {
		// _ = bytesLit()
		_ = string2bytes()
	}
}

We have about 25% performance difference:

name             old time/op  new time/op  delta
OSTRARRAYBYTE-8  59.6ns ± 3%  44.2ns ± 6%  -25.94%  (p=0.000 n=10+10)

For even smaller strings, it's bigger (obviously, for bigger strings alloc/memcpy time will dominate).

I'll send a CL that rewrites []byte("...") into []byte{'.', '.', '.'} in a moment.

This doesn't solve #10170 because escape analysis happens before walk.

[gopherbot]

Change https://golang.org/cl/125796 mentions this issue: cmd/compile/internal/gc: optimize []byte(stringlit)

[gopherbot]

Change https://golang.org/cl/140698 mentions this issue: cmd/compile: make []byte("...") more efficient

[gopherbot]

Change https://golang.org/cl/141118 mentions this issue: cmd/compile: optimize loads from readonly globals into constants