From ff5dfc94a6721f829bf2e7bb98c26a01cb2079c9 Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Wed, 25 Oct 2023 10:49:06 +0200 Subject: [PATCH 1/4] Go: preallocate memory Reduce GC load by preallocating memory. Signed-off-by: Florian Lehner --- metric/system/cgroup/util.go | 2 +- metric/system/process/process_darwin.go | 4 ++-- metric/system/process/process_linux_common.go | 4 ++-- metric/system/process/process_windows.go | 5 +++-- 4 files changed, 8 insertions(+), 7 deletions(-) diff --git a/metric/system/cgroup/util.go b/metric/system/cgroup/util.go index bf43828a2d..1c91dc9647 100644 --- a/metric/system/cgroup/util.go +++ b/metric/system/cgroup/util.go @@ -67,7 +67,7 @@ type PathList struct { // Flatten combines the V1 and V2 cgroups in cases where we don't need a map with keys func (pl PathList) Flatten() []ControllerPath { - list := []ControllerPath{} + list := make([]ControllerPath, 0, len(pl.V1)+len(pl.V2)) for _, v1 := range pl.V1 { list = append(list, v1) } diff --git a/metric/system/process/process_darwin.go b/metric/system/process/process_darwin.go index 950d9734b6..a68b854b89 100644 --- a/metric/system/process/process_darwin.go +++ b/metric/system/process/process_darwin.go @@ -64,8 +64,8 @@ func (procStats *Stats) FetchPids() (ProcsMap, []ProcState, error) { bbuf := bytes.NewBuffer(buf) - procMap := make(ProcsMap, 0) - var plist []ProcState + procMap := make(ProcsMap, len(names)) + plist := make([]ProcState, 0, len(names)) for i := 0; i < num; i++ { if err := binary.Read(bbuf, binary.LittleEndian, &pid); err != nil { diff --git a/metric/system/process/process_linux_common.go b/metric/system/process/process_linux_common.go index 38427a85c8..026cf45699 100644 --- a/metric/system/process/process_linux_common.go +++ b/metric/system/process/process_linux_common.go @@ -58,8 +58,8 @@ func (procStats *Stats) FetchPids() (ProcsMap, []ProcState, error) { return nil, nil, fmt.Errorf("error reading directory names: %w", err) } - procMap := make(ProcsMap) - var plist []ProcState + procMap := make(ProcsMap, len(names)) + plist := make([]ProcState, 0, len(names)) // Iterate over the directory, fetch just enough info so we can filter based on user input. logger := logp.L() diff --git a/metric/system/process/process_windows.go b/metric/system/process/process_windows.go index 61882d6ece..30a677fcb1 100644 --- a/metric/system/process/process_windows.go +++ b/metric/system/process/process_windows.go @@ -38,8 +38,9 @@ func (procStats *Stats) FetchPids() (ProcsMap, []ProcState, error) { return nil, nil, fmt.Errorf("EnumProcesses failed: %w", err) } - procMap := make(ProcsMap, 0) - var plist []ProcState + procMap := make(ProcsMap, len(names)) + plist := make([]ProcState, 0, len(names)) + // This is probably the only implementation that doesn't benefit from our // little fillPid callback system. We'll need to iterate over everything // manually. From 7ac91fee84e53a968968b519ce4c7d3d65b8fea9 Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Thu, 26 Oct 2023 10:42:28 +0200 Subject: [PATCH 2/4] metric/system/cgroup: add cache for v2 entries As multiple PIDs can use the same v2 cgroup, cache the result to reduce CPU usage and number of syscalls. --- metric/system/cgroup/reader.go | 6 +++++- metric/system/cgroup/util.go | 10 ++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/metric/system/cgroup/reader.go b/metric/system/cgroup/reader.go index 1debc009b9..6e27be8958 100644 --- a/metric/system/cgroup/reader.go +++ b/metric/system/cgroup/reader.go @@ -23,6 +23,7 @@ import ( "path/filepath" "strconv" "strings" + "sync" "github.com/elastic/elastic-agent-libs/logp" "github.com/elastic/elastic-agent-system-metrics/metric/system/cgroup/cgv1" @@ -68,7 +69,7 @@ const ( memoryStat = "memory" ) -//nolint: deadcode,structcheck,unused // needed by other platforms +// nolint: deadcode,structcheck,unused // needed by other platforms type mount struct { subsystem string // Subsystem name (e.g. cpuacct). mountpoint string // Mountpoint of the subsystem (e.g. /cgroup/cpuacct). @@ -85,6 +86,9 @@ type Reader struct { ignoreRootCgroups bool // Ignore a cgroup when its path is "/". cgroupsHierarchyOverride string cgroupMountpoints Mountpoints // Mountpoints for each subsystem (e.g. cpu, cpuacct, memory, blkio). + + // Cache to map known v2 cgroup controllerPaths to PathList. + v2ControllerPathCache sync.Map } // ReaderOptions holds options for NewReaderOptions. diff --git a/metric/system/cgroup/util.go b/metric/system/cgroup/util.go index 1c91dc9647..0562aaaa25 100644 --- a/metric/system/cgroup/util.go +++ b/metric/system/cgroup/util.go @@ -255,6 +255,7 @@ func (r Reader) ProcessCgroupPaths(pid int) (PathList, error) { if r.cgroupsHierarchyOverride != "" { path = r.cgroupsHierarchyOverride } + // cgroup V2 // cgroup v2 controllers will always start with this string if strings.Contains(line, "0::/") { @@ -275,6 +276,14 @@ the container as /sys/fs/cgroup/unified and start the system module with the hos controllerPath = r.rootfsMountpoint.ResolveHostFS(filepath.Join("/sys/fs/cgroup/unified", path)) } + if tmp, ok := r.v2ControllerPathCache.Load(controllerPath); ok { + pathList, ok := tmp.(PathList) + if ok { + cPaths.V2 = pathList.V2 + continue + } + } + cgpaths, err := os.ReadDir(controllerPath) if err != nil { return cPaths, fmt.Errorf("error fetching cgroupV2 controllers for cgroup location '%s' and path line '%s': %w", r.cgroupMountpoints.V2Loc, line, err) @@ -287,6 +296,7 @@ the container as /sys/fs/cgroup/unified and start the system module with the hos cPaths.V2[controllerName] = ControllerPath{ControllerPath: path, FullPath: controllerPath, IsV2: true} } } + r.v2ControllerPathCache.Store(controllerPath, cPaths) // cgroup v1 } else { subsystems := strings.Split(fields[1], ",") From 9cb7cc2ab44559438d4dfa3f897d7c5a784efa43 Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Tue, 31 Oct 2023 09:27:23 +0100 Subject: [PATCH 3/4] metric/system/cgroup: add timestamp to cache for v2 entries Make sure v2 entries do not stay forever in the cache. Signed-off-by: Florian Lehner --- metric/system/cgroup/reader.go | 9 ++++++++- metric/system/cgroup/util.go | 20 ++++++++++++++++---- 2 files changed, 24 insertions(+), 5 deletions(-) diff --git a/metric/system/cgroup/reader.go b/metric/system/cgroup/reader.go index 6e27be8958..5fbd840ba9 100644 --- a/metric/system/cgroup/reader.go +++ b/metric/system/cgroup/reader.go @@ -24,6 +24,7 @@ import ( "strconv" "strings" "sync" + "time" "github.com/elastic/elastic-agent-libs/logp" "github.com/elastic/elastic-agent-system-metrics/metric/system/cgroup/cgv1" @@ -78,6 +79,12 @@ type mount struct { fullPath string // Absolute path to the cgroup. It's the mountpoint joined with the path. } +// pathListWithTime combines PathList with a timestamp. +type pathListWithTime struct { + added time.Time + pathList PathList +} + // Reader reads cgroup metrics and limits. type Reader struct { // Mountpoint of the root filesystem. Defaults to / if not set. This can be @@ -87,7 +94,7 @@ type Reader struct { cgroupsHierarchyOverride string cgroupMountpoints Mountpoints // Mountpoints for each subsystem (e.g. cpu, cpuacct, memory, blkio). - // Cache to map known v2 cgroup controllerPaths to PathList. + // Cache to map known v2 cgroup controllerPaths to pathListWithTime. v2ControllerPathCache sync.Map } diff --git a/metric/system/cgroup/util.go b/metric/system/cgroup/util.go index 0562aaaa25..8a9329bdb9 100644 --- a/metric/system/cgroup/util.go +++ b/metric/system/cgroup/util.go @@ -25,6 +25,7 @@ import ( "path/filepath" "strconv" "strings" + "time" "github.com/elastic/elastic-agent-libs/logp" "github.com/elastic/elastic-agent-system-metrics/metric/system/resolve" @@ -276,12 +277,20 @@ the container as /sys/fs/cgroup/unified and start the system module with the hos controllerPath = r.rootfsMountpoint.ResolveHostFS(filepath.Join("/sys/fs/cgroup/unified", path)) } + // Check if there is an entry for controllerPath already cached. if tmp, ok := r.v2ControllerPathCache.Load(controllerPath); ok { - pathList, ok := tmp.(PathList) + cacheEntry, ok := tmp.(pathListWithTime) if ok { - cPaths.V2 = pathList.V2 - continue + // If the cached entry for controllerPath is not older than 5 minutes, + // return the cached entry. + if time.Since(cacheEntry.added) < time.Duration(5*time.Minute) { + cPaths.V2 = cacheEntry.pathList.V2 + continue + } } + // Consider the existing entry for controllerPath invalid. The entry can + // (1) not be casted to pathListWithTime or (2) is older than 5 minutes. + r.v2ControllerPathCache.Delete(controllerPath) } cgpaths, err := os.ReadDir(controllerPath) @@ -296,7 +305,10 @@ the container as /sys/fs/cgroup/unified and start the system module with the hos cPaths.V2[controllerName] = ControllerPath{ControllerPath: path, FullPath: controllerPath, IsV2: true} } } - r.v2ControllerPathCache.Store(controllerPath, cPaths) + r.v2ControllerPathCache.Store(controllerPath, pathListWithTime{ + added: time.Now(), + pathList: cPaths, + }) // cgroup v1 } else { subsystems := strings.Split(fields[1], ",") From 210c19999fbca15d5540588e1361917204d0dd7a Mon Sep 17 00:00:00 2001 From: Florian Lehner Date: Mon, 6 Nov 2023 13:47:06 +0100 Subject: [PATCH 4/4] replace sync.Map with custom struct with sync.RWMutex Signed-off-by: Florian Lehner --- metric/system/cgroup/reader.go | 8 +++++++- metric/system/cgroup/util.go | 31 +++++++++++++++++-------------- 2 files changed, 24 insertions(+), 15 deletions(-) diff --git a/metric/system/cgroup/reader.go b/metric/system/cgroup/reader.go index 5fbd840ba9..f9bb6bb86c 100644 --- a/metric/system/cgroup/reader.go +++ b/metric/system/cgroup/reader.go @@ -85,6 +85,11 @@ type pathListWithTime struct { pathList PathList } +type pathCache struct { + sync.RWMutex + cache map[string]pathListWithTime +} + // Reader reads cgroup metrics and limits. type Reader struct { // Mountpoint of the root filesystem. Defaults to / if not set. This can be @@ -95,7 +100,7 @@ type Reader struct { cgroupMountpoints Mountpoints // Mountpoints for each subsystem (e.g. cpu, cpuacct, memory, blkio). // Cache to map known v2 cgroup controllerPaths to pathListWithTime. - v2ControllerPathCache sync.Map + v2ControllerPathCache pathCache } // ReaderOptions holds options for NewReaderOptions. @@ -146,6 +151,7 @@ func NewReaderOptions(opts ReaderOptions) (*Reader, error) { ignoreRootCgroups: opts.IgnoreRootCgroups, cgroupsHierarchyOverride: opts.CgroupsHierarchyOverride, cgroupMountpoints: mountpoints, + v2ControllerPathCache: pathCache{cache: make(map[string]pathListWithTime)}, }, nil } diff --git a/metric/system/cgroup/util.go b/metric/system/cgroup/util.go index 8a9329bdb9..95942b7a83 100644 --- a/metric/system/cgroup/util.go +++ b/metric/system/cgroup/util.go @@ -278,20 +278,21 @@ the container as /sys/fs/cgroup/unified and start the system module with the hos } // Check if there is an entry for controllerPath already cached. - if tmp, ok := r.v2ControllerPathCache.Load(controllerPath); ok { - cacheEntry, ok := tmp.(pathListWithTime) - if ok { - // If the cached entry for controllerPath is not older than 5 minutes, - // return the cached entry. - if time.Since(cacheEntry.added) < time.Duration(5*time.Minute) { - cPaths.V2 = cacheEntry.pathList.V2 - continue - } + r.v2ControllerPathCache.Lock() + cacheEntry, ok := r.v2ControllerPathCache.cache[controllerPath] + if ok { + // If the cached entry for controllerPath is not older than 5 minutes, + // return the cached entry. + if time.Since(cacheEntry.added) < 5*time.Minute { + cPaths.V2 = cacheEntry.pathList.V2 + r.v2ControllerPathCache.Unlock() + continue } - // Consider the existing entry for controllerPath invalid. The entry can - // (1) not be casted to pathListWithTime or (2) is older than 5 minutes. - r.v2ControllerPathCache.Delete(controllerPath) } + // Consider the existing entry for controllerPath invalid, as it is + // older than 5 minutes. + delete(r.v2ControllerPathCache.cache, controllerPath) + r.v2ControllerPathCache.Unlock() cgpaths, err := os.ReadDir(controllerPath) if err != nil { @@ -305,10 +306,12 @@ the container as /sys/fs/cgroup/unified and start the system module with the hos cPaths.V2[controllerName] = ControllerPath{ControllerPath: path, FullPath: controllerPath, IsV2: true} } } - r.v2ControllerPathCache.Store(controllerPath, pathListWithTime{ + r.v2ControllerPathCache.Lock() + r.v2ControllerPathCache.cache[controllerPath] = pathListWithTime{ added: time.Now(), pathList: cPaths, - }) + } + r.v2ControllerPathCache.Unlock() // cgroup v1 } else { subsystems := strings.Split(fields[1], ",")