Avoid returning slice into buffer from Readfile

[eriknordmark]

The memory usage is quite ineffecient without this fix since the whole Readfile buffer can not be garbage collected due to the name and status short strings being slices with the underlying array being the Readfile buffer.

[shirou]

process package has a BenchmarkProcessName. So I did

GODEBUG=gctrace=1  go test -bench BenchmarkProcessName  -benchmem

before

pkg: github.com/shirou/gopsutil/process
BenchmarkProcessName-2          gc 4 @1.765s 0%: 0.11+0.37+0.003 ms clock, 0.22+0/0.13/0.22+0.006 ms cpu, 0->0->0 MB, 4 MB goal, 2 P (forced)
gc 5 @1.767s 0%: 0.060+0.17+0.001 ms clock, 0.12+0/0.18/0.093+0.003 ms cpu, 0->0->0 MB, 4 MB goal, 2 P (forced)
gc 6 @1.767s 0%: 0.090+0.17+0.002 ms clock, 0.18+0/0.16/0.11+0.004 ms cpu, 0->0->0 MB, 4 MB goal, 2 P (forced)
gc 7 @1.777s 0%: 0.34+0.73+0.001 ms clock, 0.69+0/0.34/0.33+0.003 ms cpu, 0->0->0 MB, 4 MB goal, 2 P (forced)
gc 8 @2.357s 0%: 0.041+3.5+0.002 ms clock, 0.082+0/0.19/3.4+0.004 ms cpu, 0->0->0 MB, 4 MB goal, 2 P (forced)
207386386                5.76 ns/op            0 B/op          0 allocs/op

PR applied

pkg: github.com/shirou/gopsutil/process
BenchmarkProcessName-2          gc 4 @1.767s 0%: 0.23+0.24+0.002 ms clock, 0.47+0/0.29/0.078+0.004 ms cpu, 0->0->0 MB, 4 MB goal, 2 P (forced)
gc 5 @1.768s 0%: 0.050+0.16+0.001 ms clock, 0.10+0/0.15/0.11+0.003 ms cpu, 0->0->0 MB, 4 MB goal, 2 P (forced)
gc 6 @1.768s 0%: 0.13+0.34+0.001 ms clock, 0.26+0/0.082/0.19+0.003 ms cpu, 0->0->0 MB, 4 MB goal, 2 P (forced)
gc 7 @1.778s 0%: 0.12+0.27+0.002 ms clock, 0.25+0/0.16/0.25+0.005 ms cpu, 0->0->0 MB, 4 MB goal, 2 P (forced)
gc 8 @2.356s 0%: 0.23+0.56+0.005 ms clock, 0.46+0/0.56/0.12+0.011 ms cpu, 0->0->0 MB, 4 MB goal, 2 P (forced)
207829984                6.10 ns/op            0 B/op          0 allocs/op

Seems nothing changed. And I try to change get new Process() every time, also nothing changed.

 func BenchmarkProcessName(b *testing.B) {
-       p := testGetProcess()
        for i := 0; i < b.N; i++ {
+               p := testGetProcess()
                p.Name()
        }
 }

Am I missed something?

[eriknordmark]

Am I missed something?

Your test is making p.Name() available for garbage collection each time around the loop, hence you can not see how much memory is held as a result of assigning some variable to p.Name(). Note that it isn't a leak; it is a memory usage expansion where retaining a very short slice is keeping a reference on a much large chunk of memory for the array underlaying the slice.

To see that p.Name() is using holding a reference on a large chunk of memory you have to keep it around and compare with the case when it is reallocated to use a buffer which is only the size of the name string itself (plus whatever golang overhead for the slice).

The code below shows that (by doing the reallocation on top of gopsutil - I'm too clueless to build the package with my PR and use it for the test).

The output without fix is:
./process-name -c 100000
Alloc 0 Mb, TotalAlloc 0 Mb, Sys 68 Mb, NumGC 0
Alloc 223 Mb, TotalAlloc 3711 Mb, Sys 339 Mb, NumGC 141
Number of processes 100000

After:
/process-name -c 100000 -r
Alloc 0 Mb, TotalAlloc 0 Mb, Sys 68 Mb, NumGC 0
Alloc 6 Mb, TotalAlloc 3710 Mb, Sys 71 Mb, NumGC 1233
Number of processes 100000

Thus about 60 byte per p.Name vs 2.2 kbyte per p.Name.

Note that I don't think the user of gopsutil/process should need to know that p.Name has a reference on a large buffer and needs to be reallocated to save space, hence fixing it in the caller as in this test program seems just wrong. But the test program shows the impact on the memory usage.

Note that the real usecase is when saving the names of all the processes on the server, which could be thousands, and not saving a single process name over and over again as in this example code.

Simplest code (I can also share code which dumps sorted output from runtime.MemProfileRecord showing where the allocations where made):

package main

import (
	"fmt"
	"flag"
	"os"
	"runtime"

	"github.com/shirou/gopsutil/process"
)

func getProcessNames(count int, realloc bool) []string {
	var names []string

	for i := 0; i < count; i++ {
		p := testGetProcess()
		name, _ := p.Name()
		if realloc {
			name = string([]byte(name))
		}
		names = append(names, name)
	}
	return names
}

func testGetProcess() process.Process {
	checkPid := os.Getpid() // process.test
	ret, _ := process.NewProcess(int32(checkPid))
	return *ret
}

func main() {
	reallocPtr := flag.Bool("r", false, "realloc flag")
	countPtr := flag.Int("c", 10000, "process count")
	flag.Parse()
	logMemUsage()
	p := getProcessNames(*countPtr, *reallocPtr)
	logMemUsage()
	fmt.Printf("Number of processes %d\n", len(p))
}

func logMemUsage() {
	var m runtime.MemStats

	runtime.ReadMemStats(&m)
	fmt.Printf("Alloc %d Mb, TotalAlloc %d Mb, Sys %d Mb, NumGC %d\n",
		roundToMb(m.Alloc), roundToMb(m.TotalAlloc), roundToMb(m.Sys), m.NumGC)
}

func roundToMb(b uint64) uint64 {

	kb := (b + 512) / 1024
	mb := (kb + 512) / 1024
	return mb
}

[shirou]

Understand. This is realloc issue, so heap alloc keep low but GC is much more. Keep heap lower is better, so I merge this PR. Thank you for sharing your cool knowledge about memory!