From bdb09f5f9d54cd697ef03a264e3832309f594e00 Mon Sep 17 00:00:00 2001 From: Alban Crequy Date: Mon, 24 Apr 2017 16:59:17 +0200 Subject: [PATCH 1/3] proc walker: optimize readStats --- probe/process/walker_linux.go | 83 ++++++++++++++++++++++++----------- 1 file changed, 57 insertions(+), 26 deletions(-) diff --git a/probe/process/walker_linux.go b/probe/process/walker_linux.go index a748d2bee..a28ea9e66 100644 --- a/probe/process/walker_linux.go +++ b/probe/process/walker_linux.go @@ -23,7 +23,42 @@ func NewWalker(procRoot string) Walker { return &walker{procRoot: procRoot} } +// skipNSpaces skips nSpaces in buf and updates the cursor 'pos' +func skipNSpaces(buf *[]byte, pos *int, nSpaces int) { + for spaceCount := 0; *pos < len(*buf) && spaceCount < nSpaces; *pos++ { + if (*buf)[*pos] == ' ' { + spaceCount++ + } + } +} + +// parseUint64WithSpaces is similar to strconv.ParseUint64 but stops parsing +// when reading a space instead of returning an error +func parseUint64WithSpaces(buf *[]byte, pos *int) (ret uint64) { + for ; *pos < len(*buf) && (*buf)[*pos] != ' '; *pos++ { + ret = ret*10 + uint64((*buf)[*pos]-'0') + } + return +} + +// parseIntWithSpaces is similar to strconv.ParseInt but stops parsing when +// reading a space instead of returning an error +func parseIntWithSpaces(buf *[]byte, pos *int) (ret int) { + return int(parseUint64WithSpaces(buf, pos)) +} + +// readStats reads and parses '/proc//stat' files func readStats(path string) (ppid, threads int, jiffies, rss, rssLimit uint64, err error) { + const ( + // /proc//stat field positions, counting from zero + // see "man 5 proc" + procStatFieldPpid int = 3 + procStatFieldUserJiffies int = 13 + procStatFieldSysJiffies int = 14 + procStatFieldThreads int = 19 + procStatFieldRssPages int = 23 + procStatFieldRssLimit int = 24 + ) var ( buf []byte userJiffies, sysJiffies, rssPages uint64 @@ -32,34 +67,30 @@ func readStats(path string) (ppid, threads int, jiffies, rss, rssLimit uint64, e if err != nil { return } - splits := strings.Fields(string(buf)) - if len(splits) < 25 { - err = fmt.Errorf("Invalid /proc/PID/stat") - return - } - ppid, err = strconv.Atoi(splits[3]) - if err != nil { - return - } - threads, err = strconv.Atoi(splits[19]) - if err != nil { - return - } - userJiffies, err = strconv.ParseUint(splits[13], 10, 64) - if err != nil { - return - } - sysJiffies, err = strconv.ParseUint(splits[14], 10, 64) - if err != nil { - return - } + + // Parse the file without using expensive extra string allocations + + pos := 0 + skipNSpaces(&buf, &pos, procStatFieldPpid) + ppid = parseIntWithSpaces(&buf, &pos) + + skipNSpaces(&buf, &pos, procStatFieldUserJiffies-procStatFieldPpid) + userJiffies = parseUint64WithSpaces(&buf, &pos) + + pos++ // 1 space between userJiffies and sysJiffies + sysJiffies = parseUint64WithSpaces(&buf, &pos) + + skipNSpaces(&buf, &pos, procStatFieldThreads-procStatFieldSysJiffies) + threads = parseIntWithSpaces(&buf, &pos) + + skipNSpaces(&buf, &pos, procStatFieldRssPages-procStatFieldThreads) + rssPages = parseUint64WithSpaces(&buf, &pos) + + pos++ // 1 space between rssPages and rssLimit + rssLimit = parseUint64WithSpaces(&buf, &pos) + jiffies = userJiffies + sysJiffies - rssPages, err = strconv.ParseUint(splits[23], 10, 64) - if err != nil { - return - } rss = rssPages * uint64(os.Getpagesize()) - rssLimit, err = strconv.ParseUint(splits[24], 10, 64) return } From 3a8a09a60675db42dba05830f8e14f4e88c3887e Mon Sep 17 00:00:00 2001 From: Alban Crequy Date: Tue, 25 Apr 2017 12:05:29 +0200 Subject: [PATCH 2/3] proc walker: optimize readLimits --- probe/process/walker_linux.go | 35 ++++++++++++++++++++---------- probe/process/walker_linux_test.go | 2 +- 2 files changed, 25 insertions(+), 12 deletions(-) diff --git a/probe/process/walker_linux.go b/probe/process/walker_linux.go index a28ea9e66..4e9e2a9aa 100644 --- a/probe/process/walker_linux.go +++ b/probe/process/walker_linux.go @@ -2,7 +2,6 @@ package process import ( "bytes" - "fmt" "os" "path" "strconv" @@ -99,17 +98,31 @@ func readLimits(path string) (openFilesLimit uint64, err error) { if err != nil { return 0, err } - for _, line := range strings.Split(string(buf), "\n") { - if strings.HasPrefix(line, "Max open files") { - splits := strings.Fields(line) - if len(splits) < 6 { - return 0, fmt.Errorf("Invalid /proc/PID/limits") - } - openFilesLimit, err := strconv.Atoi(splits[3]) - return uint64(openFilesLimit), err - } + content := string(buf) + + // File format: one line header + one line per limit + // + // Limit Soft Limit Hard Limit Units + // ... + // Max open files 1024 4096 files + // ... + delim := "\nMax open files" + pos := strings.Index(content, delim) + + if pos < 0 { + // Tests such as TestWalker can synthetise empty files + return 0, nil } - return 0, nil + pos += len(delim) + + for pos < len(content) && content[pos] == ' ' { + pos++ + } + + var softLimit uint64 + softLimit = parseUint64WithSpaces(&buf, &pos) + + return softLimit, nil } // Walk walks the supplied directory (expecting it to look like /proc) diff --git a/probe/process/walker_linux_test.go b/probe/process/walker_linux_test.go index 68912aed0..1da50deb7 100644 --- a/probe/process/walker_linux_test.go +++ b/probe/process/walker_linux_test.go @@ -23,7 +23,7 @@ var mockFS = fs.Dir("", }, fs.File{ FName: "limits", - FContents: `Max open files 32768 65536 files`, + FContents: "Limit Soft-Limit Hard-Limit Units\nMax open files 32768 65536 files", }, fs.Dir("fd", fs.File{FName: "0"}, fs.File{FName: "1"}, fs.File{FName: "2"}), ), From 598c6a0238137edbc725bb388f0753ae897b938f Mon Sep 17 00:00:00 2001 From: Alban Crequy Date: Tue, 25 Apr 2017 15:11:39 +0200 Subject: [PATCH 3/3] proc walker: cache limits and cmdline/name after parsing --- probe/process/cache.go | 41 ------------------ probe/process/walker_linux.go | 81 ++++++++++++++++++++++++++--------- 2 files changed, 61 insertions(+), 61 deletions(-) delete mode 100644 probe/process/cache.go diff --git a/probe/process/cache.go b/probe/process/cache.go deleted file mode 100644 index 3d076eb9b..000000000 --- a/probe/process/cache.go +++ /dev/null @@ -1,41 +0,0 @@ -package process - -import ( - "time" - - "github.com/armon/go-metrics" - "github.com/coocood/freecache" - - "github.com/weaveworks/common/fs" -) - -const ( - generalTimeout = 30 // seconds - statsTimeout = 10 //seconds -) - -var ( - hitMetricsKey = []string{"process", "cache", "hit"} - missMetricsKey = []string{"process", "cache", "miss"} -) - -var fileCache = freecache.NewCache(1024 * 16) - -type entry struct { - buf []byte - err error - ts time.Time -} - -func cachedReadFile(path string) ([]byte, error) { - key := []byte(path) - if v, err := fileCache.Get(key); err == nil { - metrics.IncrCounter(hitMetricsKey, 1.0) - return v, nil - } - - buf, err := fs.ReadFile(path) - fileCache.Set(key, buf, generalTimeout) - metrics.IncrCounter(missMetricsKey, 1.0) - return buf, err -} diff --git a/probe/process/walker_linux.go b/probe/process/walker_linux.go index 4e9e2a9aa..2191044e3 100644 --- a/probe/process/walker_linux.go +++ b/probe/process/walker_linux.go @@ -2,12 +2,15 @@ package process import ( "bytes" + "encoding/binary" + "fmt" "os" "path" "strconv" "strings" linuxproc "github.com/c9s/goprocinfo/linux" + "github.com/coocood/freecache" "github.com/weaveworks/common/fs" "github.com/weaveworks/scope/probe/host" @@ -17,6 +20,23 @@ type walker struct { procRoot string } +var ( + // limitsCache caches /proc//limits + // key: filename in /proc. Example: "42" + // value: max open files (soft limit) stored in a [8]byte (uint64, little endian) + limitsCache = freecache.NewCache(1024 * 16) + + // cmdlineCache caches /proc//cmdline and /proc//name + // key: filename in /proc. Example: "42" + // value: two strings separated by a '\0' + cmdlineCache = freecache.NewCache(1024 * 16) +) + +const ( + limitsCacheTimeout = 60 + cmdlineCacheTimeout = 60 +) + // NewWalker creates a new process Walker. func NewWalker(procRoot string) Walker { return &walker{procRoot: procRoot} @@ -94,7 +114,7 @@ func readStats(path string) (ppid, threads int, jiffies, rss, rssLimit uint64, e } func readLimits(path string) (openFilesLimit uint64, err error) { - buf, err := cachedReadFile(path) + buf, err := fs.ReadFile(path) if err != nil { return 0, err } @@ -125,6 +145,27 @@ func readLimits(path string) (openFilesLimit uint64, err error) { return softLimit, nil } +func (w *walker) readCmdline(filename string) (cmdline, name string) { + if cmdlineBuf, err := fs.ReadFile(path.Join(w.procRoot, filename, "cmdline")); err == nil { + // like proc, treat name as the first element of command line + i := bytes.IndexByte(cmdlineBuf, '\000') + if i == -1 { + i = len(cmdlineBuf) + } + name = string(cmdlineBuf[:i]) + cmdlineBuf = bytes.Replace(cmdlineBuf, []byte{'\000'}, []byte{' '}, -1) + cmdline = string(cmdlineBuf) + } + if name == "" { + if commBuf, err := fs.ReadFile(path.Join(w.procRoot, filename, "comm")); err == nil { + name = "[" + strings.TrimSpace(string(commBuf)) + "]" + } else { + name = "(unknown)" + } + } + return +} + // Walk walks the supplied directory (expecting it to look like /proc) // and marshalls the files into instances of Process, which it then // passes one-by-one to the supplied function. Walk is only made public @@ -151,29 +192,29 @@ func (w *walker) Walk(f func(Process, Process)) error { continue } - openFilesLimit, err := readLimits(path.Join(w.procRoot, filename, "limits")) - if err != nil { - continue + var openFilesLimit uint64 + if v, err := limitsCache.Get([]byte(filename)); err == nil { + openFilesLimit = binary.LittleEndian.Uint64(v) + } else { + openFilesLimit, err = readLimits(path.Join(w.procRoot, filename, "limits")) + if err != nil { + continue + } + buf := make([]byte, 8) + binary.LittleEndian.PutUint64(buf, openFilesLimit) + limitsCache.Set([]byte(filename), buf, limitsCacheTimeout) } cmdline, name := "", "" - if cmdlineBuf, err := cachedReadFile(path.Join(w.procRoot, filename, "cmdline")); err == nil { - // like proc, treat name as the first element of command line - i := bytes.IndexByte(cmdlineBuf, '\000') - if i == -1 { - i = len(cmdlineBuf) - } - name = string(cmdlineBuf[:i]) - cmdlineBuf = bytes.Replace(cmdlineBuf, []byte{'\000'}, []byte{' '}, -1) - cmdline = string(cmdlineBuf) - } - if name == "" { - if commBuf, err := cachedReadFile(path.Join(w.procRoot, filename, "comm")); err == nil { - name = "[" + strings.TrimSpace(string(commBuf)) + "]" - } else { - name = "(unknown)" - } + if v, err := cmdlineCache.Get([]byte(filename)); err == nil { + separatorPos := strings.Index(string(v), "\x00") + cmdline = string(v[:separatorPos]) + name = string(v[separatorPos+1:]) + } else { + cmdline, name = w.readCmdline(filename) + cmdlineCache.Set([]byte(filename), []byte(fmt.Sprintf("%s\x00%s", cmdline, name)), cmdlineCacheTimeout) } + f(Process{ PID: pid, PPID: ppid,