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
>