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.
This commit is contained in:
Miek Gieben 2017-10-15 16:22:03 +02:00 committed by GitHub
parent ca092a4398
commit 54ceb83127
2 changed files with 42 additions and 22 deletions

View File

@ -1,7 +1,5 @@
package dns package dns
import "strings"
// Holds a bunch of helper functions for dealing with labels. // Holds a bunch of helper functions for dealing with labels.
// SplitDomainName splits a name string into it's 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 // CompareDomainName compares the names s1 and s2 and
// returns how many labels they have in common starting from the *right*. // 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. // before the comparison.
// //
// www.miek.nl. and miek.nl. have two labels in common: miek and nl // 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. // s1 and s2 must be syntactically valid domain names.
func CompareDomainName(s1, s2 string) (n int) { func CompareDomainName(s1, s2 string) (n int) {
s1, s2 = strings.ToLower(s1), strings.ToLower(s2) // the first check: root label
s1 = Fqdn(s1) if s1 == "." || s2 == "." {
s2 = Fqdn(s2) return 0
}
l1 := Split(s1) l1 := Split(s1)
l2 := Split(s2) l2 := Split(s2)
// the first check: root label
if l1 == nil || l2 == nil {
return
}
j1 := len(l1) - 1 // end j1 := len(l1) - 1 // end
i1 := len(l1) - 2 // start i1 := len(l1) - 2 // start
j2 := len(l2) - 1 j2 := len(l2) - 1
i2 := len(l2) - 2 i2 := len(l2) - 2
// the second check can be done here: last/only label // the second check can be done here: last/only label
// before we fall through into the for-loop below // 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++ n++
} else { } else {
return return
@ -78,7 +73,7 @@ func CompareDomainName(s1, s2 string) (n int) {
if i1 < 0 || i2 < 0 { if i1 < 0 || i2 < 0 {
break 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++ n++
} else { } else {
break break
@ -169,3 +164,28 @@ func PrevLabel(s string, n int) (i int, start bool) {
} }
return lab[len(lab)-n], false 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
}

View File

@ -7,8 +7,8 @@ func TestCompareDomainName(t *testing.T) {
s2 := "miek.nl." s2 := "miek.nl."
s3 := "www.bla.nl." s3 := "www.bla.nl."
s4 := "nl.www.bla." s4 := "nl.www.bla."
s5 := "nl" s5 := "nl."
s6 := "miek.nl" s6 := "miek.nl."
if CompareDomainName(s1, s2) != 2 { if CompareDomainName(s1, s2) != 2 {
t.Errorf("%s with %s should be %d", 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) { func BenchmarkSplitLabels(b *testing.B) {
for i := 0; i < b.N; i++ { for i := 0; i < b.N; i++ {
Split("www.example.com") Split("www.example.com.")
} }
} }
func BenchmarkLenLabels(b *testing.B) { func BenchmarkLenLabels(b *testing.B) {
for i := 0; i < b.N; i++ { 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() b.ReportAllocs()
for i := 0; i < b.N; i++ { 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) { func BenchmarkIsSubDomain(b *testing.B) {
b.ReportAllocs() b.ReportAllocs()
for i := 0; i < b.N; i++ { for i := 0; i < b.N; i++ {
IsSubDomain("www.example.com", "aa.example.com") IsSubDomain("www.example.com.", "aa.example.com.")
IsSubDomain("example.com", "aa.example.com") IsSubDomain("example.com.", "aa.example.com.")
IsSubDomain("miek.nl", "aa.example.com") IsSubDomain("miek.nl.", "aa.example.com.")
} }
} }