diff --git a/plugin/file/tree/less.go b/plugin/file/tree/less.go index 779300901..7668ff2ef 100644 --- a/plugin/file/tree/less.go +++ b/plugin/file/tree/less.go @@ -16,12 +16,11 @@ import ( // // The values of a and b are *not* lowercased before the comparison! func less(a, b string) int { - i := 1 aj := len(a) bj := len(b) for { - ai, oka := dns.PrevLabel(a, i) - bi, okb := dns.PrevLabel(b, i) + ai, oka := dns.PrevLabel(a[:aj], 1) + bi, okb := dns.PrevLabel(b[:bj], 1) if oka && okb { return 0 } @@ -38,7 +37,6 @@ func less(a, b string) int { return res } - i++ aj, bj = ai, bi } } diff --git a/plugin/file/tree/less_test.go b/plugin/file/tree/less_test.go index a1af23a38..0f8738c6e 100644 --- a/plugin/file/tree/less_test.go +++ b/plugin/file/tree/less_test.go @@ -1,6 +1,7 @@ package tree import ( + "bytes" "sort" "strings" "sync" @@ -119,3 +120,70 @@ func TestLess_ConcurrentNameAccess(t *testing.T) { } wg.Wait() } + +func BenchmarkLess(b *testing.B) { + // The original less function, serving as the benchmark test baseline. + less0 := func(a, b string) int { + i := 1 + aj := len(a) + bj := len(b) + for { + ai, oka := dns.PrevLabel(a, i) + bi, okb := dns.PrevLabel(b, i) + if oka && okb { + return 0 + } + + // sadly this []byte will allocate... TODO(miek): check if this is needed + // for a name, otherwise compare the strings. + ab := []byte(strings.ToLower(a[ai:aj])) + bb := []byte(strings.ToLower(b[bi:bj])) + doDDD(ab) + doDDD(bb) + + res := bytes.Compare(ab, bb) + if res != 0 { + return res + } + + i++ + aj, bj = ai, bi + } + } + + tests := []set{ + {"aaa.powerdns.de", "bbb.powerdns.net.", "xxx.powerdns.com."}, + {"aaa.POWERDNS.de", "bbb.PoweRdnS.net.", "xxx.powerdns.com."}, + {"aaa.aaaa.aa.", "aa.aaa.a.", "bbb.bbbb.bb."}, + {"aaaaa.", "aaa.", "bbb."}, + {"a.a.a.a.", "a.a.", "a.a.a."}, + {"example.", "z.example.", "a.example."}, + {"a.example.", "Z.a.example.", "z.example.", "yljkjljk.a.example.", "\\001.z.example.", "example.", "*.z.example.", "\\200.z.example.", "zABC.a.EXAMPLE."}, + {"a.example.", "Z.a.example.", "z.example.", "yljkjljk.a.example.", "example.", "*.z.example.", "zABC.a.EXAMPLE."}, + } + b.ResetTimer() + + b.Run("base", func(b *testing.B) { + for b.Loop() { + for _, t := range tests { + for m := range len(t) - 1 { + for n := m + 1; n < len(t); n++ { + less0(t[m], t[n]) + } + } + } + } + }) + + b.Run("optimized", func(b *testing.B) { + for b.Loop() { + for _, t := range tests { + for m := range len(t) - 1 { + for n := m + 1; n < len(t); n++ { + less(t[m], t[n]) + } + } + } + } + }) +} diff --git a/plugin/file/zone.go b/plugin/file/zone.go index aeb2a593e..e7afc795b 100644 --- a/plugin/file/zone.go +++ b/plugin/file/zone.go @@ -163,19 +163,22 @@ func (z *Zone) nameFromRight(qname string, i int) (string, bool) { return z.origin, false } + n := len(qname) for j := 1; j <= z.origLen; j++ { - if _, shot := dns.PrevLabel(qname, j); shot { + if m, shot := dns.PrevLabel(qname[:n], 1); shot { return qname, shot + } else { + n = m } } - k := 0 - var shot bool for j := 1; j <= i; j++ { - k, shot = dns.PrevLabel(qname, j+z.origLen) + m, shot := dns.PrevLabel(qname[:n], 1) if shot { return qname, shot + } else { + n = m } } - return qname[k:], false + return qname[n:], false } diff --git a/plugin/file/zone_test.go b/plugin/file/zone_test.go index f81a871ff..151cfedc9 100644 --- a/plugin/file/zone_test.go +++ b/plugin/file/zone_test.go @@ -1,6 +1,8 @@ package file import ( + "strconv" + "strings" "testing" "github.com/coredns/coredns/plugin/file/tree" @@ -35,6 +37,92 @@ func TestNameFromRight(t *testing.T) { } } +// Benchmark: measure performance across representative inputs and overshoot cases. +func BenchmarkNameFromRight(b *testing.B) { + origin := "example.org." + a := &Zone{origin: origin, origLen: dns.CountLabel(origin)} + + cases := []struct { + name string + qname string + i int + }{ + {"i0_origin", origin, 0}, + {"eq_origin_i1_shot", origin, 1}, + {"two_labels_i1", "a.b." + origin, 1}, + {"two_labels_i2", "a.b." + origin, 2}, + {"two_labels_i3_shot", "a.b." + origin, 3}, + {"ten_labels_i5", strings.Repeat("a.", 10) + origin, 5}, + {"ten_labels_i11_shot", strings.Repeat("a.", 10) + origin, 11}, + {"not_subdomain_shot", "other.tld.", 1}, + } + + var sink int + for _, tc := range cases { + b.Run(tc.name, func(b *testing.B) { + qn, i := tc.qname, tc.i + for b.Loop() { + s, shot := a.nameFromRight(qn, i) + sink += len(s) + if shot { + sink++ + } + } + }) + } + if sink == 42 { // prevent elimination + b.Log(sink) + } +} + +// BenchmarkNameFromRightRandomized iterates over a prebuilt pool +// of qnames and i values to emulate mixed workloads. +func BenchmarkNameFromRightRandomized(b *testing.B) { + origin := "example.org." + a := &Zone{origin: origin, origLen: dns.CountLabel(origin)} + + const poolSize = 1024 + type pair struct { + q string + i int + } + pool := make([]pair, 0, poolSize) + + // Build a variety of qnames with 1..8 labels before origin and various i, including overshoot. + for n := 1; n <= 8; n++ { + var sb strings.Builder + for k := range n { + sb.WriteString("l") + sb.WriteString(strconv.Itoa(k)) + sb.WriteByte('.') + } + sb.WriteString(origin) + q := sb.String() + for i := 0; i <= n+2; i++ { + pool = append(pool, pair{q: q, i: i}) + } + } + // Add some non-subdomain and shorter-than-origin cases. + pool = append(pool, pair{"org.", 1}, pair{"other.tld.", 1}) + + // Ensure pool length is power-of-two for fast masking; if not, trim. + // Here we just rely on modulo since the pool isn't huge. + + b.ReportAllocs() + var sink int + for n := range b.N { + p := pool[n%len(pool)] + s, shot := a.nameFromRight(p.q, p.i) + sink += len(s) + if shot { + sink++ + } + } + if sink == 43 { + b.Log(sink) + } +} + func TestInsertPreservesSRVCase(t *testing.T) { z := NewZone("home.arpa.", "stdin")