From 9ea0f5b0c417e7ead95935700ec5ce3ba826681f Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Sun, 21 Jul 2024 12:45:50 +0200 Subject: [PATCH 1/2] mountinfo: add benchmark for unescape go test -v -test.benchmem -count=10 -run ^$ -bench BenchmarkUnescape . go: downloading golang.org/x/sys v0.1.0 goos: linux goarch: arm64 pkg: github.com/moby/sys/mountinfo BenchmarkUnescape BenchmarkUnescape-10 1000000 1068 ns/op 640 B/op 31 allocs/op BenchmarkUnescape-10 992292 1082 ns/op 640 B/op 31 allocs/op BenchmarkUnescape-10 1000000 1050 ns/op 640 B/op 31 allocs/op BenchmarkUnescape-10 1000000 1038 ns/op 640 B/op 31 allocs/op BenchmarkUnescape-10 1000000 1034 ns/op 640 B/op 31 allocs/op BenchmarkUnescape-10 1000000 1045 ns/op 640 B/op 31 allocs/op BenchmarkUnescape-10 1000000 1071 ns/op 640 B/op 31 allocs/op BenchmarkUnescape-10 1000000 1033 ns/op 640 B/op 31 allocs/op BenchmarkUnescape-10 1000000 1032 ns/op 640 B/op 31 allocs/op BenchmarkUnescape-10 1000000 1070 ns/op 640 B/op 31 allocs/op PASS ok github.com/moby/sys/mountinfo 10.653s Signed-off-by: Sebastiaan van Stijn --- mountinfo/mountinfo_linux_test.go | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) diff --git a/mountinfo/mountinfo_linux_test.go b/mountinfo/mountinfo_linux_test.go index dca55536..e78ee3f9 100644 --- a/mountinfo/mountinfo_linux_test.go +++ b/mountinfo/mountinfo_linux_test.go @@ -771,3 +771,32 @@ func TestUnescape(t *testing.T) { } } } + +func BenchmarkUnescape(b *testing.B) { + testCases := []string{ + "", + "/", + "/some/longer/path", + "/path\\040with\\040spaces", + "/path/with\\134backslash", + "/tab\\011in/path", + `/path/"with'quotes`, + `/path/"with'quotes,\040space,\011tab`, + `\12`, + `\134`, + `"'"'"'`, + `/\1345`, + `/\12x`, + `\0`, + `\x`, + "\\\\", + } + + b.ResetTimer() + b.ReportAllocs() + for i := 0; i < b.N; i++ { + for x := 0; x < len(testCases); x++ { + _, _ = unescape(testCases[x]) + } + } +} From a437d7395d061b3884e22668f6484294aa515589 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Thu, 17 Sep 2020 19:46:46 +0200 Subject: [PATCH 2/2] mountinfo: implement unescape() using strings.Replacer() So, this mostly was because I saw this approach in another projefct, and liked how simple/clean it was. There are some downsides though; - performance looks to be _slightly_ less - we're no longer bothering with invalid escape sequences For the latter, I'm actually wondering how much validation we should do; should we take invalid escape sequences as literal strings? What does Linux itself do with these? I know in the past we added too much complication at times, and sometimes were validating things we should not care about (or even validating inconsistent with the host itself). Comparison between old `unescape()` and the new, using fstabUnescape.Replace(): Before: go test -v -test.benchmem -count=10 -run ^$ -bench BenchmarkUnescape . go: downloading golang.org/x/sys v0.1.0 goos: linux goarch: arm64 pkg: github.com/moby/sys/mountinfo BenchmarkUnescape BenchmarkUnescape-10 1000000 1068 ns/op 640 B/op 31 allocs/op BenchmarkUnescape-10 992292 1082 ns/op 640 B/op 31 allocs/op BenchmarkUnescape-10 1000000 1050 ns/op 640 B/op 31 allocs/op BenchmarkUnescape-10 1000000 1038 ns/op 640 B/op 31 allocs/op BenchmarkUnescape-10 1000000 1034 ns/op 640 B/op 31 allocs/op BenchmarkUnescape-10 1000000 1045 ns/op 640 B/op 31 allocs/op BenchmarkUnescape-10 1000000 1071 ns/op 640 B/op 31 allocs/op BenchmarkUnescape-10 1000000 1033 ns/op 640 B/op 31 allocs/op BenchmarkUnescape-10 1000000 1032 ns/op 640 B/op 31 allocs/op BenchmarkUnescape-10 1000000 1070 ns/op 640 B/op 31 allocs/op PASS ok github.com/moby/sys/mountinfo 10.653s After go test -v -test.benchmem -count=10 -run ^$ -bench BenchmarkUnescape . goos: linux goarch: arm64 pkg: github.com/moby/sys/mountinfo BenchmarkUnescape BenchmarkUnescape-10 1000000 1141 ns/op 520 B/op 32 allocs/op BenchmarkUnescape-10 977841 1132 ns/op 520 B/op 32 allocs/op BenchmarkUnescape-10 1000000 1160 ns/op 520 B/op 32 allocs/op BenchmarkUnescape-10 901806 1131 ns/op 520 B/op 32 allocs/op BenchmarkUnescape-10 980247 1137 ns/op 520 B/op 32 allocs/op BenchmarkUnescape-10 988596 1135 ns/op 520 B/op 32 allocs/op BenchmarkUnescape-10 975658 1139 ns/op 520 B/op 32 allocs/op BenchmarkUnescape-10 934603 1161 ns/op 520 B/op 32 allocs/op BenchmarkUnescape-10 997353 1123 ns/op 520 B/op 32 allocs/op BenchmarkUnescape-10 986551 1131 ns/op 520 B/op 32 allocs/op PASS ok github.com/moby/sys/mountinfo 11.245s From the above: - new version is ~100 ns/op slower - new version has one more allocation (32 vs 31) - new version uses 120B less memory (520B vs 640B) Signed-off-by: Sebastiaan van Stijn --- mountinfo/mountinfo_linux.go | 86 +++++++++---------------------- mountinfo/mountinfo_linux_test.go | 46 ++++++----------- 2 files changed, 39 insertions(+), 93 deletions(-) diff --git a/mountinfo/mountinfo_linux.go b/mountinfo/mountinfo_linux.go index 43ced2c1..75cf2d63 100644 --- a/mountinfo/mountinfo_linux.go +++ b/mountinfo/mountinfo_linux.go @@ -24,8 +24,6 @@ func GetMountsFromReader(r io.Reader, filter FilterFunc) ([]*Info, error) { s := bufio.NewScanner(r) out := []*Info{} for s.Scan() { - var err error - /* See http://man7.org/linux/man-pages/man5/proc.5.html @@ -85,29 +83,15 @@ func GetMountsFromReader(r io.Reader, filter FilterFunc) ([]*Info, error) { Parent: toInt(fields[1]), Major: toInt(major), Minor: toInt(minor), + Root: unescape(fields[3]), + Mountpoint: unescape(fields[4]), Options: fields[5], Optional: strings.Join(fields[6:sepIdx], " "), // zero or more optional fields + FSType: unescape(fields[sepIdx+1]), + Source: unescape(fields[sepIdx+2]), VFSOptions: fields[sepIdx+3], } - p.Mountpoint, err = unescape(fields[4]) - if err != nil { - return nil, fmt.Errorf("parsing '%s' failed: mount point: %w", fields[4], err) - } - p.FSType, err = unescape(fields[sepIdx+1]) - if err != nil { - return nil, fmt.Errorf("parsing '%s' failed: fstype: %w", fields[sepIdx+1], err) - } - p.Source, err = unescape(fields[sepIdx+2]) - if err != nil { - return nil, fmt.Errorf("parsing '%s' failed: source: %w", fields[sepIdx+2], err) - } - - p.Root, err = unescape(fields[3]) - if err != nil { - return nil, fmt.Errorf("parsing '%s' failed: root: %w", fields[3], err) - } - // Run the filter after parsing all fields. var skip, stop bool if filter != nil { @@ -188,6 +172,22 @@ func PidMountInfo(pid int) ([]*Info, error) { return GetMountsFromReader(f, nil) } +// Linux /proc/mounts shows current mounts. +// Same format as /etc/fstab. Quoting getmntent(3): +// +// Since fields in the mtab and fstab files are separated by whitespace, +// octal escapes are used to represent the four characters space (\040), +// tab (\011), newline (\012) and backslash (\134) in those files when +// they occur in one of the four strings in a mntent structure. +// +// http://linux.die.net/man/3/getmntent +var fstabUnescape = strings.NewReplacer( + `\040`, "\040", + `\011`, "\011", + `\012`, "\012", + `\134`, "\134", +) + // A few specific characters in mountinfo path entries (root and mountpoint) // are escaped using a backslash followed by a character's ascii code in octal. // @@ -197,53 +197,13 @@ func PidMountInfo(pid int) ([]*Info, error) { // backslash (aka \\) -- as \134 // // This function converts path from mountinfo back, i.e. it unescapes the above sequences. -func unescape(path string) (string, error) { +func unescape(path string) string { // try to avoid copying if strings.IndexByte(path, '\\') == -1 { - return path, nil - } - - // The following code is UTF-8 transparent as it only looks for some - // specific characters (backslash and 0..7) with values < utf8.RuneSelf, - // and everything else is passed through as is. - buf := make([]byte, len(path)) - bufLen := 0 - for i := 0; i < len(path); i++ { - if path[i] != '\\' { - buf[bufLen] = path[i] - bufLen++ - continue - } - s := path[i:] - if len(s) < 4 { - // too short - return "", fmt.Errorf("bad escape sequence %q: too short", s) - } - c := s[1] - switch c { - case '0', '1', '2', '3', '4', '5', '6', '7': - v := c - '0' - for j := 2; j < 4; j++ { // one digit already; two more - if s[j] < '0' || s[j] > '7' { - return "", fmt.Errorf("bad escape sequence %q: not a digit", s[:3]) - } - x := s[j] - '0' - v = (v << 3) | x - } - if v > 255 { - return "", fmt.Errorf("bad escape sequence %q: out of range" + s[:3]) - } - buf[bufLen] = v - bufLen++ - i += 3 - continue - default: - return "", fmt.Errorf("bad escape sequence %q: not a digit" + s[:3]) - - } + return path } - return string(buf[:bufLen]), nil + return fstabUnescape.Replace(path) } // toInt converts a string to an int, and ignores any numbers parsing errors, diff --git a/mountinfo/mountinfo_linux_test.go b/mountinfo/mountinfo_linux_test.go index e78ee3f9..07881726 100644 --- a/mountinfo/mountinfo_linux_test.go +++ b/mountinfo/mountinfo_linux_test.go @@ -733,46 +733,32 @@ func TestParseMountinfoExtraCases(t *testing.T) { func TestUnescape(t *testing.T) { testCases := []struct { input, output string - isErr bool }{ - {"", "", false}, - {"/", "/", false}, - {"/some/longer/path", "/some/longer/path", false}, - {"/path\\040with\\040spaces", "/path with spaces", false}, - {"/path/with\\134backslash", "/path/with\\backslash", false}, - {"/tab\\011in/path", "/tab\tin/path", false}, - {`/path/"with'quotes`, `/path/"with'quotes`, false}, - {`/path/"with'quotes,\040space,\011tab`, `/path/"with'quotes, space, tab`, false}, - {`\12`, "", true}, - {`\134`, `\`, false}, - {`"'"'"'`, `"'"'"'`, false}, - {`/\1345`, `/\5`, false}, - {`/\12x`, "", true}, - {`\0`, "", true}, - {`\x`, "", true}, - {"\\\\", "", true}, + {"", ""}, + {"/", "/"}, + {"/some/longer/path", "/some/longer/path"}, + {"/path\\040with\\040spaces", "/path with spaces"}, + {"/path/with\\134backslash", "/path/with\\backslash"}, + {"/tab\\011in/path", "/tab\tin/path"}, + {`/path/"with'quotes`, `/path/"with'quotes`}, + {`/path/"with'quotes,\040space,\011tab`, `/path/"with'quotes, space, tab`}, + {`\134`, `\`}, + {`"'"'"'`, `"'"'"'`}, + {`/\1345`, `/\5`}, } for _, tc := range testCases { - res, err := unescape(tc.input) - if tc.isErr == true { - if err == nil { - t.Errorf("Input %q, want error, got nil", tc.input) - } - // no more checks - continue - } + res := unescape(tc.input) if res != tc.output { t.Errorf("Input %q, want %q, got %q", tc.input, tc.output, res) } - if err != nil { - t.Errorf("Input %q, want nil, got error %v", tc.input, err) - continue - } } } func BenchmarkUnescape(b *testing.B) { + // allow the replacer to be built once: https://github.com/golang/go/blob/go1.16.7/src/strings/replace.go#L96 + _ = unescape("/path\\040with\\040spaces") + testCases := []string{ "", "/", @@ -796,7 +782,7 @@ func BenchmarkUnescape(b *testing.B) { b.ReportAllocs() for i := 0; i < b.N; i++ { for x := 0; x < len(testCases); x++ { - _, _ = unescape(testCases[x]) + _ = unescape(testCases[x]) } } }