Re: [PATCHES] [HACKERS] Possible explanation for Win32 stats regression

2006-07-29 Thread Andrew Dunstan

[EMAIL PROTECTED] wrote:
heh. I was just doing it the way Tom suggested - see attached. With a 
little more trouble we could also keep track if the listened for events 
and sometimes save ourselves a second call to WSAEventSelect, but I'm 
not sure it's worth it.



It all depends on the overhead of WSAEventSelect().  I'm sure your version would run 
faster, but I just don't know if "slower" would be measurable.
  


BTW:  I would suggest changing your comment to:

   /*
* make sure we don't multiplex this kernel event object with a 
different socket

* from a previous call
*/

Thanks for tackling this problem too.



Ok. Applied to HEAD and 8.1 and 8.0 branches.

cheers

andrew

---(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


Re: [PATCHES] [HACKERS] Possible explanation for Win32 stats regression

2006-07-29 Thread Andrew Dunstan


heh. I was just doing it the way Tom suggested - see attached. With a 
little more trouble we could also keep track if the listened for events 
and sometimes save ourselves a second call to WSAEventSelect, but I'm 
not sure it's worth it.


cheers

andrew


[EMAIL PROTECTED] wrote:

Is anyone working on this?

Tom Lane wrote:
> korry <[EMAIL PROTECTED] > writes:
> > The problem is that, each time you go through
> > pgwin32_waitforsinglesocket(), you tie the *same* kernel object
> > (waitevent is static) to each socket.
> 
> > The fix is pretty simple - just call WSAEventSelect( s, waitevent, 0 )

> > after WaitForMultipleObjectsEx() returns.  That disassociates the socket
> > from the Event (it will get re-associated the next time
> > pgwin32_waitforsingleselect() is called.  
> 
> Hmm.  Presumably we don't do this a whole lot (use multiple sockets) or

> we'd have noticed before.  Perhaps better would be to keep an additional
> static variable saying which socket the event is currently associated
> to, and only issue the extra WSAEventSelect calls if we need to change
> it.  Or is WSAEventSelect fast enough that it doesn't matter?
> 



Here's a simple patch that fixes the problem (I haven't explored the 
performance of this patch compared to Tom's suggestion).





Index: backend/port/win32/socket.c
===
RCS file: /cvsroot/pgsql/src/backend/port/win32/socket.c,v
retrieving revision 1.11
diff -c -r1.11 socket.c
*** backend/port/win32/socket.c	5 Mar 2006 15:58:35 -	1.11
--- backend/port/win32/socket.c	29 Jul 2006 14:19:23 -
***
*** 106,111 
--- 106,112 
  pgwin32_waitforsinglesocket(SOCKET s, int what)
  {
  	static HANDLE waitevent = INVALID_HANDLE_VALUE;
+ 	static SOCKET current_socket = -1;
  	HANDLE		events[2];
  	int			r;
  
***
*** 121,126 
--- 122,136 
  		ereport(ERROR,
  (errmsg_internal("Failed to reset socket waiting event: %i", (int) GetLastError(;
  
+ 	/*
+ 	 * make sure we don't multiplex this with a different socket 
+ 	 * from a previous call
+ 	 */
+ 
+ 	if (current_socket != s && current_socket != -1)
+ 		WSAEventSelect(current_socket, waitevent, 0);
+ 
+ 	current_socket = s;
  
  	if (WSAEventSelect(s, waitevent, what) == SOCKET_ERROR)
  	{

---(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