Обсуждение: Memory allocation in spi_printtup()

spi_printtup() has the following code (spi.c:1798):

        if (tuptable->free == 0)
        {
                tuptable->free = 256;
                tuptable->alloced += tuptable->free;
                tuptable->vals = (HeapTuple *) repalloc(tuptable->vals,

   tuptable->alloced * sizeof(HeapTuple));
        }

i.e., it grows the size of the tuptable by a fixed amount when it runs
out of space. That seems odd; doubling the size of the table would be
more standard. Does anyone know if there is a rationale for this
behavior?

Attached is a one-liner to double the size of the table when space is
exhausted. Constructing a simple test case in which a large result is
materialized via SPI_execute() (e.g., by passing two large queries to
crosstab() from contrib/tablefunc), this change reduces the time
required to exceed the palloc size limit from ~300 seconds to ~93
seconds on my laptop.

Of course, using SPI_execute() rather than cursors for queries with
large result sets is not a great idea, but there is demonstrably code
that does this (e.g., contrib/tablefunc -- I'll send a patch for that
shortly).

Neil

Neil Conway <neil.conway@gmail.com> writes:

Hi Neil!  Long time no see.

> spi_printtup() has the following code (spi.c:1798):
>         if (tuptable->free == 0)
>         {
>                 tuptable->free = 256;
>                 tuptable->alloced += tuptable->free;
>                 tuptable->vals = (HeapTuple *) repalloc(tuptable->vals,
>    tuptable->alloced * sizeof(HeapTuple));
>         }

> i.e., it grows the size of the tuptable by a fixed amount when it runs
> out of space. That seems odd; doubling the size of the table would be
> more standard. Does anyone know if there is a rationale for this
> behavior?

Seems like it must be just legacy code.  We're only allocating pointers
here; the actual tuples will likely be significantly larger.  So there's
not a lot of reason not to use the normal doubling rule.

> Attached is a one-liner to double the size of the table when space is
> exhausted.

I think this could use a comment, but otherwise seems OK.

Should we back-patch this change?  Seems like it's arguably a
performance bug.
        regards, tom lane

On Mon, Aug 17, 2015 at 7:56 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Hi Neil!  Long time no see.

Likewise :)

>> Attached is a one-liner to double the size of the table when space is
>> exhausted.
>
> I think this could use a comment, but otherwise seems OK.

Attached is a revised patch with a comment.

> Should we back-patch this change?  Seems like it's arguably a
> performance bug.

Sounds good to me.

Neil

Neil Conway <neil.conway@gmail.com> writes:
> On Mon, Aug 17, 2015 at 7:56 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Should we back-patch this change?  Seems like it's arguably a
>> performance bug.

> Sounds good to me.

Committed and back-patched.
        regards, tom lane