s := make([]byte, 0, 6)

// =>

s := make([]byte, 0, 8)

Note: 0-sized types should be ignored.

I don't think that this can be implemented in ruleguard rules as we need to do some complicated computations.

Calling String() once is preferred to avoid redundant []byte->string copies.

Note: if buffer is replaced with strings Builder then it's not a problem.

var zero = make([]byte, 1024 * 10)

func clear(b []byte) {
  copy(b, zero)
}

=>

func clear(b []byte) {
  for i := range b {
    b[i] = 0
  }
}

The compiler would recognize this and insert memclrNoHeapPointers call there.

go-faster/city@8e81405

var-based assignments should be treated identically to := assignments.

fmt.Sprintf("%d %d", x, y)
= 
fmt.Sprint(x) + " " + fmt.Sprint(y)
=
fmt.Sprint(x, y) // adds a space between args if neither is a string

TODO: benchmark these?

When doing []byte(str) in fmt arguments, the copy will be eagerly made. This is an extra allocation + memory copying.

If %s is used and []byte is passed as is, no copy is made, bytes are printed as a string to the result.

fmt.Sprintf("foo %s", string(b))
=>
fmt.Sprintf("foo %s", b)

We have per-line coverage, but it's hard to use it when branch prediction is needed.

Especially important for multi-line replacements.

Suppose there is an X service running in staging or production with profiling enabled.

go-perfguard should handle these cases:

• Using a profile to optimize the app code (vendor/go mod imported packages remain unchanged)
• Using a profile to optimize the library used in X (no app code is available, changes are applied to the library code)

An example:

s := strconv.FormatInt(int64(0xffffffff), 10)

This expression always allocates a new string "4294967295".

Replacing a call with that is not ideal as it hurts readability.
But we can maybe do this calculation only once and use a variable then instead?

Or maybe something like this:

s := "4294967295" // folded strconv.FormatInt(int64(0xffffffff), 10)

In any case, it may be worthwhile in hot spots.

func countUniq(data []int) int {
	set := make(map[int]struct{}, len(data))
	if len(data) == 0 {
		return 0
	}
	for _, x := range data {
		set[x] = struct{}{}
	}
	return len(set)
}

=>

func countUniq(data []int) int {
	if len(data) == 0 {
		return 0
	}
	set := make(map[int]struct{}, len(data))
	for _, x := range data {
		set[x] = struct{}{}
	}
	return len(set)
}

If io.Copy is called inside a loop, one tmp buf can be allocated and used with io.CopyBuffer.
Especially if writer doesn't implement WriterTo and reader doesn't implement ReaderFrom.

There is a problem that this would require making several code changes instead of just one:

1. Initialization of the map
2. Usages of the map (both reads and writes)

It should be possible to start from reporting the suggestion without applying it, like a warning.

For local counters that are only read after the critical section is over, it's possible to use atomics instead of Lock+increment+Unlock (and other combinations).

Some patterns can be replaced by simple string ops.

ok, _ := filepath.Match("*.go", s)
=>
ok := strings.HasSuffix(s, ".go")

strings.Count(s, ".") == 0
=>
!strings.Contains(s, ".")

And so on.

Same for bytes package.

bytes.Compare(b1, b2) == 0
=>
bytes.Equal(b1, b2)

const idLen = 8
strings.Count(s, "f") + strings.Count(s, "F") == idLen

=>

strings.EqualFold(s, "ffffffff")

strings.Contains(name, " ") || strings.Contains(name, "`")
strings.ContainsRune(name, ' ') || strings.ContainsRune(name, '`')
=>
strings.ContainsAny(name, " `")

Combine calls?

col.Name = strings.Trim(strings.Trim(field, "`[] "), `"`)
=> 
col.Name = strings.Trim(field, "`[] \"")

It should be possible to know whether X line did any significant allocations or not.

Since we're using only CPU profile, we should rely on newobject, makeslice and other allocation function calls in these places.

Other functions that can be interesting:

• runtime.convTslice (and other conv functions)
• runtime.growslice

// cap may break the optimization
dst = make([]T, len(src), len(src))
copy(dst, src)

