Repository: miekg/dns (language: go)

Issue:
IsFqdn performance issues
I have recently added the kubernetes node-cache and noticed a quite high CPU load. The cached node was actually higher than any of the load on the dns server before adding the caching layer.

So after a bit of pprof- and perf-ing around, I've noticed, that the `IsFqdn` call seems to be excessively expensive. Is this something that can be easily optimized? I'm not too deep into Go and the associated logic here, so feedback is welcome :D 

![image (11)](https://user-images.githubusercontent.com/424923/232502007-937845fd-53cb-4275-ac98-9d0a13977185.png)



Discussion / hints:
It's definitely showing up in your profiles, the logic is a little tricky, but I definitely see some opportunities to optimise it.
Jupp, that's why I started looking, as the receiving and sending side are also somewhat expensive, but it's just plain UDP mangling.

I went deeper and did some benchmarking based on the values from the unittests, but there's nothing obvious. So, you are likely right, that it could use some optimization, especially around `strings.LastIndexFunc` but it's not the performance issue I was looking for. So I guess, the code is fine and "We're holding it wrong" 

![Screenshot 2023-04-17 at 16 24 51](https://user-images.githubusercontent.com/424923/232515733-ee979a9d-01f0-4a72-b736-0f2420b9b046.png)

BTW: This is x86 on AWS, a M1 Mac is 4x as fast.

Fix patch:
diff --git a/defaults.go b/defaults.go
index 75b17f0c1..c1558b79c 100644
--- a/defaults.go
+++ b/defaults.go
@@ -272,18 +272,24 @@ func IsMsg(buf []byte) error {
 
 // IsFqdn checks if a domain name is fully qualified.
 func IsFqdn(s string) bool {
-	s2 := strings.TrimSuffix(s, ".")
-	if s == s2 {
+	// Check for (and remove) a trailing dot, returning if there isn't one.
+	if s == "" || s[len(s)-1] != '.' {
 		return false
 	}
+	s = s[:len(s)-1]
 
-	i := strings.LastIndexFunc(s2, func(r rune) bool {
+	// If we don't have an escape sequence before the final dot, we know it's
+	// fully qualified and can return here.
+	if s == "" || s[len(s)-1] != '\\' {
+		return true
+	}
+
+	// Otherwise we have to check if the dot is escaped or not by checking if
+	// there are an odd or even number of escape sequences before the dot.
+	i := strings.LastIndexFunc(s, func(r rune) bool {
 		return r != '\\'
 	})
-
-	// Test whether we have an even number of escape sequences before
-	// the dot or none.
-	return (len(s2)-i)%2 != 0
+	return (len(s)-i)%2 != 0
 }
 
 // IsRRset checks if a set of RRs is a valid RRset as defined by RFC 2181.


Test patch:
diff --git a/dns_test.go b/dns_test.go
index f1dbf23d4..cba5386af 100644
--- a/dns_test.go
+++ b/dns_test.go
@@ -314,3 +314,49 @@ func TestTKEY(t *testing.T) {
 		t.Fatalf("unable to parse TKEY string: %s", newError)
 	}
 }
+
+var (
+	sinkBool   bool
+	sinkString string
+)
+
+func BenchmarkIsFQDN(b *testing.B) {
+	b.Run("no_dot", func(b *testing.B) {
+		var r bool
+		for n := 0; n < b.N; n++ {
+			r = IsFqdn("www.google.com")
+		}
+		sinkBool = r
+	})
+	b.Run("unescaped", func(b *testing.B) {
+		var r bool
+		for n := 0; n < b.N; n++ {
+			r = IsFqdn("www.google.com.")
+		}
+		sinkBool = r
+	})
+	b.Run("escaped", func(b *testing.B) {
+		var r bool
+		for n := 0; n < b.N; n++ {
+			r = IsFqdn(`www.google.com\\\\\\\\.`)
+		}
+		sinkBool = r
+	})
+}
+
+func BenchmarkFQDN(b *testing.B) {
+	b.Run("is_fqdn", func(b *testing.B) {
+		var r string
+		for n := 0; n < b.N; n++ {
+			r = Fqdn("www.google.com.")
+		}
+		sinkString = r
+	})
+	b.Run("not_fqdn", func(b *testing.B) {
+		var r string
+		for n := 0; n < b.N; n++ {
+			r = Fqdn("www.google.com")
+		}
+		sinkString = r
+	})
+}