| From: | Andrew Dunstan <andrew(at)dunslane(dot)net> | 
|---|---|
| To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> | 
| Cc: | Qingqing Zhou <zhouqq(at)cs(dot)toronto(dot)edu>, Magnus Hagander <mha(at)sollentuna(dot)net>, pgsql-hackers(at)postgresql(dot)org, Merlin Moncure <merlin(dot)moncure(at)rcsonline(dot)com> | 
| Subject: | Re: [PATCHES] Win32 CHECK_FOR_INTERRUPTS() performance | 
| Date: | 2005-10-21 22:21:06 | 
| Message-ID: | 435969D2.4030804@dunslane.net | 
| Views: | Whole Thread | Raw Message | Download mbox | Resend email | 
| Thread: | |
| Lists: | pgsql-hackers | 
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
>
>  
>
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Kevin Brown | 2005-10-22 00:06:45 | Re: Question about Ctrl-C and less | 
| Previous Message | Tom Lane | 2005-10-21 22:09:00 | Re: [COMMITTERS] pgsql: Do all accesses to shared buffer |