Are we put everything in heap?

[SamYuan1990]

@rootfs , @marceloamaral , @wangchen615

Recently I am adding test case for our package, during the work, I found our code style as create a structure and method of the structure...

If I am right, at run time will put the instance into heap, and as most of our function without return type, hence every data update happens at heap, but not stack?

[SamYuan1990]

Additional, for example, which way below is more efficiently for us?

func do() {
something
}

func(*instance) do() output {
 something
 return
}

[SamYuan1990]

dummy test file as comparing

package main

func do() int {
	return 1
}

type Dummy struct {
	Data int
}

func (d *Dummy) do() {
	d.Data = 1
}

func (d *Dummy) String() int {
	return d.Data
}

benchmark testing case

package main

import (
	"fmt"
	"testing"
)

func BenchmarkDo(b *testing.B) {
	b.ReportAllocs()
	var n int
	for n < b.N {
		fmt.Sprint(do())
		n++
	}
}

func BenchmarkDoStructure(b *testing.B) {
	Instance := &Dummy{}
	b.ReportAllocs()
	var n int
	for n < b.N {
		Instance.do()
		fmt.Sprint(Instance.String())
		n++
	}
}

goos: darwin
goarch: amd64
pkg: tmp
cpu: Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
BenchmarkDo
BenchmarkDo-16                   1061449              1101 ns/op              55 B/op          0 allocs/op
BenchmarkDo-16                    979154              1236 ns/op              55 B/op          0 allocs/op
BenchmarkDo-16                   1087833              1107 ns/op              56 B/op          0 allocs/op
BenchmarkDo-16                   1074248              1094 ns/op              55 B/op          0 allocs/op
BenchmarkDo-16                    898882              1120 ns/op              55 B/op          0 allocs/op
BenchmarkDo-16                   1076265              1127 ns/op              55 B/op          0 allocs/op
BenchmarkDo-16                    925562              1125 ns/op              56 B/op          0 allocs/op
BenchmarkDo-16                   1085090              1106 ns/op              56 B/op          0 allocs/op
BenchmarkDo-16                   1089594              1099 ns/op              56 B/op          0 allocs/op
BenchmarkDo-16                   1080170              1106 ns/op              56 B/op          0 allocs/op
BenchmarkDoStructure
BenchmarkDoStructure-16           888404              1172 ns/op              56 B/op          0 allocs/op
BenchmarkDoStructure-16           999915              1172 ns/op              56 B/op          0 allocs/op
BenchmarkDoStructure-16           870178              1195 ns/op              55 B/op          0 allocs/op
BenchmarkDoStructure-16          1007575              1159 ns/op              56 B/op          0 allocs/op
BenchmarkDoStructure-16          1015525              1172 ns/op              56 B/op          0 allocs/op
BenchmarkDoStructure-16           894818              1199 ns/op              56 B/op          0 allocs/op
BenchmarkDoStructure-16           996910              1177 ns/op              55 B/op          0 allocs/op
BenchmarkDoStructure-16          1009312              1202 ns/op              55 B/op          0 allocs/op
BenchmarkDoStructure-16          1017454              1173 ns/op              55 B/op          0 allocs/op
BenchmarkDoStructure-16          1016924              1176 ns/op              56 B/op          0 allocs/op
PASS
coverage: 100.0% of statements
ok      tmp     37.327s

@rootfs , @marceloamaral , @wangchen615
I am not sure if my sample case above is correct, but from case above, we can see if we replace instance in heap by just return value, it seems faster at time per operator as ns/op and also memory usage as bytes per operator B/op in general?

[SamYuan1990]

btw, I am not sure how stack as cpu allocs will impacts with our real code.

[SamYuan1990]

in my point of view as prometheus collector is just require us feed data into channel, hence data in collector in fact is not mandatory for us, as can be GC by golang? as sample below prometheus.MustNewConstMetric can be a new instance in heap and p.containerDesc.containerOtherComponentsJoulesTotal, can be GCed?

ch <- prometheus.MustNewConstMetric(
				p.containerDesc.containerOtherComponentsJoulesTotal,
				prometheus.CounterValue,
				float64(container.EnergyInOther.Aggr)/miliJouleToJoule,
				container.PodName, container.ContainerName, container.Namespace, containerCommand,
			)

[rootfs]

this is a good investigation! @SamYuan1990

Also related to #382 #365

My view is that, coding wise, tight loops should be memory efficient, avoiding GC hiccups. Metrics collection should be scalable to the number of pods, the amount of metrics, or the frequency of scraping.

We also need to come up with a performance and stress test suites to support the above.

[marceloamaral]

This is a very important topic @SamYuan1990!

We should fix the heap scape issues in the code, since too much memory in the heap put a lot of pressure in the Garbage Collector.

Regarding the BenchmarkDo versus BenchmarkDoStructure it has a lot of variation, so it is a bit hard to conclude anything.
But, it looks like that BenchmarkDoStructure can do a bit more ns/op.

Regarding the Prometheus package, what do you mean with "data in collector in fact is not mandatory for us".

[SamYuan1990]

for example, let's imaging c.ContainersMetrics[containerID] is not ready from bcc package originally.
hence each 3 seconds, we will re calculate this value. and during the time, this data is at heap.
imaging, if we just need it once as a prometheus request, then we can make it a function return. then it(memory allocation) will on CPU stack and be destroyed when function return. then the value return will with return space memory allocation to the structure of prometheus as a new instance of prometheus.MustNewConstMetric(... which will always allocation memory before consumed from channel?

[SamYuan1990]

btw, just assuming as prometheus will keep the data. and most of our data is Gauge hence, why do we need keep data as c.ContainersMetrics[containerID] in our heap?

I mean for example:

c.ContainersMetrics[my_container]=5 at 1 time in memory
c.ContainersMetrics[my_container]=9 at 2 time in memory

and finally at prometheus

container[my_container]=5 at 1 time
container[my_container]=9 at 1 time

hence for kepler, we don't need keep any data, prometheus just read data from http request

# request: /metric
my_container=5 at 1 time prometheus fetch
my_container=9 at 2 time prometheus fetch

[marceloamaral]

There are a few reasons why we need to use counters.
First, the counters do not lose data, as they accumulate the prometheus pooling interval, not influencing the measurement.
Second, the Prometheus metric guideline suggests using joule counters to calculate power, then we are following it.
Third, collecting all + calculation at the time of prometheus collection can slow down the scrape process. Having a background process doing the collection and calculation improves performance.

We don't need to completely remove the heap. There are several ways to improve this.
Btw, c.ContainersMetrics is actually a pointer and will not be deleted by the garbage collector. So that's not really the problem.

[SamYuan1990]

There are a few reasons why we need to use counters. First, the counters do not lose data, as they accumulate the prometheus pooling interval, not influencing the measurement. Second, the Prometheus metric guideline suggests using joule counters to calculate power, then we are following it. Third, collecting all + calculation at the time of prometheus collection can slow down the scrape process. Having a background process doing the collection and calculation improves performance.

We don't need to completely remove the heap. There are several ways to improve this. Btw, c.ContainersMetrics is actually a pointer and will not be deleted by the garbage collector. So that's not really the problem.

May I have prometheus collection official guidelines for reference?

[marceloamaral]

Of course, the reference and discussions are in the issue #286.