From e5b00c6267d8333dc4b9c109da97f620601fe9f8 Mon Sep 17 00:00:00 2001 From: Tudor Golubenco Date: Wed, 4 May 2016 11:26:21 +0200 Subject: [PATCH] Re-factor to fix high CPU usage on windows (#1562) * Re-factor to fix high CPU usage on windows This decouples getting the basic state information from getting the more detailed information. The reason is that filtering only needs the name, but the code was getting all details and then apply filtering. Getting the command line is expensive on Windows, so that contributed to #1460. What made it worse is that due to filtering, the command line cache was invalidated on each run, which explains why topbeat was consuming more CPU when filtering out processes than when not. Fixes #1460. This also pre-compiles the regexps, which brings some slight CPU usage improvements and uncovers compilation errors earlier. --- CHANGELOG.asciidoc | 2 ++ topbeat/beat/sigar.go | 69 ++++++++++++++++++++---------------- topbeat/beat/sigar_test.go | 11 ++++-- topbeat/beat/topbeat.go | 44 +++++++++++++++++------ topbeat/beat/topbeat_test.go | 3 ++ 5 files changed, 86 insertions(+), 43 deletions(-) diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc index 36359168f238..69195b65216f 100644 --- a/CHANGELOG.asciidoc +++ b/CHANGELOG.asciidoc @@ -31,6 +31,8 @@ https://github.com/elastic/beats/compare/v1.2.2...1.2[Check the HEAD diff] *Topbeat* +- Fixed high CPU usage when using filtering under Windows. {pull}1562[1562] + *Filebeat* *Winlogbeat* diff --git a/topbeat/beat/sigar.go b/topbeat/beat/sigar.go index c87011c46a07..55b5cfea519c 100644 --- a/topbeat/beat/sigar.go +++ b/topbeat/beat/sigar.go @@ -244,52 +244,61 @@ func getProcState(b byte) string { return "unknown" } -func GetProcess(pid int, cmdline string) (*Process, error) { +// newProcess creates a new Process object based on the state information. +func newProcess(pid int) (*Process, error) { + state := sigar.ProcState{} if err := state.Get(pid); err != nil { return nil, fmt.Errorf("error getting process state for pid=%d: %v", pid, err) } + proc := Process{ + Pid: pid, + Ppid: state.Ppid, + Name: state.Name, + State: getProcState(byte(state.State)), + Username: state.Username, + ctime: time.Now(), + } + + return &proc, nil +} + +// getDetails fills in CPU, memory, and command line details for the process +func (proc *Process) getDetails(cmdline string) error { + mem := sigar.ProcMem{} - if err := mem.Get(pid); err != nil { - return nil, fmt.Errorf("error getting process mem for pid=%d: %v", pid, err) + if err := mem.Get(proc.Pid); err != nil { + return fmt.Errorf("error getting process mem for pid=%d: %v", proc.Pid, err) + } + proc.Mem = &ProcMemStat{ + Size: mem.Size, + Rss: mem.Resident, + Share: mem.Share, } cpu := sigar.ProcTime{} - if err := cpu.Get(pid); err != nil { - return nil, fmt.Errorf("error getting process cpu time for pid=%d: %v", pid, err) + if err := cpu.Get(proc.Pid); err != nil { + return fmt.Errorf("error getting process cpu time for pid=%d: %v", proc.Pid, err) + } + proc.Cpu = &ProcCpuTime{ + Start: cpu.FormatStartTime(), + Total: cpu.Total, + User: cpu.User, + System: cpu.Sys, } if cmdline == "" { args := sigar.ProcArgs{} - if err := args.Get(pid); err != nil { - return nil, fmt.Errorf("error getting process arguments for pid=%d: %v", pid, err) + if err := args.Get(proc.Pid); err != nil { + return fmt.Errorf("error getting process arguments for pid=%d: %v", proc.Pid, err) } - cmdline = strings.Join(args.List, " ") - } - - proc := Process{ - Pid: pid, - Ppid: state.Ppid, - Name: state.Name, - State: getProcState(byte(state.State)), - Username: state.Username, - CmdLine: cmdline, - Mem: &ProcMemStat{ - Size: mem.Size, - Rss: mem.Resident, - Share: mem.Share, - }, - Cpu: &ProcCpuTime{ - Start: cpu.FormatStartTime(), - Total: cpu.Total, - User: cpu.User, - System: cpu.Sys, - }, - ctime: time.Now(), + proc.CmdLine = strings.Join(args.List, " ") + } else { + proc.CmdLine = cmdline } - return &proc, nil + return nil } func GetFileSystemList() ([]sigar.FileSystem, error) { diff --git a/topbeat/beat/sigar_test.go b/topbeat/beat/sigar_test.go index 1d80b566af00..1f11a3906431 100644 --- a/topbeat/beat/sigar_test.go +++ b/topbeat/beat/sigar_test.go @@ -83,12 +83,13 @@ func TestGetProcess(t *testing.T) { for _, pid := range pids { - process, err := GetProcess(pid, "") - + process, err := newProcess(pid) if err != nil { continue } assert.NotNil(t, process) + err = process.getDetails("") + assert.NoError(t, err) assert.True(t, (process.Pid > 0)) assert.True(t, (process.Ppid >= 0)) @@ -166,7 +167,11 @@ func BenchmarkGetProcess(b *testing.B) { cmdline = p.CmdLine } - process, err := GetProcess(pid, cmdline) + process, err := newProcess(pid) + if err != nil { + continue + } + err = process.getDetails(cmdline) if err != nil { continue } diff --git a/topbeat/beat/topbeat.go b/topbeat/beat/topbeat.go index b4ecd1bd8f07..88c262036028 100644 --- a/topbeat/beat/topbeat.go +++ b/topbeat/beat/topbeat.go @@ -2,6 +2,7 @@ package beat import ( "errors" + "fmt" "math" "regexp" "strconv" @@ -21,6 +22,7 @@ type ProcsMap map[int]*Process type Topbeat struct { period time.Duration procs []string + regexps []*regexp.Regexp procsMap ProcsMap lastCpuTimes *CpuTimes lastCpuTimesList []CpuTimes @@ -101,9 +103,10 @@ func (tb *Topbeat) Setup(b *beat.Beat) error { } func (t *Topbeat) Run(b *beat.Beat) error { - var err error - - t.initProcStats() + err := t.initProcStats() + if err != nil { + return err + } ticker := time.NewTicker(t.period) defer ticker.Stop() @@ -157,12 +160,21 @@ func (t *Topbeat) Stop() { close(t.done) } -func (t *Topbeat) initProcStats() { +func (t *Topbeat) initProcStats() error { t.procsMap = make(ProcsMap) if len(t.procs) == 0 { - return + return nil + } + + t.regexps = []*regexp.Regexp{} + for _, pattern := range t.procs { + reg, err := regexp.Compile(pattern) + if err != nil { + return fmt.Errorf("Failed to compile regexp [%s]: %v", pattern, err) + } + t.regexps = append(t.regexps, reg) } pids, err := Pids() @@ -171,13 +183,20 @@ func (t *Topbeat) initProcStats() { } for _, pid := range pids { - process, err := GetProcess(pid, "") + process, err := newProcess(pid) if err != nil { logp.Debug("topbeat", "Skip process pid=%d: %v", pid, err) continue } + err = process.getDetails("") + if err != nil { + logp.Err("Error getting process details pid=%d: %v", pid, err) + continue + } t.procsMap[process.Pid] = process } + + return nil } func (t *Topbeat) exportProcStats() error { @@ -199,7 +218,7 @@ func (t *Topbeat) exportProcStats() error { cmdline = previousProc.CmdLine } - process, err := GetProcess(pid, cmdline) + process, err := newProcess(pid) if err != nil { logp.Debug("topbeat", "Skip process pid=%d: %v", pid, err) continue @@ -207,6 +226,12 @@ func (t *Topbeat) exportProcStats() error { if t.MatchProcess(process.Name) { + err = process.getDetails(cmdline) + if err != nil { + logp.Err("Error getting process details. pid=%d: %v", process.Pid, err) + continue + } + t.addProcCpuPercentage(process) t.addProcMemPercentage(process, 0 /*read total mem usage */) @@ -334,9 +359,8 @@ func collectFileSystemStats(fss []sigar.FileSystem) []common.MapStr { func (t *Topbeat) MatchProcess(name string) bool { - for _, reg := range t.procs { - matched, _ := regexp.MatchString(reg, name) - if matched { + for _, reg := range t.regexps { + if reg.MatchString(name) { return true } } diff --git a/topbeat/beat/topbeat_test.go b/topbeat/beat/topbeat_test.go index 3172de7a6b66..7d4ce2e2ea32 100644 --- a/topbeat/beat/topbeat_test.go +++ b/topbeat/beat/topbeat_test.go @@ -12,13 +12,16 @@ func TestMatchProcs(t *testing.T) { var beat = Topbeat{} beat.procs = []string{".*"} + beat.initProcStats() assert.True(t, beat.MatchProcess("topbeat")) beat.procs = []string{"topbeat"} + beat.initProcStats() assert.False(t, beat.MatchProcess("burn")) // match no processes beat.procs = []string{"$^"} + beat.initProcStats() assert.False(t, beat.MatchProcess("burn")) }