-
Notifications
You must be signed in to change notification settings - Fork 18k
net: dnsConfig could be more usable as-is #15473
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Labels
Milestone
Comments
Sounds good. How about I start with changing cfg.timeout to a duration? |
SGTM |
CL https://golang.org/cl/22548 mentions this issue. |
gopherbot
pushed a commit
that referenced
this issue
Apr 28, 2016
Instead of keeping the desired number of seconds and converting to time.Duration for every query, convert to time.Duration when building the config. Updates #15473 Change-Id: Ib24c050b593b3109011e359f4ed837a3fb45dc65 Reviewed-on: https://go-review.googlesource.com/22548 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
CL https://golang.org/cl/22556 mentions this issue. |
CL https://golang.org/cl/22571 mentions this issue. |
gopherbot
pushed a commit
that referenced
this issue
Apr 28, 2016
Avoids generating some redundant garbage from re-concatenating the same string for every DNS query. benchmark old allocs new allocs delta BenchmarkGoLookupIP-32 156 154 -1.28% BenchmarkGoLookupIPNoSuchHost-32 456 446 -2.19% BenchmarkGoLookupIPWithBrokenNameServer-32 577 564 -2.25% benchmark old bytes new bytes delta BenchmarkGoLookupIP-32 10873 10824 -0.45% BenchmarkGoLookupIPNoSuchHost-32 43303 43140 -0.38% BenchmarkGoLookupIPWithBrokenNameServer-32 46824 46616 -0.44% Update #15473. Change-Id: I3b0173dfedf31bd08eaea1069968b416850864a1 Reviewed-on: https://go-review.googlesource.com/22556 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
gopherbot
pushed a commit
that referenced
this issue
Apr 28, 2016
Avoids some extra work and string concatenation at query time. benchmark old allocs new allocs delta BenchmarkGoLookupIP-32 154 150 -2.60% BenchmarkGoLookupIPNoSuchHost-32 446 442 -0.90% BenchmarkGoLookupIPWithBrokenNameServer-32 564 568 +0.71% benchmark old bytes new bytes delta BenchmarkGoLookupIP-32 10824 10704 -1.11% BenchmarkGoLookupIPNoSuchHost-32 43140 42992 -0.34% BenchmarkGoLookupIPWithBrokenNameServer-32 46616 46680 +0.14% BenchmarkGoLookupIPWithBrokenNameServer's regression appears to be because it's actually only performing 1 LookupIP call, so the extra work done parsing the DNS config file doesn't amortize as well as for BenchmarkGoLookupIP or BenchmarkGoLOokupIPNoSuchHost, which perform 2000+ LookupIP calls per run. Update #15473. Change-Id: I98c8072f2f39e2f2ccd6c55e9e9bd309f5ad68f8 Reviewed-on: https://go-review.googlesource.com/22571 Reviewed-by: Brad Fitzpatrick <bradfitz@golang.org>
This is as done as I had in mind. Can reopen or file new issues if anyone has ideas for more work. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
dnsConfig currently reflects the contents of /etc/resolv.conf, but that means some of the fields need to be repeatedly processed in dnsclient_unix.go before being used. For example, in tryOneName we have to call JoinHostPort every time for each queried server, producing extra garbage, or convert cfg.timeout to time.Duration. (*dnsConfig).nameList could be simplified if we ensured search list entries always ended in a period. Maybe there are more.
/cc @dpiddy @bradfitz
The text was updated successfully, but these errors were encountered: