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 commi
BTW, expanding on Andrew's comment, ISTM we could move the kernel call
out of the macro, ie make it look like
#define CHECK_FOR_INTERRUPTS() \
do { \
if (UNBLOCKED_SIGNAL_QUEUE()) \
pgwin32_check_queued_signals(); \
if (InterruptPending) \
ProcessInterrupts(); \
} while(0)
"Magnus Hagander" <[EMAIL PROTECTED]> writes:
> Was that with the volatile attribute or not?
I doubt volatile would make any visible performance difference --- the
CHECK_FOR_INTERRUPTS calls that are performance-critical are in places
where the compiler couldn't try to optimize away the fetches an
>
> Wow, that's just great!
>
> Was that with the volatile attribute or not?
>
> //Magnus
not. However (I'm assuming) this should not greatly impact things.
Good work. QQ: Can you fix the patch? I'm done till Monday.
Merlin
---(end of broadcast)-
> > Hm, what were the tests exactly? Offhand I'd expect
> something like a
> > SELECT COUNT(*) on a large but not-too-wide table to show
> noticeable
> > improvement.
> >
> > regards, tom lane
> I STAND CORRECTED! My tests were high volume record by
> record iterators, e
"Merlin Moncure" <[EMAIL PROTECTED]> writes:
> I STAND CORRECTED! My tests were high volume record by record
> iterators, etc. Read and drool, gentlemen.
Looks good to me ;-) ...
If I recall the bidding correctly, the original patch needs DLLIMPORT
qualifiers attached to both of the variables,
> "Merlin Moncure" <[EMAIL PROTECTED]> writes:
> Hm, what were the tests exactly? Offhand I'd expect something like a
> SELECT COUNT(*) on a large but not-too-wide table to show noticeable
> improvement.
>
> regards, tom lane
I STAND CORRECTED! My tests were high volume rec
"Merlin Moncure" <[EMAIL PROTECTED]> writes:
>> Will have performance #s up in a bit.
> I have a couple of cpu-bound performance tests that I just ran with and
> without the patch. Everything is ran with n=1 until volatile issue is
> sorted out but so far I am not seeing any performance
> improve
On Fri, 21 Oct 2005, Magnus Hagander wrote:
> > > Shall we add "volatile" quanlifier to at least pg_signal_queue?
> >
> > If that's changed by a separate thread, "volatile" seems essential.
> > What about the mask variable?
>
> Yes, that does seem right. Previously it would never be concurrently
> > > I can't get the postgres to link with the patch...
> > > Am I missing something?
> > > Merlin
> > >
> > False alarm. I had to rerun configure which copies win32.h in
various
> > places, as Qingqing noted.
> >
> Not false alarm :) Only with DLLIMPORT can I link all the libraries.
> Will have
> > Shall we add "volatile" quanlifier to at least pg_signal_queue?
>
> If that's changed by a separate thread, "volatile" seems essential.
> What about the mask variable?
Yes, that does seem right. Previously it would never be concurrently
modified, because it was always locked by the critical s
"Qingqing Zhou" <[EMAIL PROTECTED]> writes:
> Shall we add "volatile" quanlifier to at least pg_signal_queue?
If that's changed by a separate thread, "volatile" seems essential.
What about the mask variable?
regards, tom lane
---(end of broadcast)-
> > I can't get the postgres to link with the patch...
> > Am I missing something?
> > Merlin
> >
> False alarm. I had to rerun configure which copies win32.h in various
> places, as Qingqing noted.
>
Not false alarm :) Only with DLLIMPORT can I link all the libraries.
Will have performance #s u
> I can't get the postgres to link with the patch...
> Am I missing something?
> Merlin
>
False alarm. I had to rerun configure which copies win32.h in various
places, as Qingqing noted.
Merlin
---(end of broadcast)---
TIP 6: explain analyze is yo
"Tom Lane" <[EMAIL PROTECTED]> wrote
>
> ... so definitely worth fixing for 8.1 if we can convince ourselves
> it's correct.
>
Despite the performance, there is one thing I am not exactly sure. Shall we
add "volatile" quanlifier to at least pg_signal_queue? The dangerous place
is PGSemaphoreLoc
"Merlin Moncure" <[EMAIL PROTECTED]> writes:
> I can't get the postgres to link with the patch...
> Am I missing something?
Probably those variables need "extern DLLIMPORT".
regards, tom lane
---(end of broadcast)---
TIP 1:
I can't get the postgres to link with the patch...
Am I missing something?
Merlin
Info: resolving _pg_signal_mask by linking to __imp__pg_signal_mask
(auto-import)
Info: resolving _pg_signal_queue by linking to __imp__pg_signal_queue
(auto-import)
fu01.o(.idata$3+0xc): undefined reference to
"Magnus Hagander" <[EMAIL PROTECTED]> writes:
> Do you have any numbers on how much performance increases with it?
A rough estimate is that it would cost more than half as much as EXPLAIN
ANALYZE does: that imposes two extra syscalls per ExecProcNode, while
this adds one. There are other high-fre
> > This patch improves the win32 CHECK_FOR_INTERRUPTS() performance by
> > testing if any unblocked signals are queued before check
> > pgwin32_signal_event. This avoids an unnecessary system call.
>
> http://archives.postgresql.org/pgsql-patches/2005-10/msg00191.php
>
> This looks to me like
Qingqing Zhou <[EMAIL PROTECTED]> writes:
> This patch improves the win32 CHECK_FOR_INTERRUPTS() performance by
> testing if any unblocked signals are queued before check
> pgwin32_signal_event. This avoids an unnecessary system call.
http://archives.postgresql.org/pgsql-patches/2005-10/msg00191.p
20 matches
Mail list logo