Also, accessing src slice with expressions like o.src can also break the optimization.

See also: https://github.com/golang/go/issues/44628

This analysis can be func-local.

If we see a statement like parser.tab = map[T]K{} and that parser.tab is used below (in the same function), then it could be beneficial to clear the map instead of replacing it with a new map.

Same goes for the statements like:

m = make(map[T]K, len(m))

We can start by some simple ruleguard rules and then implement a proper analysis for this.

Should be careful here: mapclear is not always better than a realloc.

Update: partially implemented by #98

MethodByName uses a linear search and it may be beneficial to cache the results.
Or avoid using MethodByName in hot paths at all, whether possible.

If we add an o2 rule in opt_rules.go, then we can report usages of MethodByName in hot paths.

for _, x := range xs {
  obj := &object{x: x}
  f(obj)
}
// =>
var obj object
for _, x := range xs {
  obj = object{x: x}
  f(&obj)
}

But we need to know (somehow) that this object pointer is not retained inside f.

Another example of this would be:

for _, x := range xs {
  var buf bytes.Buffer
  buf.Write(x.a)
  buf.Write(x.b)
  f(buf.Bytes())
}
// =>
var buf bytes.Buffer
for _, x := range xs {
  buf.Reset()
  buf.Write(x.a)
  buf.Write(x.b)
  f(buf.Bytes())
}

When there are multiple edits for a single line, we may want to select the one that has the highest weight.

var xs []T
xs = append(xs, x)

// =>

var xs = []T{x}

var xs []T
xs = append(xs, x1)
xs = append(xs, x2)

// =>

var xs = []T{x1, x2}

More examples https://go.dev/play/p/e34FVX-sqQx

Use case:

func f() string {
  buf := bytes.NewBuffer(make([]byte, 0, sizehint))
  // use buf...
  return buf.String()
}

// =>

func f() string {
  buf := strings.Builder{}
  buf.Grow(sizehint)
  // use buf...
  return buf.String()
}

When writes are unconditional and it's possible to find out their len, we can have a pretty good Grow size hint.

func test(s1, s2, s3 string) string {
	var buf strings.Builder
	if s1 != "" {
		buf.WriteString(s1)
	}
	buf.WriteString(s2)
	buf.WriteString(s3)
	return buf.String()
}

// =>

func test(s1, s2, s3 string) string {
	var buf strings.Builder
	buf.Grow(len(s2) + len(s3))
	if s1 != "" {
		buf.WriteString(s1)
	}
	buf.WriteString(s2)
	buf.WriteString(s3)
	return buf.String()
}

Since not only increment does avoid the double hashing, but any <op>= operation, we should suggest all of them.

For example, this is not optimized and runtime.slicebytetostring will be generated:

func lookup(m map[string]int, k []byte) int {
	key := string(k)
	return m[key]
}

Needs investigation.
The constant overhead of calling Write via WriteString should be small enough, I think.
If w happens to have WriteString method, it could save some time and remove the redundant data copying.

Maybe we can use the profiling info to see whether that call actually involved big data copying and suggest it only in these cases?

We can have a growslice index that will help us identify appends to a slice with a hint that resulted in allocation.
See #66

Sometimes people allocate slices before checking that they actually need to do this.

func f(o *object, num int) []item {
  result := make([]item, 0, num)
  if o != nil {
    for _, v := range o.items[:num] {
      result = append(result, v)
    }
  }
  return result
}

// =>

func f(o *object, num int) []item {
  result := []item{} // To avoid returning a nil slice, that could change the API
  if o != nil {
    result = make([]item, 0, num)
    for _, v := range o.items[:num] {
      result = append(result, v)
    }
  }
  return result
}

func joinData(x, y []byte) (result []byte) {
  result = append(result, x...)
  result = append(result, y...)
  return result
}
=>
func joinData(x, y []byte) (result []byte) {
  result = make([]byte, 0, len(x) + len(y))
  result = append(result, x...)
  result = append(result, y...)
  return result
}

This is probably something that is easier to do in SSA form.

• Which rules matched and how many times