Re: Win32 signal code - first try

Lists: pgsql-hackers-win32
From: "Magnus Hagander" <mha(at)sollentuna(dot)net>
To: "Merlin Moncure" <merlin(dot)moncure(at)rcsonline(dot)com>
Cc: "pgsql-hackers-win32" <pgsql-hackers-win32(at)postgresql(dot)org>
Subject: Re: Win32 signal code - first try
Date: 2004-01-08 21:56:38
Message-ID: 6BCB9D8A16AC4241919521715F4D8BCE2A6A5F@algol.sollentuna.se
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

>> Here is a first sketch at Win32 signal handling. First a couple of
>> comments:
>
>Couple of quick questions/points regarding your implementation:
>
>1. Fully support your decision to use named pipes.
>
>2. __pg_signal_count is guarded by a CriticalSection. Are
>Interlocked{Exchange / Increment} still necessary?

If we only used it inside the CriticalSection, they would be
unnecessary. But we also check the variable outside the critical section
in the polling function.

>3. you are absolutely certain that
>__pg_poll_signals() +
>EnterCriticalSection +
>memset +
>if... +
>LeaveCriticalSection +
>if...
>
>is more efficient than a single call to WaitForSingleObjectEx() with 0
>timeout?
Almost :-) I remember doing some tests no this earlier, but I have no
numbers around.

>> * Does *not* use user APCs. Why? Well, we had to use
>polling. And with
>> user APCs we'd have to go into kernel mode (however briefly) on every
>> poll. I replaced that with a simple counter that is checked.
>Thast way
>> we don't slow down the main path of the program much.
>
>Have you have given up on using a kernel mode driver to throw a thread
>into alertable state?

I think we agreed that we'd go with the polling method if it worked well
enough, so we don't need a kernel driver. If that doesn't work out, the
kernel driver would be the fallback method.

From your other mail:
> Some crude tests show that my 1 GHz P3 can execute about 1.2 million
> calls to WaitForSingleObjectEx() in 1 second. However, this is about
> 3.5 times slower than a quick mock up of your polling function I put
> together. Either approach, though, is pretty darn quick :). Unlike
> SleepEx(), WFSO does not stall the thread.

Hmm. Depending on how often we need to poll (meaning how often we need
to deliver signals), perhaps we can go with the WFSOEx method anyway.
The code would be slightly easier:
I've attached a version that uses this one instead.

(You'd probably move the WaitFor()... call into the #define as well)

Looking at this code, I'm thinking we can probably do away with the
critical section alltogether. All that code now executes on the main
thread. Does this seem correct?

//Magnus

Attachment Content-Type Size
pgsignal_w32_apc.c application/octet-stream 4.9 KB

From: Andrew Dunstan <andrew(at)dunslane(dot)net>
To: pgsql-hackers-win32 <pgsql-hackers-win32(at)postgresql(dot)org>
Subject: Re: Win32 signal code - first try
Date: 2004-01-08 23:38:00
Message-ID: 3FFDE9D8.5020408@dunslane.net
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers-win32

Magnus Hagander wrote:

>I think we agreed that we'd go with the polling method if it worked well
>enough, so we don't need a kernel driver. If that doesn't work out, the
>kernel driver would be the fallback method.
>

I think your code has almost certainly ruled out any necessity for a
kernel mode driver.

>
>Hmm. Depending on how often we need to poll (meaning how often we need
>to deliver signals), perhaps we can go with the WFSOEx method anyway.
>The code would be slightly easier:
>I've attached a version that uses this one instead.
>
>(You'd probably move the WaitFor()... call into the #define as well)
>
>Looking at this code, I'm thinking we can probably do away with the
>critical section alltogether. All that code now executes on the main
>thread. Does this seem correct?
>
>
>

I understood your first version better than I understand this one. What
calls __pg_poll_signals()? As I understand the first version, we
wouldn't need to put any polling calls into the main thread code - the
signal detector would just queue a call to pg_signal_apc() on the main
thread as needed, which would in turn do some cleanup and call the
signal handler. That seems to me to be *very* clean and nice. Am I
missing something? (As you can no doubt tell, IANAWP :-) )

BTW, well done!

cheers

andrew