Re: Combining Aggregates

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: David Rowley <david(dot)rowley(at)2ndquadrant(dot)com>, Haribabu Kommi <kommi(dot)haribabu(at)gmail(dot)com>, Heikki Linnakangas <hlinnaka(at)iki(dot)fi>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Simon Riggs <simon(at)2ndquadrant(dot)com>, Kouhei Kaigai <kaigai(at)ak(dot)jp(dot)nec(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>, Amit Kapila <amit(dot)kapila(at)enterprisedb(dot)com>
Subject: Re: Combining Aggregates
Date: 2016-01-18 17:34:13
Message-ID: CA+TgmoZSkOZ23Vf4UvBEk-1V7Y_3Cm48d96HV3dxFOJUvqYoJg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jan 18, 2016 at 12:09 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> Robert Haas <robertmhaas(at)gmail(dot)com> writes:
>> Yeah. The API contract for an expanded object took me quite a while
>> to puzzle out, but it seems to be this: if somebody hands you an R/W
>> pointer to an expanded object, you're entitled to assume that you can
>> "take over" that object and mutate it however you like. But the
>> object might be in some other memory context, so you have to move it
>> into your own memory context.
>
> Only if you intend to keep it --- for example, a function that is mutating
> and returning an object isn't required to move it somewhere else, if the
> input is R/W, and I think it generally shouldn't.
>
> In the context here, I'd think it is the responsibility of nodeAgg.c
> not individual datatype functions to make sure that expanded objects
> live where it wants them to.

That's how I did it in my prototype, but the problem with that is that
spinning up a memory context for every group sucks when there are many
groups with only a small number of elements each - hence the 50%
regression that David Rowley observed. If we're going to use expanded
objects here, which seems like a good idea in principle, that's going
to have to be fixed somehow. We're flogging the heck out of malloc by
repeatedly creating a context, doing one or two allocations in it, and
then destroying the context.

I think that, in general, the memory context machinery wasn't really
designed to manage lots of small contexts. The overhead of spinning
up a new context for just a few allocations is substantial. That
matters in some other situations too, I think - I've commonly seen
AllocSetContextCreate taking several percent of runtime in profiles.
But here it's much exacerbated. I'm not sure whether it's better to
attack that problem at the root and try to make AllocSetContextCreate
cheaper, or whether we should try to figure out some change to the
expanded-object machinery to address the issue.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2016-01-18 17:35:55 Re: 2016-01 Commitfest
Previous Message Andrew Dunstan 2016-01-18 17:32:27 Re: Removing service-related code in pg_ctl for Cygwin