From 22cda6dc4f8b43ab129d7e79c24975180a0aace6 Mon Sep 17 00:00:00 2001 From: Julian Picht Date: Wed, 4 Dec 2019 08:25:08 +0100 Subject: [PATCH] Speed up NextLabel and PrevLabel (#1039) * change NextLabel and PrevLabel to be faster This reduces readability, but they are in the hot path of coredns. * @redyeti pointed out, that my implementation disregarded triple backslashes * add synthetic benchmark-tests for PrevLabel and NextLabel * rename ii -> j * invert compare * PrevLabel: add empty string check + test case * NextLabel: fix and add testcase for NextLabel("", offset>0) --- labels.go | 60 ++++++++++++++++++++--------- labels_test.go | 101 ++++++++++++++++++++++++++++++++++++++++++++----- 2 files changed, 133 insertions(+), 28 deletions(-) diff --git a/labels.go b/labels.go index e32d2a1d25..10d824718a 100644 --- a/labels.go +++ b/labels.go @@ -126,20 +126,23 @@ func Split(s string) []int { // The bool end is true when the end of the string has been reached. // Also see PrevLabel. func NextLabel(s string, offset int) (i int, end bool) { - quote := false + if s == "" { + return 0, true + } for i = offset; i < len(s)-1; i++ { - switch s[i] { - case '\\': - quote = !quote - default: - quote = false - case '.': - if quote { - quote = !quote - continue - } - return i + 1, false + if s[i] != '.' { + continue + } + j := i - 1 + for j >= 0 && s[j] == '\\' { + j-- + } + + if (j-i)%2 == 0 { + continue } + + return i + 1, false } return i + 1, true } @@ -149,17 +152,38 @@ func NextLabel(s string, offset int) (i int, end bool) { // The bool start is true when the start of the string has been overshot. // Also see NextLabel. func PrevLabel(s string, n int) (i int, start bool) { + if s == "" { + return 0, true + } if n == 0 { return len(s), false } - lab := Split(s) - if lab == nil { - return 0, true + + l := len(s) - 1 + if s[l] == '.' { + l-- } - if n > len(lab) { - return 0, true + + for ; l >= 0 && n > 0; l-- { + if s[l] != '.' { + continue + } + j := l - 1 + for j >= 0 && s[j] == '\\' { + j-- + } + + if (j-l)%2 == 0 { + continue + } + + n-- + if n == 0 { + return l + 1, false + } } - return lab[len(lab)-n], false + + return 0, n > 1 } // equal compares a and b while ignoring case. It returns true when equal otherwise false. diff --git a/labels_test.go b/labels_test.go index 3f666df662..ee7621b3b0 100644 --- a/labels_test.go +++ b/labels_test.go @@ -40,16 +40,17 @@ func TestCompareDomainName(t *testing.T) { func TestSplit(t *testing.T) { splitter := map[string]int{ - "www.miek.nl.": 3, - "www.miek.nl": 3, - "www..miek.nl": 4, - `www\.miek.nl.`: 2, - `www\\.miek.nl.`: 3, - ".": 0, - "nl.": 1, - "nl": 1, - "com.": 1, - ".com.": 2, + "www.miek.nl.": 3, + "www.miek.nl": 3, + "www..miek.nl": 4, + `www\.miek.nl.`: 2, + `www\\.miek.nl.`: 3, + `www\\\.miek.nl.`: 2, + ".": 0, + "nl.": 1, + "nl": 1, + "com.": 1, + ".com.": 2, } for s, i := range splitter { if x := len(Split(s)); x != i { @@ -79,12 +80,32 @@ func TestSplit2(t *testing.T) { } } +func TestNextLabel(t *testing.T) { + type next struct { + string + int + } + nexts := map[next]int{ + {"", 1}: 0, + {"www.miek.nl.", 0}: 4, + {"www.miek.nl.", 4}: 9, + {"www.miek.nl.", 9}: 12, + } + for s, i := range nexts { + x, ok := NextLabel(s.string, s.int) + if i != x { + t.Errorf("label should be %d, got %d, %t: nexting %d, %s", i, x, ok, s.int, s.string) + } + } +} + func TestPrevLabel(t *testing.T) { type prev struct { string int } prever := map[prev]int{ + {"", 1}: 0, {"www.miek.nl.", 0}: 12, {"www.miek.nl.", 1}: 9, {"www.miek.nl.", 2}: 4, @@ -237,3 +258,63 @@ func BenchmarkIsSubDomain(b *testing.B) { IsSubDomain("miek.nl.", "aa.example.com.") } } + +func BenchmarkNextLabelSimple(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + NextLabel("www.example.com", 0) + NextLabel("www.example.com", 5) + NextLabel("www.example.com", 12) + } +} + +func BenchmarkPrevLabelSimple(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + PrevLabel("www.example.com", 0) + PrevLabel("www.example.com", 5) + PrevLabel("www.example.com", 12) + } +} + +func BenchmarkNextLabelComplex(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + NextLabel(`www\.example.com`, 0) + NextLabel(`www\\.example.com`, 0) + NextLabel(`www\\\.example.com`, 0) + } +} + +func BenchmarkPrevLabelComplex(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + PrevLabel(`www\.example.com`, 10) + PrevLabel(`www\\.example.com`, 10) + PrevLabel(`www\\\.example.com`, 10) + } +} + +func BenchmarkNextLabelMixed(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + NextLabel("www.example.com", 0) + NextLabel(`www\.example.com`, 0) + NextLabel("www.example.com", 5) + NextLabel(`www\\.example.com`, 0) + NextLabel("www.example.com", 12) + NextLabel(`www\\\.example.com`, 0) + } +} + +func BenchmarkPrevLabelMixed(b *testing.B) { + b.ReportAllocs() + for i := 0; i < b.N; i++ { + PrevLabel("www.example.com", 0) + PrevLabel(`www\.example.com`, 10) + PrevLabel("www.example.com", 5) + PrevLabel(`www\\.example.com`, 10) + PrevLabel("www.example.com", 12) + PrevLabel(`www\\\.example.com`, 10) + } +}