Thread: Re: [PERFORM] insert performance for win32

Re: [PERFORM] insert performance for win32

From
Tom Lane
Date:
"Merlin Moncure" <merlin.moncure@rcsonline.com> writes:
> Nailed it.

> problem is in mainloop.c -> setup_cancel_handler.  Apparently you can
> have multiple handlers and windows keeps track of them all, even if they
> do the same thing.  Keeping track of so many system handles would
> naturally slow the whole process down.

Yipes.  So we really want to do that only once.

AFAICS it is appropriate to move the sigsetjmp and setup_cancel_handler
calls in front of the per-line loop inside MainLoop --- can anyone see
a reason not to?

I'm inclined to treat this as an outright bug, not just a minor
performance issue, because it implies that a sufficiently long psql
script would probably crash a Windows machine.

            regards, tom lane

Re: [PERFORM] insert performance for win32

From
David Fetter
Date:
On Fri, Nov 04, 2005 at 01:01:20PM -0500, Tom Lane wrote:
> "Merlin Moncure" <merlin.moncure@rcsonline.com> writes:
> > Nailed it.
> 
> > problem is in mainloop.c -> setup_cancel_handler.  Apparently you
> > can have multiple handlers and windows keeps track of them all,
> > even if they do the same thing.  Keeping track of so many system
> > handles would naturally slow the whole process down.
> 
> Yipes.  So we really want to do that only once.
> 
> AFAICS it is appropriate to move the sigsetjmp and
> setup_cancel_handler calls in front of the per-line loop inside
> MainLoop --- can anyone see a reason not to?
> 
> I'm inclined to treat this as an outright bug, not just a minor
> performance issue, because it implies that a sufficiently long psql
> script would probably crash a Windows machine.

Ouch.  In light of this, are we *sure* what we've got a is a candidate
for release?

Cheers,
D
-- 
David Fetter david@fetter.org http://fetter.org/
phone: +1 510 893 6100   mobile: +1 415 235 3778

Remember to vote!


Re: [PERFORM] insert performance for win32

From
Bruce Momjian
Date:
Tom Lane wrote:
> "Merlin Moncure" <merlin.moncure@rcsonline.com> writes:
> > Nailed it.
>
> > problem is in mainloop.c -> setup_cancel_handler.  Apparently you can
> > have multiple handlers and windows keeps track of them all, even if they
> > do the same thing.  Keeping track of so many system handles would
> > naturally slow the whole process down.
>
> Yipes.  So we really want to do that only once.
>
> AFAICS it is appropriate to move the sigsetjmp and setup_cancel_handler
> calls in front of the per-line loop inside MainLoop --- can anyone see
> a reason not to?

Nope.

> I'm inclined to treat this as an outright bug, not just a minor
> performance issue, because it implies that a sufficiently long psql
> script would probably crash a Windows machine.

Agreed.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: [PERFORM] insert performance for win32

From
Tom Lane
Date:
David Fetter <david@fetter.org> writes:
> On Fri, Nov 04, 2005 at 01:01:20PM -0500, Tom Lane wrote:
>> I'm inclined to treat this as an outright bug, not just a minor
>> performance issue, because it implies that a sufficiently long psql
>> script would probably crash a Windows machine.

> Ouch.  In light of this, are we *sure* what we've got a is a candidate
> for release?

Sure.  This problem exists in 8.0.* too.  Pre-existing bugs don't
disqualify an RC in my mind --- we fix them and move on, same as we
would do at any other time.

            regards, tom lane

Re: [PERFORM] insert performance for win32

From
Bruce Momjian
Date:
David Fetter wrote:
> On Fri, Nov 04, 2005 at 01:01:20PM -0500, Tom Lane wrote:
> > "Merlin Moncure" <merlin.moncure@rcsonline.com> writes:
> > > Nailed it.
> >
> > > problem is in mainloop.c -> setup_cancel_handler.  Apparently you
> > > can have multiple handlers and windows keeps track of them all,
> > > even if they do the same thing.  Keeping track of so many system
> > > handles would naturally slow the whole process down.
> >
> > Yipes.  So we really want to do that only once.
> >
> > AFAICS it is appropriate to move the sigsetjmp and
> > setup_cancel_handler calls in front of the per-line loop inside
> > MainLoop --- can anyone see a reason not to?
> >
> > I'm inclined to treat this as an outright bug, not just a minor
> > performance issue, because it implies that a sufficiently long psql
> > script would probably crash a Windows machine.
>
> Ouch.  In light of this, are we *sure* what we've got a is a candidate
> for release?

Good point.  It is something we would fix in a minor release, so it
doesn't seem worth doing another RC just for that.

--
  Bruce Momjian                        |  http://candle.pha.pa.us
  pgman@candle.pha.pa.us               |  (610) 359-1001
  +  If your life is a hard drive,     |  13 Roberts Road
  +  Christ can be your backup.        |  Newtown Square, Pennsylvania 19073

Re: [PERFORM] insert performance for win32

From
Simon Riggs
Date:
On Fri, 2005-11-04 at 13:21 -0500, Bruce Momjian wrote:
> David Fetter wrote:
> > On Fri, Nov 04, 2005 at 01:01:20PM -0500, Tom Lane wrote:
> > > I'm inclined to treat this as an outright bug, not just a minor
> > > performance issue, because it implies that a sufficiently long psql
> > > script would probably crash a Windows machine.
> >
> > Ouch.  In light of this, are we *sure* what we've got a is a candidate
> > for release?
>
> Good point.  It is something we would fix in a minor release, so it
> doesn't seem worth doing another RC just for that.

Will this be documented in the release notes? If we put unimplemented
features in TODO, where do we list things we regard as bugs?

Best Regards, Simon Riggs


Re: [PERFORM] insert performance for win32

From
Andrew Dunstan
Date:

Simon Riggs wrote:

>On Fri, 2005-11-04 at 13:21 -0500, Bruce Momjian wrote:
>  
>
>>David Fetter wrote:
>>    
>>
>>>On Fri, Nov 04, 2005 at 01:01:20PM -0500, Tom Lane wrote:
>>>      
>>>
>>>>I'm inclined to treat this as an outright bug, not just a minor
>>>>performance issue, because it implies that a sufficiently long psql
>>>>script would probably crash a Windows machine.
>>>>        
>>>>
>>>Ouch.  In light of this, are we *sure* what we've got a is a candidate
>>>for release?
>>>      
>>>
>>Good point.  It is something we would fix in a minor release, so it
>>doesn't seem worth doing another RC just for that.
>>    
>>
>
>Will this be documented in the release notes? If we put unimplemented
>features in TODO, where do we list things we regard as bugs?
>
>
>  
>

No need, I think. It was patched 2 days ago.

cheers

andrew