rethinking dense_alloc (HashJoin) as a memory context

Hi,

In the thread [1] dealing with hashjoin bug introduced in 9.5, Tom
voiced his dislike of dense_alloc. I kinda agree with him that
introducing "local allocators" may not be the best idea, and as
dense_alloc was introduced by me I was playing with the idea to wrap
this into a regular memory context, perhaps with some restrictions (e.g.
no pfree). But I'm having trouble with that approach ...

Let me quickly explain the idea behind dense_alloc. When building the
tuple hash table in hash join, we simply allocate large chunk of memory
using palloc (~32kB), and then store the tuples into the chunk on our
own without calling palloc for each tuple. Each tuple already has length
in the header, so we don't need chunk header. Also, we don't do the 2^k
chunk sizes and instead store the tuples densely.

This means we can't do repalloc or pfree on the tuples, but fine. We
never did repalloc in hashjoin anyway, and pfree is only needed when
increasing the number of batches. But with dense_alloc we can simply
walk through the tuples as stored in the allocated chunks, which has the
nice benefit that it's sequential, making memory prefetching more
efficient than with the old code (walking through buckets). Also, no
freelists and such.

So the dense_alloc has several benefits:

(a) memory reduction thanks to eliminating StandardChunkHeader (which is
16B, and quite noticeable for narrow tuples)

(b) memory reduction thanks to dense packing tuples (not leaving free
space in each chunk)

(c) improving efficiency by sequential memory accesses (compared to
random accesses caused by access through buckets)

Per the measurements done in thread [2], (a) and (b) may reduce memory
requirements by 50% in some cases. I also vaguely remember doing
benchmarks for (c) and seeing measurable improvements, but I don't see
the numbers in the thread, so either it was posted somewhere else or not
at all :-/

Anyway, I'm explaining this because I think it's important the new
reworked code achieves the same benefits. But when trying to implement
it as a special memory context, I quickly ran into the requirement that
each chunk has a chunk header [3] which would prevent (a).

I know it was proposed to only include the chunk header when compiled
with asserts, but I don't like the idea of having a reusable code that
depends on that (and fails with a segfault without it).

If I have to choose between a memory context that is essentially meant
to be reused, but likely to fail unexpectedly in non-assert builds, and
a special local allocator isolated to a single node, I choose the
latter. Perhaps I'd see this differently had there been other places
that could use the new memory context, but I can't think of one.

Opinions?

[1]
https://www.postgresql.org/message-id/flat/7034.1454722453%40sss.pgh.pa.us

[2] https://www.postgresql.org/message-id/flat/53B4A03F.3070409%40fuzzy.cz

[3]
https://github.com/postgres/postgres/blob/master/src/include/utils/memutils.h#L49

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services