Thread: stack usage in toast_insert_or_update()

<br clear="all" />Not sure whether its worth optimizing, but had spotted this while browsing<br />the code a while
back.So thought would post it anyways.<br /><br />The stack usage for toast_insert_or_update() may run into several KBs
since<br />the MaxHeapAttributeNumber is set to a very large value of 1600. The usage<br />could anywhere between 28K
to48K depending on alignment and whether its a<br />32-bit or a 64-bit machine.<br /><br />Is it very common to have so
manyattributes in a table ? If not, would it be worth <br />to allocate only as much space as required ?<br /><br
/>Thanks,<br/>Pavan<br /><br />-- <br /><br />EnterpriseDB     <a
href="http://www.enterprisedb.com">http://www.enterprisedb.com</a>

"Pavan Deolasee" <pavan.deolasee@gmail.com> writes:
> The stack usage for toast_insert_or_update() may run into several KBs since
> the MaxHeapAttributeNumber is set to a very large value of 1600. The usage
> could anywhere between 28K to 48K depending on alignment and whether its a
> 32-bit or a 64-bit machine.

So?  The routine is not re-entrant so I don't see that the stack space
is a big problem.  It's coded that way to avoid palloc/pfree cycles...
        regards, tom lane

"Pavan Deolasee" <pavan.deolasee@gmail.com> writes:
> Btw, I noticed that the toast_insert_or_update() is re-entrant.
> toast_save_datum() calls simple_heap_insert() which somewhere down the
> line calls toast_insert_or_update() again.

The toast code takes pains to ensure that the tuples it creates won't be
subject to re-toasting.  Else it'd be an infinite recursion.
        regards, tom lane

"Pavan Deolasee" <pavan.deolasee@gmail.com> writes:
> On 1/31/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> The toast code takes pains to ensure that the tuples it creates won't be
>> subject to re-toasting.  Else it'd be an infinite recursion.

> I think I found it. The toast_insert_or_update() function gets into an
> unnecessary recursion because of alignment issues. It thus toasts
> already toasted data.  This IMHO might be causing unnecessary
> overheads for each toast operation.

Interesting --- I'd never seen this because both of my usual development
machines have MAXALIGN 8, and it works out that that makes
TOAST_MAX_CHUNK_SIZE 1986, which makes the actual toasted tuple size
2030, which maxaligns to 2032, which is still less than
TOAST_TUPLE_THRESHOLD.  I think the coding was implicitly assuming that
TOAST_TUPLE_THRESHOLD would itself be a maxalign'd value, but it's not
necessarily (and in fact not, with the current page header size ---
I wonder whether the bug was originally masked because the page header
size was different??)

We can't change TOAST_MAX_CHUNK_SIZE without forcing an initdb, but I
think that it would be safe to remove the MAXALIGN'ing of the tuple
size in the tests in heapam.c, that is
   if (HeapTupleHasExternal(tup) ||       (MAXALIGN(tup->t_len) > TOAST_TUPLE_THRESHOLD))       heaptup =
toast_insert_or_update(relation,tup, NULL);   else       heaptup = tup;
 

becomes
   if (HeapTupleHasExternal(tup) ||       (tup->t_len > TOAST_TUPLE_THRESHOLD))       heaptup =
toast_insert_or_update(relation,tup, NULL);   else       heaptup = tup;
 

which'll save a cycle or two as well as avoid this corner case.
It seems like a number of the uses of MAXALIGN in tuptoaster.c
are useless/bogus as well.  Comments?
        regards, tom lane

On 1/31/2007 12:41 PM, Tom Lane wrote:
> "Pavan Deolasee" <pavan.deolasee@gmail.com> writes:
>> On 1/31/07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> The toast code takes pains to ensure that the tuples it creates won't be
>>> subject to re-toasting.  Else it'd be an infinite recursion.
> 
>> I think I found it. The toast_insert_or_update() function gets into an
>> unnecessary recursion because of alignment issues. It thus toasts
>> already toasted data.  This IMHO might be causing unnecessary
>> overheads for each toast operation.
> 
> Interesting --- I'd never seen this because both of my usual development
> machines have MAXALIGN 8, and it works out that that makes
> TOAST_MAX_CHUNK_SIZE 1986, which makes the actual toasted tuple size
> 2030, which maxaligns to 2032, which is still less than
> TOAST_TUPLE_THRESHOLD.  I think the coding was implicitly assuming that
> TOAST_TUPLE_THRESHOLD would itself be a maxalign'd value, but it's not
> necessarily (and in fact not, with the current page header size ---
> I wonder whether the bug was originally masked because the page header
> size was different??)
> 
> We can't change TOAST_MAX_CHUNK_SIZE without forcing an initdb, but I
> think that it would be safe to remove the MAXALIGN'ing of the tuple
> size in the tests in heapam.c, that is
> 
>     if (HeapTupleHasExternal(tup) ||
>         (MAXALIGN(tup->t_len) > TOAST_TUPLE_THRESHOLD))
>         heaptup = toast_insert_or_update(relation, tup, NULL);
>     else
>         heaptup = tup;
> 
> becomes
> 
>     if (HeapTupleHasExternal(tup) ||
>         (tup->t_len > TOAST_TUPLE_THRESHOLD))
>         heaptup = toast_insert_or_update(relation, tup, NULL);
>     else
>         heaptup = tup;
> 
> which'll save a cycle or two as well as avoid this corner case.
> It seems like a number of the uses of MAXALIGN in tuptoaster.c
> are useless/bogus as well.  Comments?

Can't we maxalign the page header in the calculations?


Jan

> 
>             regards, tom lane
> 
> ---------------------------(end of broadcast)---------------------------
> TIP 3: Have you checked our extensive FAQ?
> 
>                http://www.postgresql.org/docs/faq


-- 
#======================================================================#
# It's easier to get forgiveness for being wrong than for being right. #
# Let's break this rule - forgive me.                                  #
#================================================== JanWieck@Yahoo.com #

Jan Wieck <JanWieck@Yahoo.com> writes:
> On 1/31/2007 12:41 PM, Tom Lane wrote:
>> We can't change TOAST_MAX_CHUNK_SIZE without forcing an initdb, but I
>> think that it would be safe to remove the MAXALIGN'ing of the tuple
>> size in the tests in heapam.c, that is

> Can't we maxalign the page header in the calculations?

Actually, the page header size *is* maxaligned.  The problem is that
TOAST_TUPLE_THRESHOLD isn't.

We could make it so, but that would change TOAST_MAX_CHUNK_SIZE thus
forcing an initdb.  I think simply removing the MAXALIGN operations
in the code is a better answer, as it eliminates some rather pointless
overhead.  There's no particular reason why the threshold needs to be
maxaligned ...
        regards, tom lane