Обсуждение: 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