Thanks for updating the patch.
On 2018/11/14 13:16, David Rowley wrote:
> Thanks for looking at this again.
>
> On 14 November 2018 at 13:47, Amit Langote
> <Langote_Amit_f8@lab.ntt.co.jp> wrote:
>> + if (dispatchidx >= proute->dispatch_allocsize)
>> + {
>> + /* Expand allocated space. */
>> + proute->dispatch_allocsize *= 2;
>> + proute->partition_dispatch_info = (PartitionDispatchData **)
>> + repalloc(proute->partition_dispatch_info,
>> + sizeof(PartitionDispatchData *) *
>> + proute->dispatch_allocsize);
>> + }
>>
>> Sorry, I forgot to point this out before, but can this code in
>> ExecInitPartitionDispatchInfo be accommodated in
>> ExecCheckPartitionArraySpace() for consistency?
>
> I don't really want to put that code in ExecCheckPartitionArraySpace()
> as the way the function is now, it makes quite a lot of sense for the
> compiler to inline it. If we add redundant work in there, then it
> makes less sense. There's never any need to check both arrays at once
> as we're only adding the new item to one array at a time.
>
> Instead, I've written a new function named
> ExecCheckDispatchArraySpace() and put the resize code inside that.
Okay, seems fine.
> I've fixed the typos you mentioned. The only other thing I changed was
> to only allocate the PartitionDispatch->tupslot if a conversion is
> required. The previous code allocated this regardless if it was going
> to be used or not. This saves both the redundant allocation and also
> very slightly reduces the cost of the if test in ExecFindPartition().
> There's now no need to check if the map ! NULL as if the slot is there
Also makes sense.
Although it seems that Alvaro has already started at looking at this, I'll
mark the CF entry as Ready for Committer anyway, because I don't have any
more comments. :)
Thanks,
Amit
