On 2014-01-31 17:52:58 -0800, Peter Geoghegan wrote:
> On Fri, Jan 31, 2014 at 1:54 AM, Andres Freund <andres@2ndquadrant.com> wrote:
> > I plan to split the atomics patch into smaller chunks before
> > reposting. Imo the "Convert the PGPROC->lwWaitLink list into a dlist
> > instead of open coding it." is worth being applied independently from
> > the rest of the series, it simplies code and it fixes a bug...
>
> For things that require a format-patch series, I personally find it
> easier to work off a feature branch on a remote under the control of
> the patch author. The only reason that I don't do so myself is that I
> know that that isn't everyone's preference.
I do to, that's why I have a git branch for all but the most trivial
branches.
> I have access to a large server for the purposes of benchmarking this.
> On the plus side, this is a box very much capable of exercising these
> bottlenecks: a 48 core AMD system, with 256GB of RAM. However, I had
> to instruct someone else on how to conduct the benchmark, since I
> didn't have SSH access, and the OS and toolchain were antiquated,
> particularly for this kind of thing. This is Linux kernel
> 2.6.18-274.3.1.el5 (RHEL5.7). The GCC version that Postgres was built
> with was 4.1.2. This is not what I'd hoped for; obviously I would have
> preferred to be able to act on your warning: "Please also note that
> due to the current state of the atomics implementation this likely
> will only work on a somewhat recent gcc and that the performance might
> be slightly worse than in the previous version because the atomic add
> is implemented using the CAS fallback". Even still, it might be
> marginally useful to get a sense of that cost.
I *think* it should actually work on gcc 4.1, I've since added the
fallbacks I hadn't back when I wrote the above. I've added exactly those
atomics that are needed for the scalable lwlock things (xadd, xsub (yes,
that's essentially the same) and cmpxchg).
> I'm looking at alternative options, because this is not terribly
> helpful. With those big caveats in mind, consider the results of the
> benchmark, which show the patch performing somewhat worse than the
> master baseline at higher client counts:
I think that's actually something else. I'd tried to make some
definitions simpler, and that has, at least for the machine I have
occasional access to, pessimized things. I can't always run the tests
there, so I hadn't noticed before the repost.
I've pushed a preliminary relief to the git repository, any chance you
could retry?
Greetings,
Andres Freund
-- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services
