dns/dnsmessage: don't include bytes after name.Length in the compression map
This improves the performance of name compression and makes
the name.Data[name.Length:] not included in the compression
map, it is unnecessary and might cause issues (i.e. reusing
the Name struct, without using the NewName function).
goos: linux
goarch: amd64
pkg: golang.org/x/net/dns/dnsmessage
cpu: Intel(R) Core(TM) i5-4200M CPU @ 2.50GHz
│ before │ after │
│ sec/op │ sec/op vs base │
Pack-4 15.672µ ± 13% 5.470µ ± 14% -65.10% (p=0.000 n=10)
AppendPack-4 15.144µ ± 12% 5.330µ ± 10% -64.80% (p=0.000 n=10)
geomean 15.41µ 5.400µ -64.95%
│ before │ after │
│ B/op │ B/op vs base │
Pack-4 6.051Ki ± 0% 1.285Ki ± 0% -78.76% (p=0.000 n=10)
AppendPack-4 5684.0 ± 0% 804.0 ± 0% -85.86% (p=0.000 n=10)
geomean 5.795Ki 1.005Ki -82.67%
│ before │ after │
│ allocs/op │ allocs/op vs base │
Pack-4 21.00 ± 0% 11.00 ± 0% -47.62% (p=0.000 n=10)
AppendPack-4 20.00 ± 0% 10.00 ± 0% -50.00% (p=0.000 n=10)
geomean 20.49 10.49 -48.82%
Change-Id: Idf40d5d4790d37eb7253214f089eff859a937c60
GitHub-Last-Rev: a3182830e27086a0e12e116c7f7916468eb1edf2
GitHub-Pull-Request: golang/net#190
Reviewed-on: https://go-review.googlesource.com/c/net/+/522817
Run-TryBot: Mateusz Poliwczak <mpoliwczak34@gmail.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Ian Lance Taylor <iant@google.com>
Run-TryBot: Ian Lance Taylor <iant@google.com>
Reviewed-by: Damien Neil <dneil@google.com>
Auto-Submit: Ian Lance Taylor <iant@google.com>
diff --git a/dns/dnsmessage/message.go b/dns/dnsmessage/message.go
index cd997ba..9ddf2c2 100644
--- a/dns/dnsmessage/message.go
+++ b/dns/dnsmessage/message.go
@@ -1961,6 +1961,8 @@
return append(msg, 0), nil
}
+ var nameAsStr string
+
// Emit sequence of counted strings, chopping at dots.
for i, begin := 0, 0; i < int(n.Length); i++ {
// Check for the end of the segment.
@@ -1991,7 +1993,7 @@
// segment. A pointer is two bytes with the two most significant
// bits set to 1 to indicate that it is a pointer.
if (i == 0 || n.Data[i-1] == '.') && compression != nil {
- if ptr, ok := compression[string(n.Data[i:])]; ok {
+ if ptr, ok := compression[string(n.Data[i:n.Length])]; ok {
// Hit. Emit a pointer instead of the rest of
// the domain.
return append(msg, byte(ptr>>8|0xC0), byte(ptr)), nil
@@ -2000,7 +2002,12 @@
// Miss. Add the suffix to the compression table if the
// offset can be stored in the available 14 bytes.
if len(msg) <= int(^uint16(0)>>2) {
- compression[string(n.Data[i:])] = len(msg) - compressionOff
+ if nameAsStr == "" {
+ // allocate n.Data on the heap once, to avoid allocating it
+ // multiple times (for next labels).
+ nameAsStr = string(n.Data[:n.Length])
+ }
+ compression[nameAsStr[i:]] = len(msg) - compressionOff
}
}
}
diff --git a/dns/dnsmessage/message_test.go b/dns/dnsmessage/message_test.go
index 1b7f3cb..ee42feb 100644
--- a/dns/dnsmessage/message_test.go
+++ b/dns/dnsmessage/message_test.go
@@ -1813,3 +1813,37 @@
t.Fatalf("p.SkipAllAuthorities(): unexpected success in Answer section")
}
}
+
+func TestBuilderNameCompressionWithNonZeroedName(t *testing.T) {
+ b := NewBuilder(nil, Header{})
+ b.EnableCompression()
+ if err := b.StartQuestions(); err != nil {
+ t.Fatalf("b.StartQuestions() unexpected error: %v", err)
+ }
+
+ name := MustNewName("go.dev.")
+ if err := b.Question(Question{Name: name}); err != nil {
+ t.Fatalf("b.Question() unexpected error: %v", err)
+ }
+
+ // Character that is not part of the name (name.Data[:name.Length]),
+ // shouldn't affect the compression algorithm.
+ name.Data[name.Length] = '1'
+ if err := b.Question(Question{Name: name}); err != nil {
+ t.Fatalf("b.Question() unexpected error: %v", err)
+ }
+
+ msg, err := b.Finish()
+ if err != nil {
+ t.Fatalf("b.Finish() unexpected error: %v", err)
+ }
+
+ expect := []byte{
+ 0, 0, 0, 0, 0, 2, 0, 0, 0, 0, 0, 0, // header
+ 2, 'g', 'o', 3, 'd', 'e', 'v', 0, 0, 0, 0, 0, // question 1
+ 0xC0, 12, 0, 0, 0, 0, // question 2
+ }
+ if !bytes.Equal(msg, expect) {
+ t.Fatalf("b.Finish() = %v, want: %v", msg, expect)
+ }
+}
