Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions containers/container.go
Original file line number Diff line number Diff line change
Expand Up @@ -569,32 +569,32 @@ func (c *Container) Collect(ch chan<- prometheus.Metric) {
func (c *Container) onProcessStart(pid uint32) *Process {
c.lock.Lock()
defer c.lock.Unlock()
stats, err := TaskstatsPID(pid)
if err != nil {
startedAt := proc.GetStartTime(pid)
if startedAt.IsZero() {
return nil
}
if p := c.processes[pid]; p != nil {
// Already registered for this same process: a connection can be
// handled before the process start event (see attachTlsUprobes).
// Replacing it would drop its uprobes without closing them.
if p.StartedAt.Equal(stats.BeginTime) {
if p.StartedAt.Equal(startedAt) {
return p
}
// The pid was reused: the previous process exited unnoticed.
c.closeProcess(pid, p)
}
c.zombieAt = time.Time{}
p := NewProcess(pid, stats, c.registry.tracer)
p := NewProcess(pid, startedAt, c.registry.tracer)

if p == nil {
return nil
}
c.processes[pid] = p

if c.startedAt.IsZero() {
c.startedAt = stats.BeginTime
c.startedAt = startedAt
} else {
min := stats.BeginTime
min := startedAt
for _, p := range c.processes {
if p.StartedAt.Before(min) {
min = p.StartedAt
Expand Down
5 changes: 2 additions & 3 deletions containers/process.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"github.com/coroot/coroot-node-agent/gpu"
"github.com/coroot/coroot-node-agent/proc"
"github.com/jpillora/backoff"
"github.com/mdlayher/taskstats"
)

type GpuUsage struct {
Expand Down Expand Up @@ -66,8 +65,8 @@ type Process struct {
nodejsChecked bool
}

func NewProcess(pid uint32, stats *taskstats.Stats, tracer *ebpftracer.Tracer) *Process {
p := &Process{Pid: pid, StartedAt: stats.BeginTime}
func NewProcess(pid uint32, startedAt time.Time, tracer *ebpftracer.Tracer) *Process {
p := &Process{Pid: pid, StartedAt: startedAt}
p.Flags, _ = proc.GetFlags(pid)
p.ctx, p.cancelFunc = context.WithCancel(context.Background())
go p.instrument(tracer)
Expand Down
13 changes: 0 additions & 13 deletions containers/taskstats.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,16 +33,3 @@ func TaskstatsTGID(pid uint32) (*taskstats.Stats, error) {
}
return s, nil
}

func TaskstatsPID(pid uint32) (*taskstats.Stats, error) {
if taskstatsClient == nil {
return nil, fmt.Errorf("taskstats client not initialized")
}
taskstatsLock.Lock()
defer taskstatsLock.Unlock()
s, err := taskstatsClient.PID(int(pid))
if err != nil {
return nil, err
}
return s, nil
}
1 change: 1 addition & 0 deletions proc/fixtures/123/stat
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
123 (my (odd) proc) S 1 4242 4242 0 -1 4194560 100 0 0 0 5 3 0 0 20 0 1 0 12345 1000000 200 18446744073709551615 0 0 0 0 0 0 0 0 0 0 0 0 17 0 0 0 0 0 0
40 changes: 39 additions & 1 deletion proc/proc.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,28 @@ import (
"path"
"strconv"
"strings"
"time"

"github.com/coroot/coroot-node-agent/cgroup"
)

var root = "/proc"
var (
root = "/proc"
bootTime int64
)

func init() {
data, err := os.ReadFile(root + "/stat")
if err != nil {
return
}
for _, line := range strings.Split(string(data), "\n") {
if fields := strings.Fields(line); len(fields) == 2 && fields[0] == "btime" {
bootTime, _ = strconv.ParseInt(fields[1], 10, 64)
return
}
}
}

func Path(pid uint32, subpath ...string) string {
return path.Join(append([]string{root, strconv.Itoa(int(pid))}, subpath...)...)
Expand Down Expand Up @@ -64,6 +81,27 @@ func ReadCgroup(pid uint32) (*cgroup.Cgroup, error) {
return cgroup.NewFromProcessCgroupFile(Path(pid, "cgroup"))
}

func GetStartTime(pid uint32) time.Time {
data, err := os.ReadFile(Path(pid, "stat"))
if err != nil {
return time.Time{}
}
s := string(data)
idx := strings.LastIndex(s, ")")
if idx < 0 {
return time.Time{}
}
fields := strings.Fields(s[idx+1:])
if len(fields) < 20 {
return time.Time{}
}
startTicks, err := strconv.ParseUint(fields[19], 10, 64)
if err != nil || bootTime == 0 {
return time.Time{}
}
return time.Unix(bootTime+int64(float64(startTicks)/100), 0)
}
Comment on lines +84 to +103

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The current implementation of GetStartTime converts the entire /proc/<pid>/stat file to a string and splits it into fields using strings.Fields. Since /proc/<pid>/stat contains around 50 fields, this approach triggers dozens of string allocations on every process start.

Because GetStartTime is executed on every process start event, optimizing it to be allocation-free significantly reduces CPU and memory overhead in high-churn environments.

We can achieve this by scanning the byte slice directly to find the 20th field after the last ) character, and parsing the uint64 directly from the bytes without any string conversions or allocations.

func GetStartTime(pid uint32) time.Time {
	data, err := os.ReadFile(Path(pid, "stat"))
	if err != nil {
		return time.Time{}
	}
	idx := bytes.LastIndex(data, []byte{')'})
	if idx < 0 {
		return time.Time{}
	}
	tail := data[idx+1:]
	fieldCount := 0
	inField := false
	start, end := -1, -1
	for i, b := range tail {
		if b == ' ' || b == '\t' || b == '\n' || b == '\r' {
			if inField {
				inField = false
				if fieldCount == 20 {
					end = i
					break
				}
			}
		} else {
			if !inField {
				inField = true
				fieldCount++
				if fieldCount == 20 {
					start = i
				}
			}
		}
	}
	if inField && fieldCount == 20 {
		end = len(tail)
	}
	if start < 0 || end < 0 {
		return time.Time{}
	}
	var startTicks uint64
	for _, b := range tail[start:end] {
		if b < '0' || b > '9' {
			return time.Time{}
		}
		startTicks = startTicks*10 + uint64(b-'0')
	}
	if bootTime == 0 {
		return time.Time{}
	}
	return time.Unix(bootTime+int64(startTicks/100), 0)
}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I measured this, and I'm not changing it.

  • strings.Fields doesn't allocate per field: the substrings share the original string's memory.
  • With testing.AllocsPerRun, GetStartTime makes 10 allocations per call. 8 of them come from Path plus os.ReadFile, which the suggested version keeps. Parsing adds 2: the string(data) copy and the Fields slice.
  • So the hand-written scanner would save 2 small allocations per process start: about 200/s at 100 process starts per second. That isn't worth the extra parsing code.
  • The change this replaces, a taskstats netlink call per process start, already cut agent CPU by about 14% in the e2e.


func ListPids() ([]uint32, error) {
root, err := os.Open(root)
if err != nil {
Expand Down
24 changes: 24 additions & 0 deletions proc/start_time_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
package proc

import (
"testing"
"time"

"github.com/stretchr/testify/assert"
)

func TestGetStartTime(t *testing.T) {
saved := bootTime
defer func() { bootTime = saved }()
bootTime = 1700000000

// starttime is field 22 of /proc/<pid>/stat, in clock ticks since boot:
// 12345 ticks = 123.45s. The comm contains spaces and ')', so fields
// must be counted from the last ')'.
assert.Equal(t, time.Unix(1700000000+123, 0), GetStartTime(123))

assert.True(t, GetStartTime(999999).IsZero(), "missing pid")

bootTime = 0
assert.True(t, GetStartTime(123).IsZero(), "unknown boot time")
}
Loading