Re: [PATCHES] Win32 CHECK_FOR_INTERRUPTS() performance

And the patch that was applied gives the same result.

What is more, I am not seeing the reported speedup - in fact I am seeing
no speedup worth mentioning.

This is on XP-Pro, with default postgres settings. The test sets were
somewhat larger than thos Magnus used - basically TPC-H lineitems and
orders tables (6m and 1.5m rows respectively).

cheers

andrew

Andrew Dunstan wrote:

>
> Heads up - I have seen 2 regression hangs. I am checking further. Has
> anyone else run the regression suite with any version of this patch?
>
> cheers
>
> andrew
>
> Tom Lane wrote:
>
>> I wrote:
>>
>>
>>> BTW, expanding on Andrew's comment, ISTM we could move the kernel call
>>> out of the macro, ie make it look like ...
>>>
>>
>>
>> I've applied the attached version of Qingqing's revised patch. I'm not
>> in a position to test it, and am about to go out for the evening, but if
>> anyone can check the committed version soon and verify that I didn't
>> break anything ...
>>
>> (BTW, DLLIMPORT is only needed in extern declarations, not in the actual
>> definition of the variable.)
>>
>> regards, tom lane
>>
>> *** src/backend/port/win32/signal.c.orig Fri Oct 14 22:59:56 2005
>> --- src/backend/port/win32/signal.c Fri Oct 21 17:36:24 2005
>> ***************
>> *** 15,32 ****
>>
>> #include <libpq/pqsignal.h>
>>
>> ! ! /* pg_signal_crit_sec is used to protect only pg_signal_queue.
>> That is the only
>> ! * variable that can be accessed from the signal sending threads! */
>> static CRITICAL_SECTION pg_signal_crit_sec;
>> - static int pg_signal_queue;
>>
>> static pqsigfunc pg_signal_array[PG_SIGNAL_COUNT];
>> static pqsigfunc pg_signal_defaults[PG_SIGNAL_COUNT];
>> - static int pg_signal_mask;
>> - - DLLIMPORT HANDLE pgwin32_signal_event;
>> - HANDLE pgwin32_initial_signal_pipe = INVALID_HANDLE_VALUE;
>>
>>
>> /* Signal handling thread function */
>> --- 15,40 ----
>>
>> #include <libpq/pqsignal.h>
>>
>> ! /*
>> ! * These are exported for use by the UNBLOCKED_SIGNAL_QUEUE() macro.
>> ! * pg_signal_queue must be volatile since it is changed by the signal
>> ! * handling thread and inspected without any lock by the main thread.
>> ! * pg_signal_mask is only changed by main thread so shouldn't need it.
>> ! */
>> ! volatile int pg_signal_queue;
>> ! int pg_signal_mask;
>> ! ! HANDLE pgwin32_signal_event;
>> ! HANDLE pgwin32_initial_signal_pipe = INVALID_HANDLE_VALUE;
>> ! ! /*
>> ! * pg_signal_crit_sec is used to protect only pg_signal_queue. That
>> is the only
>> ! * variable that can be accessed from the signal sending threads!
>> ! */
>> static CRITICAL_SECTION pg_signal_crit_sec;
>>
>> static pqsigfunc pg_signal_array[PG_SIGNAL_COUNT];
>> static pqsigfunc pg_signal_defaults[PG_SIGNAL_COUNT];
>>
>>
>> /* Signal handling thread function */
>> ***************
>> *** 81,101 ****
>> (errmsg_internal("failed to set console control
>> handler")));
>> }
>>
>>
>> ! /* Dispatch all signals currently queued and not blocked
>> * Blocked signals are ignored, and will be fired at the time of
>> ! * the sigsetmask() call. */
>> void
>> pgwin32_dispatch_queued_signals(void)
>> {
>> int i;
>>
>> EnterCriticalSection(&pg_signal_crit_sec);
>> ! while (pg_signal_queue & ~pg_signal_mask)
>> {
>> /* One or more unblocked signals queued for execution */
>> ! ! int exec_mask = pg_signal_queue &
>> ~pg_signal_mask;
>>
>> for (i = 0; i < PG_SIGNAL_COUNT; i++)
>> {
>> --- 89,119 ----
>> (errmsg_internal("failed to set console control
>> handler")));
>> }
>>
>> + /*
>> + * Support routine for CHECK_FOR_INTERRUPTS() macro
>> + */
>> + void
>> + pgwin32_check_queued_signals(void)
>> + {
>> + if (WaitForSingleObjectEx(pgwin32_signal_event, 0, TRUE) ==
>> WAIT_OBJECT_0)
>> + pgwin32_dispatch_queued_signals();
>> + }
>>
>> ! /*
>> ! * Dispatch all signals currently queued and not blocked
>> * Blocked signals are ignored, and will be fired at the time of
>> ! * the sigsetmask() call.
>> ! */
>> void
>> pgwin32_dispatch_queued_signals(void)
>> {
>> int i;
>>
>> EnterCriticalSection(&pg_signal_crit_sec);
>> ! while (UNBLOCKED_SIGNAL_QUEUE())
>> {
>> /* One or more unblocked signals queued for execution */
>> ! int exec_mask = UNBLOCKED_SIGNAL_QUEUE();
>>
>> for (i = 0; i < PG_SIGNAL_COUNT; i++)
>> {
>> *** src/include/miscadmin.h.orig Fri Oct 14 23:00:22 2005
>> --- src/include/miscadmin.h Fri Oct 21 17:36:18 2005
>> ***************
>> *** 83,97 ****
>> if (InterruptPending) \
>> ProcessInterrupts(); \
>> } while(0)
>> #else /* WIN32 */
>>
>> #define CHECK_FOR_INTERRUPTS() \
>> do { \
>> ! if (WaitForSingleObjectEx(pgwin32_signal_event,0,TRUE) ==
>> WAIT_OBJECT_0) \
>> ! pgwin32_dispatch_queued_signals(); \
>> if (InterruptPending) \
>> ProcessInterrupts(); \
>> } while(0)
>> #endif /* WIN32 */
>>
>>
>> --- 83,99 ----
>> if (InterruptPending) \
>> ProcessInterrupts(); \
>> } while(0)
>> + #else /* WIN32 */
>>
>> #define CHECK_FOR_INTERRUPTS() \
>> do { \
>> ! if (UNBLOCKED_SIGNAL_QUEUE()) \
>> ! pgwin32_check_queued_signals(); \
>> if (InterruptPending) \
>> ProcessInterrupts(); \
>> } while(0)
>> + #endif /* WIN32 */
>>
>>
>> *** src/include/port/win32.h.orig Fri Oct 14 23:00:30 2005
>> --- src/include/port/win32.h Fri Oct 21 17:36:12 2005
>> ***************
>> *** 214,224 ****
>>
>>
>> /* In backend/port/win32/signal.c */
>> ! extern DLLIMPORT HANDLE pgwin32_signal_event;
>> extern HANDLE pgwin32_initial_signal_pipe;
>>
>> void pgwin32_signal_initialize(void);
>> HANDLE pgwin32_create_signal_listener(pid_t pid);
>> void pgwin32_dispatch_queued_signals(void);
>> void pg_queue_signal(int signum);
>>
>> --- 214,230 ----
>>
>>
>> /* In backend/port/win32/signal.c */
>> ! extern DLLIMPORT volatile int pg_signal_queue;
>> ! extern DLLIMPORT int pg_signal_mask;
>> ! extern HANDLE pgwin32_signal_event;
>> extern HANDLE pgwin32_initial_signal_pipe;
>>
>> + #define UNBLOCKED_SIGNAL_QUEUE() (pg_signal_queue &
>> ~pg_signal_mask)
>> + + void pgwin32_signal_initialize(void);
>> HANDLE pgwin32_create_signal_listener(pid_t pid);
>> + void pgwin32_check_queued_signals(void);
>> void pgwin32_dispatch_queued_signals(void);
>> void pg_queue_signal(int signum);
>>
>>
>> ---------------------------(end of broadcast)---------------------------
>> TIP 9: In versions below 8.0, the planner will ignore your desire to
>> choose an index scan if your joining column's datatypes do not
>> match
>>
>>
>>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 2: Don't 'kill -9' the postmaster
>