From 54ceb83127d7dadaf2701e59bbc9e1919b95b630 Mon Sep 17 00:00:00 2001 From: Miek Gieben Date: Sun, 15 Oct 2017 16:22:03 +0200 Subject: [PATCH] Optimize CompareDomainName (#535) Optimize CompareDomainName: old: BenchmarkCompareDomainName-2 1000000 1869 ns/op 64 B/op 2 allocs/op new: BenchmarkCompareDomainName-2 2000000 854 ns/op 64 B/op 2 allocs/op This removes the strings.ToLower and fixes the documentation. It also does not Fqdn's the names anymore (the documentation said we didn't, now the documentation is right again). Unlike what the documentation said we are comparing in a ignore-case manor, add helper function equal that does this without calling strings.ToLower. --- labels.go | 46 +++++++++++++++++++++++++++++++++------------- labels_test.go | 18 +++++++++--------- 2 files changed, 42 insertions(+), 22 deletions(-) diff --git a/labels.go b/labels.go index 9538d9c3..760b89e7 100644 --- a/labels.go +++ b/labels.go @@ -1,7 +1,5 @@ package dns -import "strings" - // Holds a bunch of helper functions for dealing with labels. // SplitDomainName splits a name string into it's labels. @@ -44,7 +42,7 @@ func SplitDomainName(s string) (labels []string) { // CompareDomainName compares the names s1 and s2 and // returns how many labels they have in common starting from the *right*. -// The comparison stops at the first inequality. The names are not downcased +// The comparison stops at the first inequality. The names are downcased // before the comparison. // // www.miek.nl. and miek.nl. have two labels in common: miek and nl @@ -52,24 +50,21 @@ func SplitDomainName(s string) (labels []string) { // // s1 and s2 must be syntactically valid domain names. func CompareDomainName(s1, s2 string) (n int) { - s1, s2 = strings.ToLower(s1), strings.ToLower(s2) - s1 = Fqdn(s1) - s2 = Fqdn(s2) + // the first check: root label + if s1 == "." || s2 == "." { + return 0 + } + l1 := Split(s1) l2 := Split(s2) - // the first check: root label - if l1 == nil || l2 == nil { - return - } - j1 := len(l1) - 1 // end i1 := len(l1) - 2 // start j2 := len(l2) - 1 i2 := len(l2) - 2 // the second check can be done here: last/only label // before we fall through into the for-loop below - if s1[l1[j1]:] == s2[l2[j2]:] { + if equal(s1[l1[j1]:], s2[l2[j2]:]) { n++ } else { return @@ -78,7 +73,7 @@ func CompareDomainName(s1, s2 string) (n int) { if i1 < 0 || i2 < 0 { break } - if s1[l1[i1]:l1[j1]] == s2[l2[i2]:l2[j2]] { + if equal(s1[l1[i1]:l1[j1]], s2[l2[i2]:l2[j2]]) { n++ } else { break @@ -169,3 +164,28 @@ func PrevLabel(s string, n int) (i int, start bool) { } return lab[len(lab)-n], false } + +// equal compares a and b while ignoring case. It returns true when equal otherwise false. +func equal(a, b string) bool { + // might be lifted into API function. + la := len(a) + lb := len(b) + if la != lb { + return false + } + + for i := la - 1; i >= 0; i-- { + ai := a[i] + bi := b[i] + if ai >= 'A' && ai <= 'Z' { + ai |= ('a' - 'A') + } + if bi >= 'A' && bi <= 'Z' { + bi |= ('a' - 'A') + } + if ai != bi { + return false + } + } + return true +} diff --git a/labels_test.go b/labels_test.go index 9875d6cd..108bb708 100644 --- a/labels_test.go +++ b/labels_test.go @@ -7,8 +7,8 @@ func TestCompareDomainName(t *testing.T) { s2 := "miek.nl." s3 := "www.bla.nl." s4 := "nl.www.bla." - s5 := "nl" - s6 := "miek.nl" + s5 := "nl." + s6 := "miek.nl." if CompareDomainName(s1, s2) != 2 { t.Errorf("%s with %s should be %d", s1, s2, 2) @@ -176,28 +176,28 @@ func TestIsDomainName(t *testing.T) { func BenchmarkSplitLabels(b *testing.B) { for i := 0; i < b.N; i++ { - Split("www.example.com") + Split("www.example.com.") } } func BenchmarkLenLabels(b *testing.B) { for i := 0; i < b.N; i++ { - CountLabel("www.example.com") + CountLabel("www.example.com.") } } -func BenchmarkCompareLabels(b *testing.B) { +func BenchmarkCompareDomainName(b *testing.B) { b.ReportAllocs() for i := 0; i < b.N; i++ { - CompareDomainName("www.example.com", "aa.example.com") + CompareDomainName("www.example.com.", "aa.example.com.") } } func BenchmarkIsSubDomain(b *testing.B) { b.ReportAllocs() for i := 0; i < b.N; i++ { - IsSubDomain("www.example.com", "aa.example.com") - IsSubDomain("example.com", "aa.example.com") - IsSubDomain("miek.nl", "aa.example.com") + IsSubDomain("www.example.com.", "aa.example.com.") + IsSubDomain("example.com.", "aa.example.com.") + IsSubDomain("miek.nl.", "aa.example.com.") } }