On Thu, Oct 31, 2013 at 11:20 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Amit Kapila <amit.kapila16@gmail.com> writes: > On Thu, Oct 31, 2013 at 2:41 AM, Gurjeet Singh <gurjeet@singh.im> wrote: >> Just a small patch; hopefully useful.
> This is valid saving as we are filling array ListenSocket[] in > StreamServerPort() serially, so during ClosePostmasterPorts() once if > it encountered PGINVALID_SOCKET, it is valid to break the loop. > Although savings are small considering this doesn't occur in any > performance path, still I think this is right thing to do in code.
> It is better to register this patch in CF app list, unless someone > feels this is not right.
I think this is adding fragility for absolutely no meaningful savings. The existing code does not depend on the assumption that the array is filled consecutively and no entries are closed early. Why should we add such an assumption here?
IMHO, typical configurations have one, or maybe two of these array elements populated; one for TCP 5432 (for localhost or *), and the other for Unix Domain Sockets. Saving 62 iterations and comparisons in startup sequence may not be much, but IMHO it does match logic elsewhere.
In fact, this was inspired by the following code in ServerLoop():
ServerLoop() { ... /* * New connection pending on any of our sockets? If so, fork a child * process to deal with it. */ if (selres > 0) { int i;
for (i = 0; i < MAXLISTEN; i++) { if (ListenSocket[i] == PGINVALID_SOCKET) break; if (FD_ISSET(ListenSocket[i], &rmask)) {
And looking for other precedences, I found that initMasks() function also relies on the array being filled consecutively and the first PGINVALID_SOCKET marks end of valid array members.
