libct/system.Stat: improve and speed up

Rewrite parseStat() to make it faster:

 - do not use fmt.Scanf as it is very slow;
 - avoid splitting data into 20+ fields, of which we only need 2;
 - use LastIndexByte instead of LastIndex.

This results in about 7x speedup, without much decrease in readability.

Before:

> BenchmarkParseStat-4            129914              7950 ns/op

After:

> BenchmarkParseStat-4   	 1207336	      1166 ns/op

While at it, do not ignore any possible errors, and properly wrap those.

[v2: use pkg/errors more, remove t.Logf from test]

Signed-off-by: Kir Kolyshkin <[email protected]>

libcontainer/system/proc.go

@@ -3,9 +3,12 @@ package system
 import (
 	"fmt"
 	"io/ioutil"
+	"math/bits"
 	"path/filepath"
 	"strconv"
 	"strings"
+
+	"github.com/pkg/errors"
 )
 
 // State is the status of a process.
@@ -75,29 +78,42 @@ func parseStat(data string) (stat Stat_t, err error) {
 	// From proc(5), field 2 could contain space and is inside `(` and `)`.
 	// The following is an example:
 	// 89653 (gunicorn: maste) S 89630 89653 89653 0 -1 4194560 29689 28896 0 3 146 32 76 19 20 0 1 0 2971844 52965376 3920 18446744073709551615 1 1 0 0 0 0 0 16781312 137447943 0 0 0 17 1 0 0 0 0 0 0 0 0 0 0 0 0 0
-	i := strings.LastIndex(data, ")")
+	i := strings.LastIndexByte(data, ')')
 	if i <= 2 || i >= len(data)-1 {
-		return stat, fmt.Errorf("invalid stat data: %q", data)
+		return stat, errors.Errorf("invalid stat data (no comm): %q", data)
 	}
 
-	parts := strings.SplitN(data[:i], "(", 2)
+	parts := strings.SplitN(data[:i], " (", 2)
 	if len(parts) != 2 {
-		return stat, fmt.Errorf("invalid stat data: %q", data)
+		return stat, errors.Errorf("invalid stat data (no comm): %q", data)
 	}
 
 	stat.Name = parts[1]
-	_, err = fmt.Sscanf(parts[0], "%d", &stat.PID)
+	pid, err := strconv.ParseUint(parts[0], 10, bits.UintSize)
 	if err != nil {
-		return stat, err
+		return stat, errors.Wrap(err, "invalid stat data (bad pid)")
+	}
+	stat.PID = uint(pid)
+
+	// Remove the first two fields and a space after.
+	data = data[i+2:]
+	stat.State = State(data[0])
+
+	// StartTime is field 22, data is at field 3 now, so we need to skip 19 spaces.
+	skipSpaces := 22 - 3
+	for i = 0; skipSpaces > 0 && i < len(data); i++ {
+		if data[i] == ' ' {
+			skipSpaces--
+		}
+	}
+	j := strings.IndexByte(data[i:], ' ')
+	if j <= 0 {
+		return stat, errors.Errorf("invalid stat data (too short): %q", data)
+	}
+	stat.StartTime, err = strconv.ParseUint(data[i:i+j], 10, 64)
+	if err != nil {
+		return stat, errors.Wrap(err, "invalid stat data (bad start time)")
 	}
 
-	// parts indexes should be offset by 3 from the field number given
-	// proc(5), because parts is zero-indexed and we've removed fields
-	// one (PID) and two (Name) in the paren-split.
-	parts = strings.Split(data[i+2:], " ")
-	var state int
-	fmt.Sscanf(parts[3-3], "%c", &state) //nolint:staticcheck // "3-3" is more readable in this context.
-	stat.State = State(state)
-	fmt.Sscanf(parts[22-3], "%d", &stat.StartTime)
 	return stat, nil
 }

libcontainer/system/proc_test.go

@@ -32,7 +32,7 @@ var procdata = map[string]Stat_t{
 	},
 }
 
-func TestParseStartTime(t *testing.T) {
+func TestParseStat(t *testing.T) {
 	for line, exp := range procdata {
 		st, err := parseStat(line)
 		if err != nil {
@@ -43,6 +43,35 @@ func TestParseStartTime(t *testing.T) {
 	}
 }
 
+func TestParseStatBadInput(t *testing.T) {
+	cases := []struct {
+		desc, input string
+	}{
+		{"no (",
+			"123 ) S 2 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0"},
+		{"no )",
+			"123 ( S 2 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0"},
+		{"negative pid",
+			"-1 (cmd) S 2 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0"},
+		{"empty line",
+			""},
+		{"short line",
+			"123 (cmd) S 2 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0"},
+		{"short line (no space after stime)",
+			"123 (cmd) S 2 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 42"},
+		{"bad stime",
+			"123 (cmd) S 2 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 0 -1 "},
+		{"bad stime 2", // would be valid if not -1
+			"123 (cmd) S                   -1 "},
+	}
+	for _, c := range cases {
+		_, err := parseStat(c.input)
+		if err == nil {
+			t.Errorf("case %q, expected error, got nil", c.desc)
+		}
+	}
+}
+
 func BenchmarkParseStat(b *testing.B) {
 	var (
 		st, exp Stat_t