From a2e092e1c7b9710c6b63ed226040971246323bff Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 4 Feb 2007 20:00:37 +0000 Subject: [PATCH] Don't MAXALIGN in the checks to decide whether a tuple is over TOAST's threshold for tuple length. On 4-byte-MAXALIGN machines, the toast code creates tuples that have t_len exactly TOAST_TUPLE_THRESHOLD ... but this number is not itself maxaligned, so if heap_insert maxaligns t_len before comparing to TOAST_TUPLE_THRESHOLD, it'll uselessly recurse back to tuptoaster.c, wasting cycles. (It turns out that this does not happen on 8-byte-MAXALIGN machines, because for them the outer MAXALIGN in the TOAST_MAX_CHUNK_SIZE macro reduces TOAST_MAX_CHUNK_SIZE so that toast tuples will be less than TOAST_TUPLE_THRESHOLD in size. That MAXALIGN is really incorrect, but we can't remove it now, see below.) There isn't any particular value in maxaligning before comparing to the thresholds, so just don't do that, which saves a small number of cycles in itself. These numbers should be rejiggered to minimize wasted space on toast-relation pages, but we can't do that in the back branches because changing TOAST_MAX_CHUNK_SIZE would force an initdb (by changing the contents of toast tables). We can move the toast decision thresholds a bit, though, which is what this patch effectively does. Thanks to Pavan Deolasee for discovering the unintended recursion. Back-patch into 8.2, but not further, pending more testing. (HEAD is about to get a further patch modifying the thresholds, so it won't help much for testing this form of the patch.) --- src/backend/access/heap/heapam.c | 13 +++++------ src/backend/access/heap/tuptoaster.c | 33 ++++++++++++++++------------ src/include/access/tuptoaster.h | 9 +++++--- 3 files changed, 31 insertions(+), 24 deletions(-) diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c index c0f5f2074b..042a33a5d2 100644 --- a/src/backend/access/heap/heapam.c +++ b/src/backend/access/heap/heapam.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.225 2007/01/25 02:17:25 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.226 2007/02/04 20:00:37 tgl Exp $ * * * INTERFACE ROUTINES @@ -1418,8 +1418,7 @@ heap_insert(Relation relation, HeapTuple tup, CommandId cid, * Note: below this point, heaptup is the data we actually intend to store * into the relation; tup is the caller's original untoasted data. */ - if (HeapTupleHasExternal(tup) || - (MAXALIGN(tup->t_len) > TOAST_TUPLE_THRESHOLD)) + if (HeapTupleHasExternal(tup) || tup->t_len > TOAST_TUPLE_THRESHOLD) heaptup = toast_insert_or_update(relation, tup, NULL, use_wal); else heaptup = tup; @@ -2073,14 +2072,14 @@ l2: * We need to invoke the toaster if there are already any out-of-line * toasted values present, or if the new tuple is over-threshold. */ - newtupsize = MAXALIGN(newtup->t_len); - need_toast = (HeapTupleHasExternal(&oldtup) || HeapTupleHasExternal(newtup) || - newtupsize > TOAST_TUPLE_THRESHOLD); + newtup->t_len > TOAST_TUPLE_THRESHOLD); pagefree = PageGetFreeSpace((Page) dp); + newtupsize = MAXALIGN(newtup->t_len); + if (need_toast || newtupsize > pagefree) { oldtup.t_data->t_infomask &= ~(HEAP_XMAX_COMMITTED | @@ -2100,7 +2099,7 @@ l2: * * Note: below this point, heaptup is the data we actually intend to * store into the relation; newtup is the caller's original untoasted - * data. (We always use WAL for toast table updates.) + * data. */ if (need_toast) { diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.c index 449c57268e..6fde18104a 100644 --- a/src/backend/access/heap/tuptoaster.c +++ b/src/backend/access/heap/tuptoaster.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/access/heap/tuptoaster.c,v 1.69 2007/01/25 02:17:26 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/access/heap/tuptoaster.c,v 1.70 2007/02/04 20:00:37 tgl Exp $ * * * INTERFACE ROUTINES @@ -506,17 +506,23 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup, bool us * 4: Store attributes with attstorage 'm' external * ---------- */ + + /* compute header overhead --- this should match heap_form_tuple() */ maxDataLen = offsetof(HeapTupleHeaderData, t_bits); if (has_nulls) maxDataLen += BITMAPLEN(numAttrs); - maxDataLen = TOAST_TUPLE_TARGET - MAXALIGN(maxDataLen); + if (newtup->t_data->t_infomask & HEAP_HASOID) + maxDataLen += sizeof(Oid); + maxDataLen = MAXALIGN(maxDataLen); + Assert(maxDataLen == newtup->t_data->t_hoff); + /* now convert to a limit on the tuple data size */ + maxDataLen = TOAST_TUPLE_TARGET - maxDataLen; /* * Look for attributes with attstorage 'x' to compress */ - while (MAXALIGN(heap_compute_data_size(tupleDesc, - toast_values, toast_isnull)) > - maxDataLen) + while (heap_compute_data_size(tupleDesc, + toast_values, toast_isnull) > maxDataLen) { int biggest_attno = -1; int32 biggest_size = MAXALIGN(sizeof(varattrib)); @@ -575,9 +581,9 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup, bool us * Second we look for attributes of attstorage 'x' or 'e' that are still * inline. */ - while (MAXALIGN(heap_compute_data_size(tupleDesc, - toast_values, toast_isnull)) > - maxDataLen && rel->rd_rel->reltoastrelid != InvalidOid) + while (heap_compute_data_size(tupleDesc, + toast_values, toast_isnull) > maxDataLen && + rel->rd_rel->reltoastrelid != InvalidOid) { int biggest_attno = -1; int32 biggest_size = MAXALIGN(sizeof(varattrib)); @@ -627,9 +633,8 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup, bool us * Round 3 - this time we take attributes with storage 'm' into * compression */ - while (MAXALIGN(heap_compute_data_size(tupleDesc, - toast_values, toast_isnull)) > - maxDataLen) + while (heap_compute_data_size(tupleDesc, + toast_values, toast_isnull) > maxDataLen) { int biggest_attno = -1; int32 biggest_size = MAXALIGN(sizeof(varattrib)); @@ -687,9 +692,9 @@ toast_insert_or_update(Relation rel, HeapTuple newtup, HeapTuple oldtup, bool us /* * Finally we store attributes of type 'm' external */ - while (MAXALIGN(heap_compute_data_size(tupleDesc, - toast_values, toast_isnull)) > - maxDataLen && rel->rd_rel->reltoastrelid != InvalidOid) + while (heap_compute_data_size(tupleDesc, + toast_values, toast_isnull) > maxDataLen && + rel->rd_rel->reltoastrelid != InvalidOid) { int biggest_attno = -1; int32 biggest_size = MAXALIGN(sizeof(varattrib)); diff --git a/src/include/access/tuptoaster.h b/src/include/access/tuptoaster.h index ee8cfa7449..49d69836dc 100644 --- a/src/include/access/tuptoaster.h +++ b/src/include/access/tuptoaster.h @@ -6,7 +6,7 @@ * * Copyright (c) 2000-2007, PostgreSQL Global Development Group * - * $PostgreSQL: pgsql/src/include/access/tuptoaster.h,v 1.30 2007/01/25 02:17:26 momjian Exp $ + * $PostgreSQL: pgsql/src/include/access/tuptoaster.h,v 1.31 2007/02/04 20:00:37 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -26,8 +26,11 @@ /* * These symbols control toaster activation. If a tuple is larger than * TOAST_TUPLE_THRESHOLD, we will try to toast it down to no more than - * TOAST_TUPLE_TARGET bytes. Both numbers include all tuple header and - * alignment-padding overhead. + * TOAST_TUPLE_TARGET bytes. Both numbers include all tuple header overhead + * and between-fields alignment padding, but we do *not* consider any + * end-of-tuple alignment padding; hence the values can be compared directly + * to a tuple's t_len field. (Note that the symbol values are not + * necessarily MAXALIGN multiples.) * * The numbers need not be the same, though they currently are. */ -- 2.40.0