[vlc-devel] Patch: accelerate net_Connect

[Rémi Denis-Courmont]

	Hello,

Le samedi 27 août 2011 03:36:44 Kiran Kotla, vous avez écrit :
>        Here is a patch that makes the net_Connect function connect
> to the fastest server attempting to connect to multiple servers from
> DNS resolution in parallel by making the sockets non-blocking and
> using select to choose the best among them. We call this new
> mechanism 'smart connect'. It also stores the fastest server from the
> previous resolution of a DNS name into a cache (with a configurable
> timeout) and connects to it directly without performing DNS resolution.

Is there any OS that returns more than one addrinfo structure per family per 
protocol? As far as I know, if the DNS query returned multiple addresses in 
the same familty, getaddrinfo() picks one pseudo-randomly. This being the 
case, this patch will basically change the logic to determine which of IPv4 or 
IPv6 is used, right?

That is covered by http://tools.ietf.org/html/draft-ietf-v6ops-happy-eyeballs 
and I suspect your proposed implementation does not adhere to the address 
policy, as required by §4.1. Of course, this is only a draft, but I doubt this 
requirement is ever going to be dropped. Probably the simplest solution is the 
short timeout reportedly implemented by Chrome and Firefox (§6.0).

I am also not sure that caching DNS lookups in VLC is a good idea. VLC does 
not know the lifetime of the DNS responses, so it cannot perform correct cache 
expiration. DNS cache really ought to be maintained by the DNS resolver, and 
indeed some operating systems do have a caching resolver (e.g. nscd on Linux).

(...)
>       The patch file is attached. We would appreciate any comments or
> suggestions on the changes described.

* net_Connect() normally returns sockets in non-blocking mode.

* select() is not used due to design bugs (FD_SET() does not check for buffer 
overflow). poll() is used instead.

* net_Connect() is thread-safe. It cannot use unprotected non-constant global 
data.

* strerror() is forbidden in VLC code base due to not being thread-safe.

* memset() is preferred over bzero().

* Identifiers with leading underscores are deprecated due for conformance with 
the C standard.

-- 
Rémi Denis-Courmont
http://www.remlab.net/
http://fi.linkedin.com/in/remidenis