{"action":"created","issue":{"url":"https://api.github.com/repos/miekg/dns/issues/366","repository_url":"https://api.github.com/repos/miekg/dns","labels_url":"https://api.github.com/repos/miekg/dns/issues/366/labels{/name}","comments_url":"https://api.github.com/repos/miekg/dns/issues/366/comments","events_url":"https://api.github.com/repos/miekg/dns/issues/366/events","html_url":"https://github.com/miekg/dns/pull/366","id":159229590,"number":366,"title":"Remove temporary buffer used by packTxtString.","user":{"login":"michaelharo","id":7861967,"avatar_url":"https://avatars.githubusercontent.com/u/7861967?v=3","gravatar_id":"","url":"https://api.github.com/users/michaelharo","html_url":"https://github.com/michaelharo","followers_url":"https://api.github.com/users/michaelharo/followers","following_url":"https://api.github.com/users/michaelharo/following{/other_user}","gists_url":"https://api.github.com/users/michaelharo/gists{/gist_id}","starred_url":"https://api.github.com/users/michaelharo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/michaelharo/subscriptions","organizations_url":"https://api.github.com/users/michaelharo/orgs","repos_url":"https://api.github.com/users/michaelharo/repos","events_url":"https://api.github.com/users/michaelharo/events{/privacy}","received_events_url":"https://api.github.com/users/michaelharo/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"milestone":null,"comments":2,"created_at":"2016-06-08T17:45:04Z","updated_at":"2016-06-08T19:05:24Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/miekg/dns/pulls/366","html_url":"https://github.com/miekg/dns/pull/366","diff_url":"https://github.com/miekg/dns/pull/366.diff","patch_url":"https://github.com/miekg/dns/pull/366.patch"},"body":"This buffer was used to copy a string into it before copying that copy\r\ninto msg.  The intermediate buffer was never modified so there is no\r\nneed to make a copy."},"comment":{"url":"https://api.github.com/repos/miekg/dns/issues/comments/224695107","html_url":"https://github.com/miekg/dns/pull/366#issuecomment-224695107","issue_url":"https://api.github.com/repos/miekg/dns/issues/366","id":224695107,"user":{"login":"michaelharo","id":7861967,"avatar_url":"https://avatars.githubusercontent.com/u/7861967?v=3","gravatar_id":"","url":"https://api.github.com/users/michaelharo","html_url":"https://github.com/michaelharo","followers_url":"https://api.github.com/users/michaelharo/followers","following_url":"https://api.github.com/users/michaelharo/following{/other_user}","gists_url":"https://api.github.com/users/michaelharo/gists{/gist_id}","starred_url":"https://api.github.com/users/michaelharo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/michaelharo/subscriptions","organizations_url":"https://api.github.com/users/michaelharo/orgs","repos_url":"https://api.github.com/users/michaelharo/repos","events_url":"https://api.github.com/users/michaelharo/events{/privacy}","received_events_url":"https://api.github.com/users/michaelharo/received_events","type":"User","site_admin":false},"created_at":"2016-06-08T19:05:24Z","updated_at":"2016-06-08T19:05:24Z","body":"It looks like currently a new 1k+1b buffer is allocated per RR that has a string type in it.\r\nI see the string buffers used by the following RR types: (type: count)\r\nGPOS: 3\r\nHINFO: 2\r\nNAPTR: 3\r\nTKEY: 2\r\nTXT: potentially > 1\r\n\r\nCount listed above minus 1 is the savings from reusing the buffer.  Is this worth it?  If yes, then I think the code should be changed so that a nil buffer could be passed to indicate create it locally as many uses of this function allocate a buffer immediately before calling the function.\r\n\r\nI wrote a quick benchmark to try to test this.  Please let me know how it's flawed.  This test assumes there are either 1 or 3 strings to be converted per RR and that the buffer isn't shared across RRs.\r\n\r\n```\r\nfunc BenchmarkWithoutBuffer3(b *testing.B) {\r\n  s1 := \"My string is this long\"\r\n  s2 := \"My other string is long too\"\r\n  s3 := \"My last string is not short.\"\r\n  b.ReportAllocs()\r\n  b.ResetTimer()\r\n  for i := 0; i < b.N; i++ {\r\n    _ = []byte(s1)\r\n    _ = []byte(s2)\r\n    _ = []byte(s3)\r\n  }\r\n}\r\nfunc BenchmarkWithBuffer3(b *testing.B) {\r\n  s1 := \"My string is this long\"\r\n  s2 := \"My other string is long too\"\r\n  s3 := \"My last string is not short.\"\r\n  b.ReportAllocs()\r\n  b.ResetTimer()\r\n  for i := 0; i < b.N; i++ {\r\n    buf := make([]byte, 1025)\r\n    copy(buf, s1)\r\n    copy(buf, s2)\r\n    copy(buf, s3)\r\n  }\r\n}\r\n\r\nfunc BenchmarkWithoutBuffer1(b *testing.B) {\r\n  s1 := \"My string is this long\"\r\n  b.ReportAllocs()\r\n  b.ResetTimer()\r\n  for i := 0; i < b.N; i++ {\r\n    _ = []byte(s1)\r\n  }\r\n}\r\nfunc BenchmarkWithBuffer1(b *testing.B) {\r\n  s1 := \"My string is this long\"\r\n  b.ReportAllocs()\r\n  b.ResetTimer()\r\n  for i := 0; i < b.N; i++ {\r\n    buf := make([]byte, 1025)\r\n    copy(buf, s1)\r\n  }\r\n}\r\n```\r\n\r\nThe results from running this:\r\n```\r\nBenchmarkWithoutBuffer3-6       50000000                37.6 ns/op             0 B/op          0 allocs/op\r\nBenchmarkWithBuffer3-6          20000000                57.0 ns/op             0 B/op          0 allocs/op\r\nBenchmarkWithoutBuffer1-6       100000000               13.3 ns/op             0 B/op          0 allocs/op\r\nBenchmarkWithBuffer1-6          30000000                47.2 ns/op             0 B/op          0 allocs/op"}}
{"actor":{"display_login":"michaelharo"}}