From: | Peter Geoghegan <pg(at)heroku(dot)com> |
---|---|
To: | Heikki Linnakangas <hlinnaka(at)iki(dot)fi> |
Cc: | Claudio Freire <klaussfreire(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Tuplesort merge pre-reading |
Date: | 2016-09-11 22:59:18 |
Message-ID: | CAM3SWZRrUCOXJKnW_bU1x3nj5e0=iQx=nrrhL9F27jyXZ4gR0A@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Sun, Sep 11, 2016 at 3:13 PM, Peter Geoghegan <pg(at)heroku(dot)com> wrote:
> * Doesn't this code need to call MemoryContextAllocHuge() rather than palloc()?:
>
>> @@ -709,18 +765,19 @@ LogicalTapeRewind(LogicalTapeSet *lts, int tapenum, bool forWrite)
>> Assert(lt->frozen);
>> datablocknum = ltsRewindFrozenIndirectBlock(lts, lt->indirect);
>> }
>> +
>> + /* Allocate a read buffer */
>> + if (lt->buffer)
>> + pfree(lt->buffer);
>> + lt->buffer = palloc(lt->read_buffer_size);
>> + lt->buffer_size = lt->read_buffer_size;
Of course, when you do that you're going to have to make the new
"buffer_size" and "read_buffer_size" fields of type "Size" (or,
possibly, "int64", to match tuplesort.c's own buffer sizing variables
ever since Noah added MaxAllocSizeHuge). Ditto for the existing "pos"
and "nbytes" fields next to "buffer_size" within the struct
LogicalTape, I think. ISTM that logtape.c blocknums can remain of type
"long", though, since that reflects an existing hardly-relevant
limitation that you're not making any worse.
More generally, I think you also need to explain in comments why there
is a "read_buffer_size" field in addition to the "buffer_size" field.
--
Peter Geoghegan
From | Date | Subject | |
---|---|---|---|
Next Message | Vitaly Burovoy | 2016-09-11 23:11:25 | Re: [REVIEW] Tab Completion for CREATE DATABASE ... TEMPLATE ... |
Previous Message | Daniel Gustafsson | 2016-09-11 22:56:46 | Typo in filename identification |