Skip to content
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

net: dnsConfig could be more usable as-is #15473

Closed
mdempsky opened this issue Apr 27, 2016 · 6 comments
Closed

net: dnsConfig could be more usable as-is #15473

mdempsky opened this issue Apr 27, 2016 · 6 comments

Comments

@mdempsky
Copy link
Member

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

@mdempsky mdempsky added this to the Go1.7Maybe milestone Apr 27, 2016
@danp
Copy link
Contributor

danp commented Apr 27, 2016

Sounds good. How about I start with changing cfg.timeout to a duration?

@mdempsky
Copy link
Member Author

SGTM

@gopherbot
Copy link

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>
@gopherbot
Copy link

CL https://golang.org/cl/22556 mentions this issue.

@gopherbot
Copy link

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>
@mdempsky
Copy link
Member Author

This is as done as I had in mind. Can reopen or file new issues if anyone has ideas for more work.

@golang golang locked and limited conversation to collaborators Apr 28, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants