Bug 13159 – std.socket.getAddress allocates once per DNS lookup hit

[Arch x86_64, dmd/phobos/druntime from git 140719] std.socket.getAddress is very allocation-happy. A DNS lookup often/(always?) gives more than one resulting IP, and getAddress naïvely concatenates each into an Address[] array in a foreach loop. As a concrete example, getAddress("irc.freenode.net", cast(ushort)6667) returned 51 hits. In Address[] getAddress(in char[], in char[]) near line 1116: > // use getAddressInfo > Address[] results; > auto infos = getAddressInfo(hostname, service); > foreach (ref info; infos) > results ~= info.address; > return results; Unless this is a valid use-case for ScopeBuffer, could we at least tack a results.reserve(64) in there? (I think AddressInfo.sizeof is 40 bytes.)

Assuming that infos has a length property, it would be better yet to simply do: auto infos = getAddressInfo(hostname, service); auto results = new Address[infos.length]; Which would result in exactly the amount of memory needed being allocated, and only 1 allocation being done. I would also suggest the possibility of adding an OutputRange based version.

I already posted a PR for this BTW: https://github.com/D-Programming-Language/phobos/pull/2351 (In reply to Orvid King from comment #1) > Assuming that infos has a length property, it would be better yet to simply > do: > > auto infos = getAddressInfo(hostname, service); > auto results = new Address[infos.length]; This would allocate even when infos.length is 0. > Which would result in exactly the amount of memory needed being allocated, > and only 1 allocation being done. `getAddressInfo` allocates too, and easily more than once, so that wouldn't be quite true. > I would also suggest the possibility of adding an OutputRange based version. We can do better - a version that returns the results as a lazy forward range. The most underlying data structure here is a singly linked list. We can use reference counting to ensure the list is freed (freeaddrinfo). It's a much heavier change though so I'm not going to do it in PR #2351.

Commit pushed to master at https://github.com/D-Programming-Language/phobos https://github.com/D-Programming-Language/phobos/commit/96893cbf467524d05d9f8b0f51398585ca77a423 Pre-allocate result array in getAddress and use Appender in getAddressInfoImpl Fixes issue #13159

??? A concatenation is not an allocation! The runtime will effectively increase arrays when they are appended to by powers of two until the GC block size (4096 bytes), because GC object bins have sizes of powers of two (16 to 2048). Furthermore, didn't recent benchmarks show that std.array.appender performed no better than built-in arrays for concatenation?

The getAddress patch is fine. The getAddressInfo patch seems pointless to me, it does not preallocate any memory (but could be made to if the linked list is traversed twice).

(In reply to Vladimir Panteleev from comment #5) > The getAddress patch is fine. The getAddressInfo patch seems pointless to > me, it does not preallocate any memory (but could be made to if the linked > list is traversed twice). Appender has gone through a lot of revision in recent years. Array appends might be better today. The difference is probably negligible; the getAddress patch was the main point. The ideal would be a lazy range version of getAddressInfo. With both `getAddress` and `getAddressInfo` taken, bikeshedding ahoy :)

THIS ISSUE HAS BEEN MOVED TO GITHUB https://github.com/dlang/phobos/issues/9637 DO NOT COMMENT HERE ANYMORE, NOBODY WILL SEE IT, THIS ISSUE HAS BEEN MOVED TO GITHUB