From 9682bbfbdacf44e86d788da2065a0094dc70cec9 Mon Sep 17 00:00:00 2001 From: Jakob Borg Date: Sun, 3 Sep 2017 10:26:12 +0000 Subject: [PATCH] lib/protocol: Optimize luhn and chunk functions These functions were very naive and slow. We haven't done much about them because they pretty much don't matter at all for Syncthing performance. They are however called very often in the discovery server and these optimizations have a huge effect on the CPU load on the public discovery servers. The code isn't exactly obvious, but we have good test coverage on all these functions. benchmark old ns/op new ns/op delta BenchmarkLuhnify-8 12458 1045 -91.61% BenchmarkUnluhnify-8 12598 1074 -91.47% BenchmarkChunkify-8 10792 104 -99.04% benchmark old allocs new allocs delta BenchmarkLuhnify-8 18 1 -94.44% BenchmarkUnluhnify-8 18 1 -94.44% BenchmarkChunkify-8 44 2 -95.45% benchmark old bytes new bytes delta BenchmarkLuhnify-8 1278 64 -94.99% BenchmarkUnluhnify-8 1278 64 -94.99% BenchmarkChunkify-8 42552 128 -99.70% GitHub-Pull-Request: https://github.com/syncthing/syncthing/pull/4346 --- lib/protocol/deviceid.go | 30 +++++++++++++--------- lib/protocol/deviceid_test.go | 38 ++++++++++++++++++++++++++++ vendor/github.com/calmh/luhn/luhn.go | 21 +++++++-------- vendor/manifest | 2 +- 4 files changed, 66 insertions(+), 25 deletions(-) diff --git a/lib/protocol/deviceid.go b/lib/protocol/deviceid.go index 2e412461c..df1cbdece 100644 --- a/lib/protocol/deviceid.go +++ b/lib/protocol/deviceid.go @@ -8,7 +8,6 @@ import ( "encoding/binary" "errors" "fmt" - "regexp" "strings" "github.com/calmh/luhn" @@ -155,16 +154,17 @@ func luhnify(s string) (string, error) { panic("unsupported string length") } - res := make([]string, 0, 4) + res := make([]byte, 4*(13+1)) for i := 0; i < 4; i++ { p := s[i*13 : (i+1)*13] + copy(res[i*(13+1):], p) l, err := luhn.Base32.Generate(p) if err != nil { return "", err } - res = append(res, fmt.Sprintf("%s%c", p, l)) + res[(i+1)*(13)+i] = byte(l) } - return res[0] + res[1] + res[2] + res[3], nil + return string(res), nil } func unluhnify(s string) (string, error) { @@ -172,25 +172,31 @@ func unluhnify(s string) (string, error) { return "", fmt.Errorf("%q: unsupported string length %d", s, len(s)) } - res := make([]string, 0, 4) + res := make([]byte, 52) for i := 0; i < 4; i++ { - p := s[i*14 : (i+1)*14-1] + p := s[i*(13+1) : (i+1)*(13+1)-1] + copy(res[i*13:], p) l, err := luhn.Base32.Generate(p) if err != nil { return "", err } - if g := fmt.Sprintf("%s%c", p, l); g != s[i*14:(i+1)*14] { + if s[(i+1)*14-1] != byte(l) { return "", fmt.Errorf("%q: check digit incorrect", s) } - res = append(res, p) } - return res[0] + res[1] + res[2] + res[3], nil + return string(res), nil } func chunkify(s string) string { - s = regexp.MustCompile("(.{7})").ReplaceAllString(s, "$1-") - s = strings.Trim(s, "-") - return s + chunks := len(s) / 7 + res := make([]byte, chunks*(7+1)-1) + for i := 0; i < chunks; i++ { + if i > 0 { + res[i*(7+1)-1] = '-' + } + copy(res[i*(7+1):], s[i*7:(i+1)*7]) + } + return string(res) } func unchunkify(s string) string { diff --git a/lib/protocol/deviceid_test.go b/lib/protocol/deviceid_test.go index 02893b2a4..0f9e3527a 100644 --- a/lib/protocol/deviceid_test.go +++ b/lib/protocol/deviceid_test.go @@ -150,3 +150,41 @@ func TestNewDeviceIDMarshalling(t *testing.T) { t.Error("Mismatch in old -> new direction") } } + +var resStr string + +func BenchmarkLuhnify(b *testing.B) { + str := "ABCDEFGHIJABCDEFGHIJABCDEFGHIJABCDEFGHIJABCDEFGHIJAB" + var err error + for i := 0; i < b.N; i++ { + resStr, err = luhnify(str) + if err != nil { + b.Fatal(err) + } + } +} + +func BenchmarkUnluhnify(b *testing.B) { + str, _ := luhnify("ABCDEFGHIJABCDEFGHIJABCDEFGHIJABCDEFGHIJABCDEFGHIJAB") + var err error + for i := 0; i < b.N; i++ { + resStr, err = unluhnify(str) + if err != nil { + b.Fatal(err) + } + } +} + +func BenchmarkChunkify(b *testing.B) { + str := "ABCDEFGHIJABCDEFGHIJABCDEFGHIJABCDEFGHIJABCDEFGHIJAB" + for i := 0; i < b.N; i++ { + resStr = chunkify(str) + } +} + +func BenchmarkUnchunkify(b *testing.B) { + str := chunkify("ABCDEFGHIJABCDEFGHIJABCDEFGHIJABCDEFGHIJABCDEFGHIJAB") + for i := 0; i < b.N; i++ { + resStr = unchunkify(str) + } +} diff --git a/vendor/github.com/calmh/luhn/luhn.go b/vendor/github.com/calmh/luhn/luhn.go index 8bcb0cd11..f09f3f298 100644 --- a/vendor/github.com/calmh/luhn/luhn.go +++ b/vendor/github.com/calmh/luhn/luhn.go @@ -19,10 +19,6 @@ var ( // Generate returns a check digit for the string s, which should be composed // of characters from the Alphabet a. func (a Alphabet) Generate(s string) (rune, error) { - if err := a.check(); err != nil { - return 0, err - } - factor := 1 sum := 0 n := len(a) @@ -57,14 +53,15 @@ func (a Alphabet) Validate(s string) bool { return rune(s[len(s)-1]) == c } -// check returns an error if the given alphabet does not consist of unique characters -func (a Alphabet) check() error { - cm := make(map[byte]bool, len(a)) - for i := range a { - if cm[a[i]] { - return fmt.Errorf("Digit %q non-unique in alphabet %q", a[i], a) +// NewAlphabet converts the given string an an Alphabet, verifying that it +// is correct. +func NewAlphabet(s string) (Alphabet, error) { + cm := make(map[byte]bool, len(s)) + for i := range s { + if cm[s[i]] { + return "", fmt.Errorf("Digit %q non-unique in alphabet %q", s[i], s) } - cm[a[i]] = true + cm[s[i]] = true } - return nil + return Alphabet(s), nil } diff --git a/vendor/manifest b/vendor/manifest index a6080cd20..72d9c1c01 100644 --- a/vendor/manifest +++ b/vendor/manifest @@ -53,7 +53,7 @@ "importpath": "github.com/calmh/luhn", "repository": "https://github.com/calmh/luhn", "vcs": "git", - "revision": "0c8388ff95fa92d4094011e5a04fc99dea3d1632", + "revision": "c0f1d77264fb3d1bfc65b70eea6ee264058c57c0", "branch": "master", "notests": true },