Re: [HACKERS] Current Win32 port status

> Bruce Momjian wrote:
> > > * a workable pipe replacement
> >
> > I don't have 'pipe' mentioned on the win32 patch. Can you
> > give details?
>
> Yeah you do. The second point under "Problems with select()".
>
> Basically, the Win32 call to pipe() returns a file descriptor
> which is invalid to pass on to Win32 select() (as it only
> takes socket handles).
>
> So, we need to replace the select'ing mechanism under Win32
> (yech), or write a Win32 pipe() replacement that returns two
> socket endpoints (good enough for our purposes), or something else...

I think you want to be investigating
WSAEventSelect() and then WaitForMultipleObjectsEx().

WSAEventSelect() claims it needs a WSAEVENT, but according to docs
otherwhere it should accept a standard event handle on NT+ platforms.

WaitForMultiple... will accept pipes, events, anything. (The Ex function
will also allow dispatching of user APCs, see related discussion about
signals)

//Magnus

Magnus Hagander wrote:

>>Bruce Momjian wrote:
>>
>>
>>>> * a workable pipe replacement
>>>>
>>>>
>>>I don't have 'pipe' mentioned on the win32 patch. Can you
>>>give details?
>>>
>>>
>>Yeah you do. The second point under "Problems with select()".
>>
>>Basically, the Win32 call to pipe() returns a file descriptor
>>which is invalid to pass on to Win32 select() (as it only
>>takes socket handles).
>>
>>So, we need to replace the select'ing mechanism under Win32
>>(yech), or write a Win32 pipe() replacement that returns two
>>socket endpoints (good enough for our purposes), or something else...
>>
>>
>
>I think you want to be investigating
>WSAEventSelect() and then WaitForMultipleObjectsEx().
>
>WSAEventSelect() claims it needs a WSAEVENT, but according to docs
>otherwhere it should accept a standard event handle on NT+ platforms.
>
>WaitForMultiple... will accept pipes, events, anything. (The Ex function
>will also allow dispatching of user APCs, see related discussion about
>signals)
>
>
>

Using a socket or a pair of sockets is a very common practice in porting
this sort of code from Unix to Windows. IIRC this is what Cygwin does
under the hood.

That would help to preserve the programming paradigms already in use in
Postgres. If it proves to be a performance bottleneck then it could be
revisited, but it seems unlikely.

Tom (rightly) admonished me not long ago that we do not need to use
every last part of the Unix API in our code. The same goes a fortiori
for the Windows API, IMNSHO. Minimal disturbance and acceptable
performance should be the initial goals.

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> Using a socket or a pair of sockets is a very common practice in porting
> this sort of code from Unix to Windows. IIRC this is what Cygwin does
> under the hood.

> That would help to preserve the programming paradigms already in use in
> Postgres. If it proves to be a performance bottleneck then it could be
> revisited, but it seems unlikely.

AFAIR there is no place in Postgres where performance of a pipe
connection is critical. Don't go out of your way to make it fast.

In fact, right offhand I only see two pipes used at all in the source
code: they are both in pgstat.c. It's fairly likely that that could be
redesigned if it poses a problem on Windows. (One of the pipes never
even transports any data; it's only used as a cheap-and-dirty means of
letting the statistics subprocess detect postmaster exit.)

regards, tom lane

Tom Lane wrote:

>
>AFAIR there is no place in Postgres where performance of a pipe
>connection is critical. Don't go out of your way to make it fast.
>
>In fact, right offhand I only see two pipes used at all in the source
>code: they are both in pgstat.c. It's fairly likely that that could be
>redesigned if it poses a problem on Windows. (One of the pipes never
>even transports any data; it's only used as a cheap-and-dirty means of
>letting the statistics subprocess detect postmaster exit.)
>
>
>

You are correct - I noticed that. Also, the only places in the backend
where we seem to use select() on FDs are in pgstat.c and postmaster.c,
and in the latter case the FDs *are* sockets, so we won't have a problem
there on Windows. There are a couple of other places where it is used
for small sleeps (storage/lmgr/s_lock.c and access/transam/xact.c) -
those should possibly be abstracted out (Windows doesn't behave well
there anyway, I believe - with 0 FDs I read somewhere it returns
immediately regardless of the timeout setting).

Bottom line: this should be a very small nut to crack.

cheers

andrew

I wrote:

>
> There are a couple of other places where [select()] is used for small
> sleeps (storage/lmgr/s_lock.c and access/transam/xact.c) - those
> should possibly be abstracted out (Windows doesn't behave well there
> anyway, I believe - with 0 FDs I read somewhere it returns immediately
> regardless of the timeout setting).
>

What is the preferred way to handle these 2 cases? We could handle them
with #ifdef'd code inline, or create a new function pg_usleep(), or
possibly handle it with conditional macros inline. If a new function or
macro, where should they go?

cheers

andrew

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> I wrote:
>> There are a couple of other places where [select()] is used for small
>> sleeps (storage/lmgr/s_lock.c and access/transam/xact.c) -

> What is the preferred way to handle these 2 cases? We could handle them
> with #ifdef'd code inline, or create a new function pg_usleep(), or
> possibly handle it with conditional macros inline. If a new function or
> macro, where should they go?

I'd go with a new function. There is no reason to try to "optimize"
this code by putting it inline; if you're trying to delay, another few
nanoseconds to enter a subroutine doesn't matter.

As for where, maybe make a new file in src/port/. That would make it
relatively easy to use the same function in client-side code if we
needed to.

regards, tom lane

Tom Lane wrote:

>Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>
>
>>I wrote:
>>
>>
>>>There are a couple of other places where [select()] is used for small
>>>sleeps (storage/lmgr/s_lock.c and access/transam/xact.c) -
>>>
>>>
>
>
>
>>What is the preferred way to handle these 2 cases? We could handle them
>>with #ifdef'd code inline, or create a new function pg_usleep(), or
>>possibly handle it with conditional macros inline. If a new function or
>>macro, where should they go?
>>
>>
>
>I'd go with a new function. There is no reason to try to "optimize"
>this code by putting it inline; if you're trying to delay, another few
>nanoseconds to enter a subroutine doesn't matter.
>
>As for where, maybe make a new file in src/port/. That would make it
>relatively easy to use the same function in client-side code if we
>needed to.
>
>
>
patch + new file attached. Haven't tested on Windows, but should be fine.

cheers

andrew