On Mon, Oct 10, 2016 at 5:21 AM, Heikki Linnakangas <hlinn...@iki.fi> wrote:
> Admittedly that's confusing. Thinking about this some more, I came up with
> the attached. I removed the separate LogicalTapeAssignReadBufferSize() call
> altogether - the read buffer size is now passed as argument to the
> LogicalTapeRewindForRead() call.
> I split LogicalTapeRewind() into two: LogicalTapeRewindForRead() and
> LogicalTapeRewindForWrite(). A boolean argument like 'forWrite' is mentally
> challenging, because when reading a call site, you have to remember which
> way round the boolean is. And now you also pass the extra buffer_size
> argument when rewinding for reading, but not when writing.

I like the general idea here.

> I gave up on the logic to calculate the quotient and reminder of the
> available memory, and assigning one extra buffer to some of the tapes. I'm
> now just rounding it down to the nearest BLCKSZ boundary. That simplifies
> the code further, although we're now not using all the memory we could. I'm
> pretty sure that's OK, although I haven't done any performance testing of
> this.

The only thing I notice is that this new struct field could use a comment:

> --- a/src/backend/utils/sort/tuplesort.c
> +++ b/src/backend/utils/sort/tuplesort.c
> @@ -366,6 +366,8 @@ struct Tuplesortstate
>     char       *slabMemoryEnd;  /* end of slab memory arena */
>     SlabSlot   *slabFreeHead;   /* head of free list */
> +   size_t      read_buffer_size;
> +

Also, something about trace_sort here:

> +#ifdef TRACE_SORT
> +   if (trace_sort)
> +       elog(LOG, "using " INT64_FORMAT " KB of memory for read buffers among 
> %d input tapes",
> +            (state->availMem) / 1024, numInputTapes);
> +#endif
> +
> +   state->read_buffer_size = state->availMem / numInputTapes;
> +   USEMEM(state, state->availMem);

Maybe you should just make the trace_sort output talk about blocks at
this point?

Peter Geoghegan

Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:

Reply via email to