Repository: elastic/go-sysinfo
Pull request #186 (2023-07-26 10:10:01)

linux: optimize parseKeyValue
As seen in this `alloc_objects` profile from auditbeat `linux.parseKeyValue()` accounts for a significant amount of the program allocations:
![profile003](https://github.com/elastic/go-sysinfo/assets/1712219/91ff72fd-936a-47b1-9d1c-f7035c24ee08)

This PR adds tests on the current implem to validate the expected behavior and proposes a new implementation for this function that removes _all_ allocs. As a nice side effect, it also cuts down cpu time by ~90%:

```
goos: linux
goarch: amd64
pkg: github.com/elastic/go-sysinfo/providers/linux
cpu: Intel(R) Core(TM) i7-10850H CPU @ 2.70GHz
                 │   old.bench   │              new.bench              │
                 │    sec/op     │   sec/op     vs base                │
ParseKeyValue-12   9169.0n ± 13%   984.8n ± 4%  -89.26% (p=0.000 n=10)

                 │  old.bench   │               new.bench               │
                 │     B/op     │     B/op      vs base                 │
ParseKeyValue-12   6.672Ki ± 0%   0.000Ki ± 0%  -100.00% (p=0.000 n=10)

                 │ old.bench  │             new.bench              │
                 │ allocs/op  │ allocs/op  vs base                 │
ParseKeyValue-12   58.00 ± 0%   0.00 ± 0%  -100.00% (p=0.000 n=10)
```

elasticmachine: 
## :grey_exclamation: Build Aborted
  
  > The PR is not allowed to run in the CI yet
  


<!-- BUILD BADGES-->
> _the below badges are clickable and redirect to their specific view in the CI or DOCS_
[![Pipeline View](https://img.shields.io/badge/pipeline-pipeline%20-green)](https://beats-ci.elastic.co/blue/organizations/jenkins/Library%2Fgo-sysinfo-mbp%2FPR-186/detail/PR-186/1//pipeline) [![Test View](https://img.shields.io/badge/test-test-green)](https://beats-ci.elastic.co/blue/organizations/jenkins/Library%2Fgo-sysinfo-mbp%2FPR-186/detail/PR-186/1//tests) [![Changes](https://img.shields.io/badge/changes-changes-green)](https://beats-ci.elastic.co/blue/organizations/jenkins/Library%2Fgo-sysinfo-mbp%2FPR-186/detail/PR-186/1//changes) [![Artifacts](https://img.shields.io/badge/artifacts-artifacts-yellow)](https://beats-ci.elastic.co/blue/organizations/jenkins/Library%2Fgo-sysinfo-mbp%2FPR-186/detail/PR-186/1//artifacts)   [![preview](https://img.shields.io/badge/elastic-observability-blue)](https://ci-stats.elastic.co/app/apm/services/beats-ci/transactions/view?rangeFrom=2023-07-26T10:00:17.350Z&rangeTo=2023-07-26T10:20:17.350Z&transactionName=BUILD+Library%2Fgo-sysinfo-mbp%2FPR-%7Bnumber%7D&transactionType=job&latencyAggregationType=avg&traceId=bab0a08c59d2fa3a42ecdf03e1a535f7&transactionId=350674be2dda9c26)

<!-- BUILD SUMMARY-->
<details><summary>Expand to view the summary</summary>
<p>

#### Build stats




* Start Time: 2023-07-26T10:10:17.350+0000


* Duration: 6 min 38 sec





</p>
</details>

<!-- TEST RESULTS IF ANY-->


<!-- STEPS ERRORS IF ANY -->














### Steps errors [![2](https://img.shields.io/badge/2%20-red)](https://beats-ci.elastic.co/blue/organizations/jenkins/Library%2Fgo-sysinfo-mbp%2FPR-186/detail/PR-186/1//pipeline)
  <details><summary>Expand to view the steps failures</summary>
  <p>


  
  
  
  
##### `Load a resource file from a library`
<ul>
<li>Took 0 min 0 sec . View more details <a href="https://beats-ci.elastic.co//blue/rest/organizations/jenkins/pipelines/Library/pipelines/go-sysinfo-mbp/pipelines/PR-186/runs/1/steps/81/log/?start=0">here</a></li>
<li>Description: <code>approval-list/elastic/go-sysinfo.yml</code></l1>
</ul>
  
  
  
  
##### `Error signal`
<ul>
<li>Took 0 min 0 sec . View more details <a href="https://beats-ci.elastic.co//blue/rest/organizations/jenkins/pipelines/Library/pipelines/go-sysinfo-mbp/pipelines/PR-186/runs/1/steps/89/log/?start=0">here</a></li>
<li>Description: <code>githubApiCall: The REST API call https://api.github.com/orgs/elastic/members/ShimmerGlass return the message : java.lang.Exception: httpRequest: Failure connecting to the service https://api.github.com/orgs/elastic/members/ShimmerGlass : httpRequest: Failure connecting to the service https://api.github.com/orgs/elastic/members/ShimmerGlass : Code: 404Error: {"message":"User does not exist or is not a member of the organization","documentation_url":"https://docs.github.com/rest/orgs/members#check-organization-membership-for-a-user"}</code></l1>
</ul>
  
  </p>
  </details>




</p>
</details>

<!--COMMENT_GENERATED_WITH_ID_comment.id-->
andrewkroh: The CLA check is failing because the author email in the commit does not match the email used to sign the CLA.
andrewkroh: A changelog entry will also be required. See https://github.com/elastic/go-sysinfo/blob/main/CONTRIBUTING.md.
ShimmerGlass: @andrewkroh signed the CLA with the correct email, also provided a changelog file. lmk if the decription of the change is ok :)
andrewkroh: /test
andrewkroh: CI is failing on a missing license header. If you run `make update` it should add the header.
andrewkroh: Tests are still failing:

```
=== Failed
=== FAIL: providers/linux TestVmStatParse (0.00s)
    vmstat_test.go:165: unterminated input
```
ShimmerGlass: fixed
andrewkroh: I compared the benchmark results from e4ac65ca824977 to f3a5024f2009. Looks good.

```
benchstat before.txt after.txt 
goos: darwin
goarch: arm64
pkg: github.com/elastic/go-sysinfo/providers/linux
                 │  before.txt   │              after.txt              │
                 │    sec/op     │    sec/op     vs base               │
ParseKeyValue-10   3540.0n ± ∞ ¹   972.6n ± ∞ ¹  -72.53% (p=0.008 n=5)
¹ need >= 6 samples for confidence interval at level 0.95

                 │  before.txt   │               after.txt               │
                 │     B/op      │     B/op       vs base                │
ParseKeyValue-10   6.672Ki ± ∞ ¹   0.000Ki ± ∞ ¹  -100.00% (p=0.008 n=5)
¹ need >= 6 samples for confidence interval at level 0.95

                 │ before.txt  │             after.txt              │
                 │  allocs/op  │ allocs/op   vs base                │
ParseKeyValue-10   58.00 ± ∞ ¹   0.00 ± ∞ ¹  -100.00% (p=0.008 n=5)
¹ need >= 6 samples for confidence interval at level 0.95
```

Patch:
diff --git a/.changelog/186.txt b/.changelog/186.txt
new file mode 100644
index 0000000..6f823f2
--- /dev/null
+++ b/.changelog/186.txt
@@ -0,0 +1,3 @@
+```release-note:enhancement
+linux: optimize linux key value parsing (ie: /proc files)
+```
diff --git a/providers/linux/capabilities_linux.go b/providers/linux/capabilities_linux.go
index b1af31d..40bf454 100644
--- a/providers/linux/capabilities_linux.go
+++ b/providers/linux/capabilities_linux.go
@@ -86,7 +86,7 @@ func capabilityName(num int) string {
 func readCapabilities(content []byte) (*types.CapabilityInfo, error) {
 	var cap types.CapabilityInfo
 
-	err := parseKeyValue(content, ":", func(key, value []byte) error {
+	err := parseKeyValue(content, ':', func(key, value []byte) error {
 		var err error
 		switch string(key) {
 		case "CapInh":
diff --git a/providers/linux/memory_linux.go b/providers/linux/memory_linux.go
index c04bad0..c0c5ab8 100644
--- a/providers/linux/memory_linux.go
+++ b/providers/linux/memory_linux.go
@@ -29,7 +29,7 @@ func parseMemInfo(content []byte) (*types.HostMemoryInfo, error) {
 	}
 
 	hasAvailable := false
-	err := parseKeyValue(content, ":", func(key, value []byte) error {
+	err := parseKeyValue(content, ':', func(key, value []byte) error {
 		num, err := parseBytesOrNumber(value)
 		if err != nil {
 			return fmt.Errorf("failed to parse %v value of %v: %w", string(key), string(value), err)
diff --git a/providers/linux/process_linux.go b/providers/linux/process_linux.go
index 10cb947..52bae25 100644
--- a/providers/linux/process_linux.go
+++ b/providers/linux/process_linux.go
@@ -229,7 +229,7 @@ func (p *process) User() (types.UserInfo, error) {
 	}
 
 	var user types.UserInfo
-	err = parseKeyValue(content, ":", func(key, value []byte) error {
+	err = parseKeyValue(content, ':', func(key, value []byte) error {
 		// See proc(5) for the format of /proc/[pid]/status
 		switch string(key) {
 		case "Uid":
diff --git a/providers/linux/seccomp_linux.go b/providers/linux/seccomp_linux.go
index d04bb3c..fd38ea4 100644
--- a/providers/linux/seccomp_linux.go
+++ b/providers/linux/seccomp_linux.go
@@ -47,7 +47,7 @@ func (m SeccompMode) String() string {
 func readSeccompFields(content []byte) (*types.SeccompInfo, error) {
 	var seccomp types.SeccompInfo
 
-	err := parseKeyValue(content, ":", func(key, value []byte) error {
+	err := parseKeyValue(content, ':', func(key, value []byte) error {
 		switch string(key) {
 		case "Seccomp":
 			mode, err := strconv.ParseUint(string(value), 10, 8)
diff --git a/providers/linux/util.go b/providers/linux/util.go
index b8705a1..8d9c27d 100644
--- a/providers/linux/util.go
+++ b/providers/linux/util.go
@@ -26,20 +26,28 @@ import (
 	"strconv"
 )
 
-func parseKeyValue(content []byte, separator string, callback func(key, value []byte) error) error {
-	sc := bufio.NewScanner(bytes.NewReader(content))
-	for sc.Scan() {
-		parts := bytes.SplitN(sc.Bytes(), []byte(separator), 2)
-		if len(parts) != 2 {
+// parseKeyValue parses key/val pairs separated by the provided separator from
+// each line in content and invokes the callback. White-space is trimmed from
+// val. Empty lines are ignored. All non-empty lines must contain the separator
+// otherwise an error is returned.
+func parseKeyValue(content []byte, separator byte, callback func(key, value []byte) error) error {
+	var line []byte
+
+	for len(content) > 0 {
+		line, content, _ = bytes.Cut(content, []byte{'\n'})
+		if len(line) == 0 {
 			continue
 		}
 
-		if err := callback(parts[0], bytes.TrimSpace(parts[1])); err != nil {
-			return err
+		key, value, ok := bytes.Cut(line, []byte{separator})
+		if !ok {
+			return fmt.Errorf("separator %q not found", separator)
 		}
+
+		callback(key, bytes.TrimSpace(value))
 	}
 
-	return sc.Err()
+	return nil
 }
 
 func findValue(filename, separator, key string) (string, error) {
diff --git a/providers/linux/vmstat.go b/providers/linux/vmstat.go
index 2b9e878..ea918c8 100644
--- a/providers/linux/vmstat.go
+++ b/providers/linux/vmstat.go
@@ -45,7 +45,7 @@ func parseVMStat(content []byte) (*types.VMStatInfo, error) {
 	var vmStat types.VMStatInfo
 	refValues := reflect.ValueOf(&vmStat).Elem()
 
-	err := parseKeyValue(content, " ", func(key, value []byte) error {
+	err := parseKeyValue(content, ' ', func(key, value []byte) error {
 		// turn our []byte value into an int
 		val, err := parseBytesOrNumber(value)
 		if err != nil {


Test patch:
diff --git a/providers/linux/util_test.go b/providers/linux/util_test.go
new file mode 100644
index 0000000..388ec8b
--- /dev/null
+++ b/providers/linux/util_test.go
@@ -0,0 +1,162 @@
+// Licensed to Elasticsearch B.V. under one or more contributor
+// license agreements. See the NOTICE file distributed with
+// this work for additional information regarding copyright
+// ownership. Elasticsearch B.V. licenses this file to you under
+// the Apache License, Version 2.0 (the "License"); you may
+// not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+//     http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied.  See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+package linux
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestParseKeyValueNoEOL(t *testing.T) {
+	vals := [][2]string{}
+	err := parseKeyValue([]byte(
+		"Name:	zsh\nUmask:	0022\nState:	S (sleeping)\nUid:	1000	1000	1000	1000",
+	), ':', func(key, value []byte) error {
+		vals = append(vals, [2]string{string(key), string(value)})
+		return nil
+	})
+	assert.NoError(t, err)
+
+	assert.Equal(t, [][2]string{
+		{"Name", "zsh"},
+		{"Umask", "0022"},
+		{"State", "S (sleeping)"},
+		{"Uid", "1000\t1000\t1000\t1000"},
+	}, vals)
+}
+
+func TestParseKeyValueEmptyLine(t *testing.T) {
+	vals := [][2]string{}
+	err := parseKeyValue([]byte(
+		"Name:	zsh\nUmask:	0022\nState:	S (sleeping)\n\nUid:	1000	1000	1000	1000",
+	), ':', func(key, value []byte) error {
+		vals = append(vals, [2]string{string(key), string(value)})
+		return nil
+	})
+	assert.NoError(t, err)
+
+	assert.Equal(t, [][2]string{
+		{"Name", "zsh"},
+		{"Umask", "0022"},
+		{"State", "S (sleeping)"},
+		{"Uid", "1000\t1000\t1000\t1000"},
+	}, vals)
+}
+
+func TestParseKeyValueEOL(t *testing.T) {
+	vals := [][2]string{}
+	err := parseKeyValue([]byte(
+		"Name:	zsh\nUmask:	0022\nState:	S (sleeping)\nUid:	1000	1000	1000	1000\n",
+	), ':', func(key, value []byte) error {
+		vals = append(vals, [2]string{string(key), string(value)})
+		return nil
+	})
+	assert.NoError(t, err)
+
+	assert.Equal(t, [][2]string{
+		{"Name", "zsh"},
+		{"Umask", "0022"},
+		{"State", "S (sleeping)"},
+		{"Uid", "1000\t1000\t1000\t1000"},
+	}, vals)
+}
+
+// from cat /proc/$$/status
+var testProcStatus = []byte(`Name:	zsh
+Umask:	0022
+State:	S (sleeping)
+Tgid:	4023363
+Ngid:	0
+Pid:	4023363
+PPid:	4023357
+TracerPid:	0
+Uid:	1000	1000	1000	1000
+Gid:	1000	1000	1000	1000
+FDSize:	64
+Groups:	24 25 27 29 30 44 46 102 109 112 116 119 131 998 1000
+NStgid:	4023363
+NSpid:	4023363
+NSpgid:	4023363
+NSsid:	4023363
+VmPeak:	   15596 kB
+VmSize:	   15144 kB
+VmLck:	       0 kB
+VmPin:	       0 kB
+VmHWM:	    9060 kB
+VmRSS:	    8716 kB
+RssAnon:	    3828 kB
+RssFile:	    4888 kB
+RssShmem:	       0 kB
+VmData:	    3500 kB
+VmStk:	     328 kB
+VmExe:	     600 kB
+VmLib:	    2676 kB
+VmPTE:	      68 kB
+VmSwap:	       0 kB
+HugetlbPages:	       0 kB
+CoreDumping:	0
+THP_enabled:	1
+Threads:	1
+SigQ:	0/126683
+SigPnd:	0000000000000000
+ShdPnd:	0000000000000000
+SigBlk:	0000000000000002
+SigIgn:	0000000000384000
+SigCgt:	0000000008013003
+CapInh:	0000000000000000
+CapPrm:	0000000000000000
+CapEff:	0000000000000000
+CapBnd:	000001ffffffffff
+CapAmb:	0000000000000000
+NoNewPrivs:	0
+Seccomp:	0
+Seccomp_filters:	0
+Speculation_Store_Bypass:	thread vulnerable
+Cpus_allowed:	fff
+Cpus_allowed_list:	0-11
+Mems_allowed:	00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000000,00000001
+Mems_allowed_list:	0
+voluntary_ctxt_switches:	223
+nonvoluntary_ctxt_switches:	25
+`)
+
+func BenchmarkParseKeyValue(b *testing.B) {
+	for i := 0; i < b.N; i++ {
+		_ = parseKeyValue(testProcStatus, ':', func(key, value []byte) error {
+			return nil
+		})
+	}
+}
+
+func FuzzParseKeyValue(f *testing.F) {
+	testcases := []string{
+		"no_separator",
+		"no_value:",
+		"empty_value: ",
+		"normal:	223",
+	}
+	for _, tc := range testcases {
+		f.Add(tc)
+	}
+	f.Fuzz(func(t *testing.T, orig string) {
+		_ = parseKeyValue([]byte(orig), ':', func(key, value []byte) error {
+			return nil
+		})
+	})
+}