Fix IPv6 requests in the HTTP client #2082
Conversation
s-ludwig
force-pushed
the
issue2080_http_client_ipv6
branch
2 times, most recently
from
a7d7995
to
28aabf5
Compare
tests/vibe.http.2080/source/app.d
Outdated
| @@ -0,0 +1,32 @@ | |||
| /++ dub.sdl: | |||
| dependency "vibe-d" version="~>0.8.3-alpha.4" | |||
There was a problem hiding this comment.
Shouldn't this be path too? (we probably should allow single files in the test suite too)
tests/vibe.http.2080/source/app.d
Outdated
| } catch (Exception e) assert(false, e.msg); | ||
| }); | ||
| runApplication(); | ||
| } else logWarn("Skipping HTTP+IPv6 test due to Travis not supporting IPv6 loopback"); |
There was a problem hiding this comment.
This probably gets forgotten easily. Have you considering removing the test in the travis.sh script?
|
|
||
| // Provide port number when it is not the default one (RFC2616 section 14.23) | ||
| // IPv6 addresses need to be put into brackets | ||
| auto hoststr = url.host.canFind(':') ? "["~url.host~"]" : url.host; |
There was a problem hiding this comment.
If you care about performance, choose(url.host.canFind(':'), url.host, chain("[", url.host, "]")) does this without allocation.
There was a problem hiding this comment.
But the result is needed as a string anyway, so until there is range support for writing headers, it wouldn't really save anything.
There was a problem hiding this comment.
format uses an appender internally and thus is probably a tiny bit faster, but yeah I agree that it won't be noticeable and the the real gain will come from not allocating at all.
There was a problem hiding this comment.
AFAIR, multiple ~ expressions are automatically joined efficiently by counting the total length first and allocating only a single buffer. I never verified this, though.
There was a problem hiding this comment.
AFAIR, multiple ~ expressions are automatically joined efficiently by counting the total length first and allocating only a single buffer.
Yes (https://github.com/dlang/druntime/blob/master/src/rt/lifetime.d#L2217), but it's still an allocation.
There was a problem hiding this comment.
But Appender has to allocate, too. Or do you mean something else?
There was a problem hiding this comment.
Yes, but my point was it has to do so in any case at the moment and one big allocation might be a tiny bit faster.
Anyhow, I doubt that it will matter and I wouldn't be too shocked if at the moment the implementation of format would even be slower due to the non-zero cost abstraction of ranges , simply not calling reserve on the appender, the appender itself being slow etc. . Unfortunately, Phobos does a really poor job performance-wise at the moment :/
There was a problem hiding this comment.
I don't get it :) So _d_arraycatnTX does a single big allocation, too (actually an allocation with the exact length, while Appender probably over-allocates?). Why would it be slower than Appender?
There was a problem hiding this comment.
Yeah, but anyway, it's not really important right now. Just the new vibe-http should get a low level API that avoids these kinds of allocations.
There was a problem hiding this comment.
I don't get it :) So _d_arraycatnTX does a single big allocation, too (actually an allocation with the exact length, while Appender probably over-allocates?). Why would it be slower than Appender?
Ah I think we were talking about different things.
I was trying to say that the combination of e.g.
auto s = a ~ "foo" ~ b;
return format("%s%s", ".", s); // <- allocates again with an appender
in a perfect world is slower than
auto s = chain(a, "foo", b);
return format("%s%s", ".", s);
Yeah, but anyway, it's not really important right now.
Yes!
Travis-CI currently does not provide an IPv6 loopback adapter.
s-ludwig
force-pushed
the
issue2080_http_client_ipv6
branch
from
fb948a2
to
bb13ea5
Compare
No description provided.