Thread: Re: [PATCHES] Win32 CHECK_FOR_INTERRUPTS() performance
OK, running the latest patch. Observations: 1. Confirmed that time for count(*) on narrow sets is greatly improved: A real easy way to show this off is: select count(*) from generate_series(1,(10^6)::integer); with about a 60% drop in time on my XP box. 2. Did ISAM style record iteration over relatively large (50k records) bill of materials. This was previous test where I reported no performance gain...for a single user (n=1). For n=6, I am seeing about a 20% reduction in the overall running time of the test, and a greatly improved overall responsiveness of the server while running the test. I tested this by doing select * from medium_sized_table while 6 backends were busy feeding records to the ISAM app. While pg under 0 load might serve the table in one second, with the n=6 test running the time might (on win32) take 5 minutes. On linux, the worst case time was around 1/(n*2). Qingqing's patch brings win32 much closer to linux! 3. A pl/pgsql function stuck in a empty loop is unkillable except by killing the process on the server, which cycles the entire server. This was the behavior before the patch, btw. I ran tests for about an hour, randomly killing/canceling backends without any problems. Merlin
"Merlin Moncure" <merlin.moncure@rcsonline.com> writes:
> 3. A pl/pgsql function stuck in a empty loop is unkillable except by
> killing the process on the server, which cycles the entire server. This
> was the behavior before the patch, btw.
Hmm, that suggests we need another CHECK_FOR_INTERRUPTS somewhere in
plpgsql. Please show the exact test case you were using.
regards, tom lane
Tom Lane wrote: >"Merlin Moncure" <merlin.moncure@rcsonline.com> writes: > > >>3. A pl/pgsql function stuck in a empty loop is unkillable except by >>killing the process on the server, which cycles the entire server. This >>was the behavior before the patch, btw. >> >> > >Hmm, that suggests we need another CHECK_FOR_INTERRUPTS somewhere in >plpgsql. Please show the exact test case you were using. > > We might be able to solve that for plpgsql, but in general we can't, ISTM. What if I write a plperl function that loops forever? We have no chance there to call CHECK_FOR_INTERRUPTS. cheers andrew
Andrew Dunstan <andrew@dunslane.net> writes:
> We might be able to solve that for plpgsql, but in general we can't,
> ISTM. What if I write a plperl function that loops forever? We have no
> chance there to call CHECK_FOR_INTERRUPTS.
Yeah, I was thinking about that too. Still, we can/should/already did
fix it in plpgsql.
regards, tom lane
"Merlin Moncure" <merlin.moncure@rcsonline.com> writes: > OK, running the latest patch. Observations: > ... > I ran tests for about an hour, randomly killing/canceling backends > without any problems. Are we all comfortable that http://archives.postgresql.org/pgsql-hackers/2005-10/msg01009.php is OK to apply? regards, tom lane
On Mon, 24 Oct 2005, Tom Lane wrote: > > Are we all comfortable that > http://archives.postgresql.org/pgsql-hackers/2005-10/msg01009.php > is OK to apply? > > regards, tom lane I tried to persuade myself that removing all WaitForSingleObjectEx() is safe ... the thing is we will false alarm EINTR as Magnus said (details to repeat it are list below in case). There are several EINTR in the code, semop() calls, socket() calls, ..., seems they are all ok except pgwin32_backend_usleep() changes a little bit performance: it can't sleep enough because of the false alarm, but it is ok though. Conclusion: Agree to apply. Regards, Qingqing --- Consider a sequence like this: 1. I am killing you signal A: enter_crit; set signal bit; leave_crit; 2. You CHECK_FOR_INTERRUPTS(): enter_crit; sig(A); ResetEvent(); leave_crit; 3. I finish my killing: SetEvent(); Now the event is signaled but the signal is handled already.
Qingqing Zhou <zhouqq@cs.toronto.edu> writes:
> I tried to persuade myself that removing all WaitForSingleObjectEx() is
> safe ... the thing is we will false alarm EINTR as Magnus said (details to
> repeat it are list below in case).
Just to repeat myself: there were false alarms before. The interleaving
you describe could equally well happen if a new signal is sent just
after the old code executes WaitForSingleObjectEx and sees that a
previous signal is waiting for it. Both old and new signals can be
cleared by the recipient before the second signal sender gets as far as
setting the event.
regards, tom lane
On Mon, 24 Oct 2005, Tom Lane wrote: > Qingqing Zhou <zhouqq@cs.toronto.edu> writes: > > I tried to persuade myself that removing all WaitForSingleObjectEx() is > > safe ... the thing is we will false alarm EINTR as Magnus said (details to > > repeat it are list below in case). > > Just to repeat myself: there were false alarms before. The interleaving > you describe could equally well happen if a new signal is sent just > after the old code executes WaitForSingleObjectEx and sees that a > previous signal is waiting for it. Both old and new signals can be > cleared by the recipient before the second signal sender gets as far as > setting the event. > Oh, yeah. Just write the detailed case down for the sake of memory: -- For previous code -- false alarm case -- 1. I am killing you signal A: enter_crit; set signal bit; leave_crit; 2. He *has killed* you signal B: 3. You CHECK_FOR_INTERRUPTS(): enter_crit; sig(A); sig(B); ResetEvent(); leave_crit; 4. I finish my killing: SetEvent(); Now the event is signaled but the signal is handled already. Regards, Qingqing