From a4af4dd14e1115ec63796347be5278bf401bcc0c Mon Sep 17 00:00:00 2001 From: Daniel Morsing Date: Thu, 5 Feb 2015 13:07:07 +0000 Subject: [PATCH] Used shared backing array for Msg.Copy If you have a system with large amounts of copies, these slice allocations start stacking up. Use a shared slice and then subslice them with a cap limit so that append works properly. Also, add a benchmark and test for Msg.Copy Benchcmp: benchmark old ns/op new ns/op delta BenchmarkCopy 1880 1672 -11.06% benchmark old allocs new allocs delta BenchmarkCopy 13 11 -15.38% benchmark old bytes new bytes delta BenchmarkCopy 528 528 +0.00% --- dns_test.go | 42 ++++++++++++++++++++++++++++++++++++++++++ msg.go | 21 +++++++++++++++------ 2 files changed, 57 insertions(+), 6 deletions(-) diff --git a/dns_test.go b/dns_test.go index df1e99f3..0b30af6f 100644 --- a/dns_test.go +++ b/dns_test.go @@ -510,6 +510,48 @@ func TestCopy(t *testing.T) { } } +func TestMsgCopy(t *testing.T) { + m := new(Msg) + m.SetQuestion("miek.nl.", TypeA) + rr, _ := NewRR("miek.nl. 2311 IN A 127.0.0.1") + m.Answer = []RR{rr} + rr, _ = NewRR("miek.nl. 2311 IN NS 127.0.0.1") + m.Ns = []RR{rr} + + m1 := m.Copy() + if m.String() != m1.String() { + t.Fatalf("Msg.Copy() failed %s != %s", m.String(), m1.String()) + } + + m1.Answer[0], _ = NewRR("somethingelse.nl. 2311 IN A 127.0.0.1") + if m.String() == m1.String() { + t.Fatalf("Msg.Copy() failed; change to copy changed template %s", m.String()) + } + + rr, _ = NewRR("miek.nl. 2311 IN A 127.0.0.2") + m1.Answer = append(m1.Answer, rr) + if m1.Ns[0].String() == m1.Answer[1].String() { + t.Fatalf("Msg.Copy() failed; append changed underlying array %s", m1.Ns[0].String()) + } +} + +func BenchmarkCopy(b *testing.B) { + b.ReportAllocs() + m := new(Msg) + m.SetQuestion("miek.nl.", TypeA) + rr, _ := NewRR("miek.nl. 2311 IN A 127.0.0.1") + m.Answer = []RR{rr} + rr, _ = NewRR("miek.nl. 2311 IN NS 127.0.0.1") + m.Ns = []RR{rr} + rr, _ = NewRR("miek.nl. 2311 IN A 127.0.0.1") + m.Extra = []RR{rr} + + b.ResetTimer() + for i := 0; i < b.N; i++ { + m.Copy() + } +} + func TestPackIPSECKEY(t *testing.T) { tests := []string{ "38.2.0.192.in-addr.arpa. 7200 IN IPSECKEY ( 10 1 2 192.0.2.38 AQNRU3mG7TVTO2BkR47usntb102uFJtugbo6BSGvgqt4AQ== )", diff --git a/msg.go b/msg.go index 578fe924..aded9653 100644 --- a/msg.go +++ b/msg.go @@ -1912,25 +1912,34 @@ func (dns *Msg) Copy() *Msg { copy(r1.Question, dns.Question) // TODO(miek): Question is an immutable value, ok to do a shallow-copy } + rrArr := make([]RR, len(dns.Answer)+len(dns.Ns)+len(dns.Extra)) + var rri int + if len(dns.Answer) > 0 { - r1.Answer = make([]RR, len(dns.Answer)) + rrbegin := rri for i := 0; i < len(dns.Answer); i++ { - r1.Answer[i] = dns.Answer[i].copy() + rrArr[rri] = dns.Answer[i].copy() + rri++ } + r1.Answer = rrArr[rrbegin:rri:rri] } if len(dns.Ns) > 0 { - r1.Ns = make([]RR, len(dns.Ns)) + rrbegin := rri for i := 0; i < len(dns.Ns); i++ { - r1.Ns[i] = dns.Ns[i].copy() + rrArr[rri] = dns.Ns[i].copy() + rri++ } + r1.Ns = rrArr[rrbegin:rri:rri] } if len(dns.Extra) > 0 { - r1.Extra = make([]RR, len(dns.Extra)) + rrbegin := rri for i := 0; i < len(dns.Extra); i++ { - r1.Extra[i] = dns.Extra[i].copy() + rrArr[rri] = dns.Extra[i].copy() + rri++ } + r1.Extra = rrArr[rrbegin:rri:rri] } return r1