Re: Performance degradation in commit ac1d794

From: Andres Freund <andres(at)anarazel(dot)de>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Васильев Дмитрий <d(dot)vasilyev(at)postgrespro(dot)ru>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Performance degradation in commit ac1d794
Date: 2016-03-17 14:53:26
Message-ID: 20160317145326.jyec5bfui6zaehlo@alap3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2016-03-17 09:40:08 -0400, Robert Haas wrote:
> On Thu, Mar 17, 2016 at 9:01 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> > I'll look at 0005 next, but thought I would send these comments along first.
>
> 0005: This is obviously very much WIP, but I think the overall
> direction of it is good.
> 0006: Same.
>
> I think you should use PGINVALID_SOCKET rather than -1 in various
> places in various patches in this series, especially if you are going
> to try to merge the Windows code path.

Sure.

> I wonder if CreateEventSet should accept a MemoryContext argument. It
> seems like callers will usually want TopMemoryContext, and just being
> able to pass that might be more convenient than having to switch back
> and forth in the calling code.

Makes sense.

> I wonder if there's a way to refactor this code to avoid having so
> much cut-and-paste duplication.

I guess you mean WaitEventSetWait() and WaitEventAdjust*? I've tried,
and my attempt ended up look nearly unreadable, because of the number of
ifdefs. I've not found a good attempt. Which is sad, because adding back
select support is going to increase the duplication further :( - but
it's also further away from poll etc. (different type of timestamp,
entirely different way of returming events).

> When iterating over the returned events, maybe check whether events is
> 0 at the top of the loop and skip it forthwith if so.

You mean in WaitEventSetWait()? There's
else if (rc == 0)
{
break;
}
which is the timeout case. There should never be any other case of
returning 0 elements?

> That's all I've got for now.

Thanks for looking.

Greetings,

Andres Freund

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2016-03-17 14:57:04 Re: Performance degradation in commit ac1d794
Previous Message Robert Haas 2016-03-17 14:10:25 Re: RFC: replace pg_stat_activity.waiting with something more descriptive