Re: MemoryContextAllocHuge(): selectively bypassing MaxAllocSize

On Wed, Jun 26, 2013 at 03:48:23PM -0700, Jeff Janes wrote:
> On Mon, May 13, 2013 at 7:26 AM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> > This patch introduces MemoryContextAllocHuge() and repalloc_huge() that
> > check
> > a higher MaxAllocHugeSize limit of SIZE_MAX/2. Chunks don't bother
> > recording
> > whether they were allocated as huge; one can start with palloc() and then
> > repalloc_huge() to grow the value.
>
>
> Since it doesn't record the size, I assume the non-use as a varlena is
> enforced only by coder discipline and not by the system?

We will rely on coder discipline, yes. The allocator actually does record a
size. I was referring to the fact that it can't distinguish the result of
repalloc(p, 7) from the result of repalloc_huge(p, 7).

> What is likely to happen if I accidentally let a pointer to huge memory
> escape to someone who then passes it to varlena constructor without me
> knowing it? (I tried sabotaging the code to make this happen, but I could
> not figure out how to). Is there a place we can put an Assert to catch
> this mistake under enable-cassert builds?

Passing a too-large value gives a modulo effect. We could inject an
AssertMacro() into SET_VARSIZE(). But it's a hot path, and I don't think this
mistake is too likely.

> The only danger I can think of is that it could sometimes make some sorts
> slower, as using more memory than is necessary can sometimes slow down an
> "external" sort (because the heap is then too big for the fastest CPU
> cache). If you use more tapes, but not enough more to reduce the number of
> passes needed, then you can get a slowdown.

Interesting point, though I don't fully understand it. The fastest CPU cache
will be a tiny L1 data cache; surely that's not the relevant parameter here?

> I can't imagine that it would make things worse on average, though, as the
> benefit of doing more sorts as quicksorts rather than merge sorts, or doing
> mergesort with fewer number of passes, would outweigh sometimes doing a
> slower mergesort. If someone has a pathological use pattern for which the
> averages don't work out favorably for them, they could probably play with
> work_mem to correct the problem. Whereas without the patch, people who
> want more memory have no options.

Agreed.

> People have mentioned additional things that could be done in this area,
> but I don't think that applying this patch will make those things harder,
> or back us into a corner. Taking an incremental approach seems suitable.

Committed with some cosmetic tweaks discussed upthread.

Thanks,
nm

--
Noah Misch
EnterpriseDB http://www.enterprisedb.com