From: Tomas Vondra <tomas.vondra@2ndquadrant.com>
To: Binguo Bao <djydewang@gmail.com>
Cc: Andrey Borodin <x4mmm@yandex-team.ru>
Cc: Paul Ramsey <pramsey@cleverelephant.ca>
Cc: Pgsql Hackers <pgsql-hackers@postgresql.org>
Subject: Re: Optimize partial TOAST decompression
Date: Sat, 6 Jul 2019 17:23:37 +0200
Message-ID: <20190706152337.l64zhoj3xs3qkfc4@development> (raw)
In-Reply-To: <CAL-OGkuvHceCv96H8PHk+GeDn5NxuojGhQQati7Z7uFHduZVBg@mail.gmail.com>
References: <CAL-OGkthU9Gs7TZchf5OWaL-Gsi=hXqufTxKv9qpNG73d5na_g@mail.gmail.com>
<7B7E4A05-F404-4083-B7F2-267869EE2910@yandex-team.ru>
<CAL-OGktyTE=Svgv8YTXou4UxjK-zFLtxU6Wy3RndgCesGexwkA@mail.gmail.com>
<DFBA504A-0E2E-4E9C-8C69-A3C308982A2A@yandex-team.ru>
<CAL-OGkuBn13N1=jBkiWs4oks_3SxVioiec9Sm9zPDKJFU4szyg@mail.gmail.com>
<CACowWR1U3guqBzqPL1rHWxYBpU-yUQzr+s07MPT9qBg8LGb+uA@mail.gmail.com>
<CAL-OGkux7+Bm_J=t5VpH7fJGGSm+PxWJtgs1+WU2g6cmLru==A@mail.gmail.com>
<BFA726EE-9C16-475D-A739-16DFC34931C1@yandex-team.ru>
<20190704174654.vnvkh7rusv25o6f5@development>
<CAL-OGkuvHceCv96H8PHk+GeDn5NxuojGhQQati7Z7uFHduZVBg@mail.gmail.com>
On Sat, Jul 06, 2019 at 02:27:56AM +0800, Binguo Bao wrote:
>Hi, Tomas!
>Thanks for your testing and the suggestion.
>
>That's quite bizarre behavior - it does work with a prefix, but not with
>> suffix. And the exact ERROR changes after the prefix query.
>
>
>I think bug is caused by "#2 0x00000000004c3b08 in
>heap_tuple_untoast_attr_slice (attr=<optimized out>, sliceoffset=0,
>slicelength=-1) at tuptoaster.c:315",
>since I ignore the case where slicelength is negative, and I've appended
>some comments for heap_tuple_untoast_attr_slice for the case.
>
>FWIW the new code added to this
>> function does not adhere to our code style, and would deserve some
>> additional explanation of what it's doing/why. Same for the
>> heap_tuple_untoast_attr_slice, BTW.
>
>
>I've added more comments to explain the code's behavior.
>Besides, I also modified the macro "TOAST_COMPRESS_RAWDATA" to
>"TOAST_COMPRESS_DATA" since
>it is used to get toast compressed data rather than raw data.
>
Thanks, this seems to address the issue - I can no longer reproduce the
crashes, allowing the benchmark to complete. I'm attaching the script I
used and spreadsheet with a summary of results.
For the cases I've tested (100k - 10M values, different compressibility,
different prefix/length values), the results are kinda mixed - many
cases got much faster (~2x), but other cases got slower too. We're
however talking about queries taking a couple of miliseconds, so in
absolute numbers the differences are small.
That does not mean the optimization is useless - but the example shared
at the beginning of this thread is quite extreme, as the values are
extremely compressible. Each value is ~38MB (in plaintext), but a table
with 100 such values has only ~40MB. Tha's 100:1 compression ratio,
which I think is not typical for real-world data sets.
The data I've used are less extreme, depending on the fraction of random
data in values.
I went through the code too. I've reworder a couple of comments and code
style issues, but there are a couple of more serious issues.
1) Why rename TOAST_COMPRESS_RAWDATA to TOAST_COMPRESS_DATA?
This seems unnecessary, and it discards the clear hint that it's about
accessing the *raw* data, and the relation to TOAST_COMPRESS_RAWSIZE.
IMHO we should keep the original naming.
2) pglz_maximum_compressed_size signatures are confusing
There are two places with pglz_maximum_compressed_size signature, and
those places are kinda out of sync when it comes to parameter names:
int32
pglz_maximum_compressed_size(int32 raw_slice_size,
int32 total_compressed_size)
extern
int32 pglz_maximum_compressed_size(int32 raw_slice_size,
int32 raw_size);
Also, pg_lzcompress.c has no concept of a "slice" because it only deals
with simple compression, slicing is responsibility of the tuptoaster. So
we should not mix those two, not even in comments.
I propose tweaks per the attached patch - I think it makes the code
clearer, and it's mostly cosmetic stuff. But I haven't tested the
changes beyond "it compiles".
regards
--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From d50ae41bf1d1c4e2786a55a610300606c1ec43a5 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas@2ndquadrant.com>
Date: Sat, 6 Jul 2019 15:41:37 +0200
Subject: [PATCH 1/2] Optimize partial TOAST decompression
---
src/backend/access/heap/tuptoaster.c | 57 ++++++++++++++++++++++------
src/common/pg_lzcompress.c | 26 +++++++++++++
src/include/common/pg_lzcompress.h | 1 +
3 files changed, 72 insertions(+), 12 deletions(-)
diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.cindex 55d6e91d1c..1eb6ccaca2 100644--- a/src/backend/access/heap/tuptoaster.c+++ b/src/backend/access/heap/tuptoaster.c@@ -61,7 +61,8 @@ typedef struct toast_compress_header
*/
#define TOAST_COMPRESS_HDRSZ ((int32) sizeof(toast_compress_header))
#define TOAST_COMPRESS_RAWSIZE(ptr) (((toast_compress_header *) (ptr))->rawsize)
-#define TOAST_COMPRESS_RAWDATA(ptr) \+#define TOAST_COMPRESS_SIZE(ptr) ((int32) VARSIZE(ptr) - TOAST_COMPRESS_HDRSZ)+#define TOAST_COMPRESS_DATA(ptr) \
(((char *) (ptr)) + TOAST_COMPRESS_HDRSZ)
#define TOAST_COMPRESS_SET_RAWSIZE(ptr, len) \
(((toast_compress_header *) (ptr))->rawsize = (len))
@@ -252,6 +253,8 @@ heap_tuple_untoast_attr(struct varlena *attr)
*
* Public entry point to get back part of a toasted value
* from compression or external storage.
+ *+ * Note if slicelength is negative, return suffix of the value.
* ----------
*/
struct varlena *
@@ -273,8 +276,23 @@ heap_tuple_untoast_attr_slice(struct varlena *attr,
if (!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer))
return toast_fetch_datum_slice(attr, sliceoffset, slicelength);
- /* fetch it back (compressed marker will get set automatically) */- preslice = toast_fetch_datum(attr);+ /*+ * If only need the prefix slice, fetch enough datums to decompress.+ * Otherwise, fetch all datums.+ */+ if (slicelength > 0 && sliceoffset >= 0)+ {+ int32 max_size;+ max_size = pglz_maximum_compressed_size(sliceoffset + slicelength,+ TOAST_COMPRESS_SIZE(attr));+ /*+ * Be sure to get enough compressed slice+ * and compressed marker will get set automatically+ */+ preslice = toast_fetch_datum_slice(attr, 0, max_size);+ }+ else+ preslice = toast_fetch_datum(attr);
}
else if (VARATT_IS_EXTERNAL_INDIRECT(attr))
{
@@ -1391,7 +1409,7 @@ toast_compress_datum(Datum value)
*/
len = pglz_compress(VARDATA_ANY(DatumGetPointer(value)),
valsize,
- TOAST_COMPRESS_RAWDATA(tmp),+ TOAST_COMPRESS_DATA(tmp),
PGLZ_strategy_default);
if (len >= 0 &&
len + TOAST_COMPRESS_HDRSZ < valsize - 2)
@@ -2031,7 +2049,8 @@ toast_fetch_datum(struct varlena *attr)
* Reconstruct a segment of a Datum from the chunks saved
* in the toast relation
*
- * Note that this function only supports non-compressed external datums.+ * Note that this function supports non-compressed external datums+ * and compressed external datum slices at the start of the object.
* ----------
*/
static struct varlena *
@@ -2072,10 +2091,10 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr);
/*
- * It's nonsense to fetch slices of a compressed datum -- this isn't lo_*- * we can't return a compressed datum which is meaningful to toast later+ * It's meaningful to fetch slices at the start of a compressed datum,+ * since we can decompress the prefix raw data from it.
*/
- Assert(!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer));+ Assert(!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) || 0 == sliceoffset);
attrsize = toast_pointer.va_extsize;
totalchunks = ((attrsize - 1) / TOAST_MAX_CHUNK_SIZE) + 1;
@@ -2086,12 +2105,26 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
length = 0;
}
+ if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) && length > 0)+ {+ /*+ * If we are going to fetch the compressed external datum slice+ * at the start of the object, also should fetch rawsize field used+ * to record the size of the raw data.+ */+ length = length + sizeof(int32);+ }+
if (((sliceoffset + length) > attrsize) || length < 0)
length = attrsize - sliceoffset;
result = (struct varlena *) palloc(length + VARHDRSZ);
- SET_VARSIZE(result, length + VARHDRSZ);+ if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer)) {+ SET_VARSIZE_COMPRESSED(result, length + VARHDRSZ);+ } else {+ SET_VARSIZE(result, length + VARHDRSZ);+ }
if (length == 0)
return result; /* Can save a lot of work at this point! */
@@ -2274,8 +2307,8 @@ toast_decompress_datum(struct varlena *attr)
palloc(TOAST_COMPRESS_RAWSIZE(attr) + VARHDRSZ);
SET_VARSIZE(result, TOAST_COMPRESS_RAWSIZE(attr) + VARHDRSZ);
- if (pglz_decompress(TOAST_COMPRESS_RAWDATA(attr),- VARSIZE(attr) - TOAST_COMPRESS_HDRSZ,+ if (pglz_decompress(TOAST_COMPRESS_DATA(attr),+ TOAST_COMPRESS_SIZE(attr),
VARDATA(result),
TOAST_COMPRESS_RAWSIZE(attr), true) < 0)
elog(ERROR, "compressed data is corrupted");
@@ -2301,7 +2334,7 @@ toast_decompress_datum_slice(struct varlena *attr, int32 slicelength)
result = (struct varlena *) palloc(slicelength + VARHDRSZ);
- rawsize = pglz_decompress(TOAST_COMPRESS_RAWDATA(attr),+ rawsize = pglz_decompress(TOAST_COMPRESS_DATA(attr),
VARSIZE(attr) - TOAST_COMPRESS_HDRSZ,
VARDATA(result),
slicelength, false);
diff --git a/src/common/pg_lzcompress.c b/src/common/pg_lzcompress.cindex 988b3987d0..ac7d66db77 100644--- a/src/common/pg_lzcompress.c+++ b/src/common/pg_lzcompress.c@@ -771,3 +771,29 @@ pglz_decompress(const char *source, int32 slen, char *dest,
*/
return (char *) dp - dest;
}
++++/* ----------+ * pglz_max_compressed_size -+ *+ * Calculate the maximum size of the compressed slice corresponding to the+ * raw slice. Return the maximum size, or total compressed size if maximum+ * size is larger than total compressed size.+ * ----------+ */+int32+pglz_maximum_compressed_size(int32 raw_slice_size, int32 total_compressed_size)+{+ int32 result;++ /*+ * Use int64 to prevent overflow during calculation.+ */+ result = (int32)((int64)raw_slice_size * 9 + 8) / 8;++ /*+ * Note that slice compressed size will never be larger than total compressed size.+ */+ return result > total_compressed_size ? total_compressed_size: result;+}diff --git a/src/include/common/pg_lzcompress.h b/src/include/common/pg_lzcompress.hindex 555576436c..cda3e1d364 100644--- a/src/include/common/pg_lzcompress.h+++ b/src/include/common/pg_lzcompress.h@@ -87,5 +87,6 @@ extern int32 pglz_compress(const char *source, int32 slen, char *dest,
const PGLZ_Strategy *strategy);
extern int32 pglz_decompress(const char *source, int32 slen, char *dest,
int32 rawsize, bool check_complete);
+extern int32 pglz_maximum_compressed_size(int32 raw_slice_size, int32 raw_size);
#endif /* _PG_LZCOMPRESS_H_ */
--
2.20.1
From 34a17fd6df4f531527b976df043aed6fb5b6add0 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas@2ndquadrant.com>
Date: Sat, 6 Jul 2019 16:39:17 +0200
Subject: [PATCH 2/2] review reworks
---
src/backend/access/heap/tuptoaster.c | 47 +++++++++++++++-------------
src/common/pg_lzcompress.c | 19 +++++------
src/include/common/pg_lzcompress.h | 2 +-
3 files changed, 37 insertions(+), 31 deletions(-)
diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.cindex 1eb6ccaca2..809a52196c 100644--- a/src/backend/access/heap/tuptoaster.c+++ b/src/backend/access/heap/tuptoaster.c@@ -62,7 +62,7 @@ typedef struct toast_compress_header
#define TOAST_COMPRESS_HDRSZ ((int32) sizeof(toast_compress_header))
#define TOAST_COMPRESS_RAWSIZE(ptr) (((toast_compress_header *) (ptr))->rawsize)
#define TOAST_COMPRESS_SIZE(ptr) ((int32) VARSIZE(ptr) - TOAST_COMPRESS_HDRSZ)
-#define TOAST_COMPRESS_DATA(ptr) \+#define TOAST_COMPRESS_RAWDATA(ptr) \
(((char *) (ptr)) + TOAST_COMPRESS_HDRSZ)
#define TOAST_COMPRESS_SET_RAWSIZE(ptr, len) \
(((toast_compress_header *) (ptr))->rawsize = (len))
@@ -254,7 +254,7 @@ heap_tuple_untoast_attr(struct varlena *attr)
* Public entry point to get back part of a toasted value
* from compression or external storage.
*
- * Note if slicelength is negative, return suffix of the value.+ * Note: when slicelength is negative, return suffix of the value.
* ----------
*/
struct varlena *
@@ -277,17 +277,23 @@ heap_tuple_untoast_attr_slice(struct varlena *attr,
return toast_fetch_datum_slice(attr, sliceoffset, slicelength);
/*
- * If only need the prefix slice, fetch enough datums to decompress.- * Otherwise, fetch all datums.+ * When a prefix of the value is requested, fetch enough slices to+ * decompress. Otherwise, fetch all slices.
*/
if (slicelength > 0 && sliceoffset >= 0)
{
int32 max_size;
++ /*+ * Determine maximum amount of compressed data needed for a prefix+ * of a given length (after decompression).+ */
max_size = pglz_maximum_compressed_size(sliceoffset + slicelength,
TOAST_COMPRESS_SIZE(attr));
+
/*
- * Be sure to get enough compressed slice- * and compressed marker will get set automatically+ * Fetch enough compressed slices, compressed marker will get set+ * automatically.
*/
preslice = toast_fetch_datum_slice(attr, 0, max_size);
}
@@ -1409,7 +1415,7 @@ toast_compress_datum(Datum value)
*/
len = pglz_compress(VARDATA_ANY(DatumGetPointer(value)),
valsize,
- TOAST_COMPRESS_DATA(tmp),+ TOAST_COMPRESS_RAWDATA(tmp),
PGLZ_strategy_default);
if (len >= 0 &&
len + TOAST_COMPRESS_HDRSZ < valsize - 2)
@@ -2050,7 +2056,8 @@ toast_fetch_datum(struct varlena *attr)
* in the toast relation
*
* Note that this function supports non-compressed external datums
- * and compressed external datum slices at the start of the object.+ * and compressed external datums (in which case the requrested slice+ * has to be a prefix, i.e. sliceoffset has to be 0).
* ----------
*/
static struct varlena *
@@ -2092,7 +2099,8 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
/*
* It's meaningful to fetch slices at the start of a compressed datum,
- * since we can decompress the prefix raw data from it.+ * since we can decompress the prefix raw data without decompressing the+ * whole value.
*/
Assert(!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) || 0 == sliceoffset);
@@ -2105,26 +2113,23 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
length = 0;
}
+ /*+ * When fetching a prefix of a compressed external datum, account for+ * the rawsize tracking amount of raw data (it's stored at the beginning+ * as an int32 value).+ */
if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) && length > 0)
- {- /*- * If we are going to fetch the compressed external datum slice- * at the start of the object, also should fetch rawsize field used- * to record the size of the raw data.- */
length = length + sizeof(int32);
- }
if (((sliceoffset + length) > attrsize) || length < 0)
length = attrsize - sliceoffset;
result = (struct varlena *) palloc(length + VARHDRSZ);
- if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer)) {+ if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer))
SET_VARSIZE_COMPRESSED(result, length + VARHDRSZ);
- } else {+ else
SET_VARSIZE(result, length + VARHDRSZ);
- }
if (length == 0)
return result; /* Can save a lot of work at this point! */
@@ -2307,7 +2312,7 @@ toast_decompress_datum(struct varlena *attr)
palloc(TOAST_COMPRESS_RAWSIZE(attr) + VARHDRSZ);
SET_VARSIZE(result, TOAST_COMPRESS_RAWSIZE(attr) + VARHDRSZ);
- if (pglz_decompress(TOAST_COMPRESS_DATA(attr),+ if (pglz_decompress(TOAST_COMPRESS_RAWDATA(attr),
TOAST_COMPRESS_SIZE(attr),
VARDATA(result),
TOAST_COMPRESS_RAWSIZE(attr), true) < 0)
@@ -2334,7 +2339,7 @@ toast_decompress_datum_slice(struct varlena *attr, int32 slicelength)
result = (struct varlena *) palloc(slicelength + VARHDRSZ);
- rawsize = pglz_decompress(TOAST_COMPRESS_DATA(attr),+ rawsize = pglz_decompress(TOAST_COMPRESS_RAWDATA(attr),
VARSIZE(attr) - TOAST_COMPRESS_HDRSZ,
VARDATA(result),
slicelength, false);
diff --git a/src/common/pg_lzcompress.c b/src/common/pg_lzcompress.cindex ac7d66db77..18766ade02 100644--- a/src/common/pg_lzcompress.c+++ b/src/common/pg_lzcompress.c@@ -773,27 +773,28 @@ pglz_decompress(const char *source, int32 slen, char *dest,
}
-
/* ----------
* pglz_max_compressed_size -
*
- * Calculate the maximum size of the compressed slice corresponding to the- * raw slice. Return the maximum size, or total compressed size if maximum- * size is larger than total compressed size.+ * Calculate the maximum compressed size for a given amount of raw data.+ * Return the maximum size, or total compressed size if maximum size is+ * larger than total compressed size.
* ----------
*/
int32
-pglz_maximum_compressed_size(int32 raw_slice_size, int32 total_compressed_size)+pglz_maximum_compressed_size(int32 rawsize, int32 total_compressed_size)
{
- int32 result;+ int32 compressed_size;
/*
* Use int64 to prevent overflow during calculation.
*/
- result = (int32)((int64)raw_slice_size * 9 + 8) / 8;+ compressed_size = (int32) ((int64) rawsize * 9 + 8) / 8;
/*
- * Note that slice compressed size will never be larger than total compressed size.+ * Maximum compressed size can't be larger than total compressed size.
*/
- return result > total_compressed_size ? total_compressed_size: result;+ compressed_size = Min(compressed_size, total_compressed_size);++ return compressed_size;
}
diff --git a/src/include/common/pg_lzcompress.h b/src/include/common/pg_lzcompress.hindex cda3e1d364..e59fe2eaea 100644--- a/src/include/common/pg_lzcompress.h+++ b/src/include/common/pg_lzcompress.h@@ -87,6 +87,6 @@ extern int32 pglz_compress(const char *source, int32 slen, char *dest,
const PGLZ_Strategy *strategy);
extern int32 pglz_decompress(const char *source, int32 slen, char *dest,
int32 rawsize, bool check_complete);
-extern int32 pglz_maximum_compressed_size(int32 raw_slice_size, int32 raw_size);+extern int32 pglz_maximum_compressed_size(int32 rawsize, int32 total_compressed_size);
#endif /* _PG_LZCOMPRESS_H_ */
--
2.20.1
Attachments:
[text/plain] 0001-Optimize-partial-TOAST-decompression.patch (6.9K, ../20190706152337.l64zhoj3xs3qkfc4@development/2-0001-Optimize-partial-TOAST-decompression.patch)
download | inline diff:
From d50ae41bf1d1c4e2786a55a610300606c1ec43a5 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas@2ndquadrant.com>
Date: Sat, 6 Jul 2019 15:41:37 +0200
Subject: [PATCH 1/2] Optimize partial TOAST decompression
---
src/backend/access/heap/tuptoaster.c | 57 ++++++++++++++++++++++------
src/common/pg_lzcompress.c | 26 +++++++++++++
src/include/common/pg_lzcompress.h | 1 +
3 files changed, 72 insertions(+), 12 deletions(-)
diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.cindex 55d6e91d1c..1eb6ccaca2 100644--- a/src/backend/access/heap/tuptoaster.c+++ b/src/backend/access/heap/tuptoaster.c@@ -61,7 +61,8 @@ typedef struct toast_compress_header
*/
#define TOAST_COMPRESS_HDRSZ ((int32) sizeof(toast_compress_header))
#define TOAST_COMPRESS_RAWSIZE(ptr) (((toast_compress_header *) (ptr))->rawsize)
-#define TOAST_COMPRESS_RAWDATA(ptr) \+#define TOAST_COMPRESS_SIZE(ptr) ((int32) VARSIZE(ptr) - TOAST_COMPRESS_HDRSZ)+#define TOAST_COMPRESS_DATA(ptr) \
(((char *) (ptr)) + TOAST_COMPRESS_HDRSZ)
#define TOAST_COMPRESS_SET_RAWSIZE(ptr, len) \
(((toast_compress_header *) (ptr))->rawsize = (len))
@@ -252,6 +253,8 @@ heap_tuple_untoast_attr(struct varlena *attr)
*
* Public entry point to get back part of a toasted value
* from compression or external storage.
+ *+ * Note if slicelength is negative, return suffix of the value.
* ----------
*/
struct varlena *
@@ -273,8 +276,23 @@ heap_tuple_untoast_attr_slice(struct varlena *attr,
if (!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer))
return toast_fetch_datum_slice(attr, sliceoffset, slicelength);
- /* fetch it back (compressed marker will get set automatically) */- preslice = toast_fetch_datum(attr);+ /*+ * If only need the prefix slice, fetch enough datums to decompress.+ * Otherwise, fetch all datums.+ */+ if (slicelength > 0 && sliceoffset >= 0)+ {+ int32 max_size;+ max_size = pglz_maximum_compressed_size(sliceoffset + slicelength,+ TOAST_COMPRESS_SIZE(attr));+ /*+ * Be sure to get enough compressed slice+ * and compressed marker will get set automatically+ */+ preslice = toast_fetch_datum_slice(attr, 0, max_size);+ }+ else+ preslice = toast_fetch_datum(attr);
}
else if (VARATT_IS_EXTERNAL_INDIRECT(attr))
{
@@ -1391,7 +1409,7 @@ toast_compress_datum(Datum value)
*/
len = pglz_compress(VARDATA_ANY(DatumGetPointer(value)),
valsize,
- TOAST_COMPRESS_RAWDATA(tmp),+ TOAST_COMPRESS_DATA(tmp),
PGLZ_strategy_default);
if (len >= 0 &&
len + TOAST_COMPRESS_HDRSZ < valsize - 2)
@@ -2031,7 +2049,8 @@ toast_fetch_datum(struct varlena *attr)
* Reconstruct a segment of a Datum from the chunks saved
* in the toast relation
*
- * Note that this function only supports non-compressed external datums.+ * Note that this function supports non-compressed external datums+ * and compressed external datum slices at the start of the object.
* ----------
*/
static struct varlena *
@@ -2072,10 +2091,10 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
VARATT_EXTERNAL_GET_POINTER(toast_pointer, attr);
/*
- * It's nonsense to fetch slices of a compressed datum -- this isn't lo_*- * we can't return a compressed datum which is meaningful to toast later+ * It's meaningful to fetch slices at the start of a compressed datum,+ * since we can decompress the prefix raw data from it.
*/
- Assert(!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer));+ Assert(!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) || 0 == sliceoffset);
attrsize = toast_pointer.va_extsize;
totalchunks = ((attrsize - 1) / TOAST_MAX_CHUNK_SIZE) + 1;
@@ -2086,12 +2105,26 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
length = 0;
}
+ if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) && length > 0)+ {+ /*+ * If we are going to fetch the compressed external datum slice+ * at the start of the object, also should fetch rawsize field used+ * to record the size of the raw data.+ */+ length = length + sizeof(int32);+ }+
if (((sliceoffset + length) > attrsize) || length < 0)
length = attrsize - sliceoffset;
result = (struct varlena *) palloc(length + VARHDRSZ);
- SET_VARSIZE(result, length + VARHDRSZ);+ if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer)) {+ SET_VARSIZE_COMPRESSED(result, length + VARHDRSZ);+ } else {+ SET_VARSIZE(result, length + VARHDRSZ);+ }
if (length == 0)
return result; /* Can save a lot of work at this point! */
@@ -2274,8 +2307,8 @@ toast_decompress_datum(struct varlena *attr)
palloc(TOAST_COMPRESS_RAWSIZE(attr) + VARHDRSZ);
SET_VARSIZE(result, TOAST_COMPRESS_RAWSIZE(attr) + VARHDRSZ);
- if (pglz_decompress(TOAST_COMPRESS_RAWDATA(attr),- VARSIZE(attr) - TOAST_COMPRESS_HDRSZ,+ if (pglz_decompress(TOAST_COMPRESS_DATA(attr),+ TOAST_COMPRESS_SIZE(attr),
VARDATA(result),
TOAST_COMPRESS_RAWSIZE(attr), true) < 0)
elog(ERROR, "compressed data is corrupted");
@@ -2301,7 +2334,7 @@ toast_decompress_datum_slice(struct varlena *attr, int32 slicelength)
result = (struct varlena *) palloc(slicelength + VARHDRSZ);
- rawsize = pglz_decompress(TOAST_COMPRESS_RAWDATA(attr),+ rawsize = pglz_decompress(TOAST_COMPRESS_DATA(attr),
VARSIZE(attr) - TOAST_COMPRESS_HDRSZ,
VARDATA(result),
slicelength, false);
diff --git a/src/common/pg_lzcompress.c b/src/common/pg_lzcompress.cindex 988b3987d0..ac7d66db77 100644--- a/src/common/pg_lzcompress.c+++ b/src/common/pg_lzcompress.c@@ -771,3 +771,29 @@ pglz_decompress(const char *source, int32 slen, char *dest,
*/
return (char *) dp - dest;
}
++++/* ----------+ * pglz_max_compressed_size -+ *+ * Calculate the maximum size of the compressed slice corresponding to the+ * raw slice. Return the maximum size, or total compressed size if maximum+ * size is larger than total compressed size.+ * ----------+ */+int32+pglz_maximum_compressed_size(int32 raw_slice_size, int32 total_compressed_size)+{+ int32 result;++ /*+ * Use int64 to prevent overflow during calculation.+ */+ result = (int32)((int64)raw_slice_size * 9 + 8) / 8;++ /*+ * Note that slice compressed size will never be larger than total compressed size.+ */+ return result > total_compressed_size ? total_compressed_size: result;+}diff --git a/src/include/common/pg_lzcompress.h b/src/include/common/pg_lzcompress.hindex 555576436c..cda3e1d364 100644--- a/src/include/common/pg_lzcompress.h+++ b/src/include/common/pg_lzcompress.h@@ -87,5 +87,6 @@ extern int32 pglz_compress(const char *source, int32 slen, char *dest,
const PGLZ_Strategy *strategy);
extern int32 pglz_decompress(const char *source, int32 slen, char *dest,
int32 rawsize, bool check_complete);
+extern int32 pglz_maximum_compressed_size(int32 raw_slice_size, int32 raw_size);
#endif /* _PG_LZCOMPRESS_H_ */
--
2.20.1
[text/plain] 0002-review-reworks.patch (7.4K, ../20190706152337.l64zhoj3xs3qkfc4@development/3-0002-review-reworks.patch)
download | inline diff:
From 34a17fd6df4f531527b976df043aed6fb5b6add0 Mon Sep 17 00:00:00 2001
From: Tomas Vondra <tomas@2ndquadrant.com>
Date: Sat, 6 Jul 2019 16:39:17 +0200
Subject: [PATCH 2/2] review reworks
---
src/backend/access/heap/tuptoaster.c | 47 +++++++++++++++-------------
src/common/pg_lzcompress.c | 19 +++++------
src/include/common/pg_lzcompress.h | 2 +-
3 files changed, 37 insertions(+), 31 deletions(-)
diff --git a/src/backend/access/heap/tuptoaster.c b/src/backend/access/heap/tuptoaster.cindex 1eb6ccaca2..809a52196c 100644--- a/src/backend/access/heap/tuptoaster.c+++ b/src/backend/access/heap/tuptoaster.c@@ -62,7 +62,7 @@ typedef struct toast_compress_header
#define TOAST_COMPRESS_HDRSZ ((int32) sizeof(toast_compress_header))
#define TOAST_COMPRESS_RAWSIZE(ptr) (((toast_compress_header *) (ptr))->rawsize)
#define TOAST_COMPRESS_SIZE(ptr) ((int32) VARSIZE(ptr) - TOAST_COMPRESS_HDRSZ)
-#define TOAST_COMPRESS_DATA(ptr) \+#define TOAST_COMPRESS_RAWDATA(ptr) \
(((char *) (ptr)) + TOAST_COMPRESS_HDRSZ)
#define TOAST_COMPRESS_SET_RAWSIZE(ptr, len) \
(((toast_compress_header *) (ptr))->rawsize = (len))
@@ -254,7 +254,7 @@ heap_tuple_untoast_attr(struct varlena *attr)
* Public entry point to get back part of a toasted value
* from compression or external storage.
*
- * Note if slicelength is negative, return suffix of the value.+ * Note: when slicelength is negative, return suffix of the value.
* ----------
*/
struct varlena *
@@ -277,17 +277,23 @@ heap_tuple_untoast_attr_slice(struct varlena *attr,
return toast_fetch_datum_slice(attr, sliceoffset, slicelength);
/*
- * If only need the prefix slice, fetch enough datums to decompress.- * Otherwise, fetch all datums.+ * When a prefix of the value is requested, fetch enough slices to+ * decompress. Otherwise, fetch all slices.
*/
if (slicelength > 0 && sliceoffset >= 0)
{
int32 max_size;
++ /*+ * Determine maximum amount of compressed data needed for a prefix+ * of a given length (after decompression).+ */
max_size = pglz_maximum_compressed_size(sliceoffset + slicelength,
TOAST_COMPRESS_SIZE(attr));
+
/*
- * Be sure to get enough compressed slice- * and compressed marker will get set automatically+ * Fetch enough compressed slices, compressed marker will get set+ * automatically.
*/
preslice = toast_fetch_datum_slice(attr, 0, max_size);
}
@@ -1409,7 +1415,7 @@ toast_compress_datum(Datum value)
*/
len = pglz_compress(VARDATA_ANY(DatumGetPointer(value)),
valsize,
- TOAST_COMPRESS_DATA(tmp),+ TOAST_COMPRESS_RAWDATA(tmp),
PGLZ_strategy_default);
if (len >= 0 &&
len + TOAST_COMPRESS_HDRSZ < valsize - 2)
@@ -2050,7 +2056,8 @@ toast_fetch_datum(struct varlena *attr)
* in the toast relation
*
* Note that this function supports non-compressed external datums
- * and compressed external datum slices at the start of the object.+ * and compressed external datums (in which case the requrested slice+ * has to be a prefix, i.e. sliceoffset has to be 0).
* ----------
*/
static struct varlena *
@@ -2092,7 +2099,8 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
/*
* It's meaningful to fetch slices at the start of a compressed datum,
- * since we can decompress the prefix raw data from it.+ * since we can decompress the prefix raw data without decompressing the+ * whole value.
*/
Assert(!VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) || 0 == sliceoffset);
@@ -2105,26 +2113,23 @@ toast_fetch_datum_slice(struct varlena *attr, int32 sliceoffset, int32 length)
length = 0;
}
+ /*+ * When fetching a prefix of a compressed external datum, account for+ * the rawsize tracking amount of raw data (it's stored at the beginning+ * as an int32 value).+ */
if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer) && length > 0)
- {- /*- * If we are going to fetch the compressed external datum slice- * at the start of the object, also should fetch rawsize field used- * to record the size of the raw data.- */
length = length + sizeof(int32);
- }
if (((sliceoffset + length) > attrsize) || length < 0)
length = attrsize - sliceoffset;
result = (struct varlena *) palloc(length + VARHDRSZ);
- if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer)) {+ if (VARATT_EXTERNAL_IS_COMPRESSED(toast_pointer))
SET_VARSIZE_COMPRESSED(result, length + VARHDRSZ);
- } else {+ else
SET_VARSIZE(result, length + VARHDRSZ);
- }
if (length == 0)
return result; /* Can save a lot of work at this point! */
@@ -2307,7 +2312,7 @@ toast_decompress_datum(struct varlena *attr)
palloc(TOAST_COMPRESS_RAWSIZE(attr) + VARHDRSZ);
SET_VARSIZE(result, TOAST_COMPRESS_RAWSIZE(attr) + VARHDRSZ);
- if (pglz_decompress(TOAST_COMPRESS_DATA(attr),+ if (pglz_decompress(TOAST_COMPRESS_RAWDATA(attr),
TOAST_COMPRESS_SIZE(attr),
VARDATA(result),
TOAST_COMPRESS_RAWSIZE(attr), true) < 0)
@@ -2334,7 +2339,7 @@ toast_decompress_datum_slice(struct varlena *attr, int32 slicelength)
result = (struct varlena *) palloc(slicelength + VARHDRSZ);
- rawsize = pglz_decompress(TOAST_COMPRESS_DATA(attr),+ rawsize = pglz_decompress(TOAST_COMPRESS_RAWDATA(attr),
VARSIZE(attr) - TOAST_COMPRESS_HDRSZ,
VARDATA(result),
slicelength, false);
diff --git a/src/common/pg_lzcompress.c b/src/common/pg_lzcompress.cindex ac7d66db77..18766ade02 100644--- a/src/common/pg_lzcompress.c+++ b/src/common/pg_lzcompress.c@@ -773,27 +773,28 @@ pglz_decompress(const char *source, int32 slen, char *dest,
}
-
/* ----------
* pglz_max_compressed_size -
*
- * Calculate the maximum size of the compressed slice corresponding to the- * raw slice. Return the maximum size, or total compressed size if maximum- * size is larger than total compressed size.+ * Calculate the maximum compressed size for a given amount of raw data.+ * Return the maximum size, or total compressed size if maximum size is+ * larger than total compressed size.
* ----------
*/
int32
-pglz_maximum_compressed_size(int32 raw_slice_size, int32 total_compressed_size)+pglz_maximum_compressed_size(int32 rawsize, int32 total_compressed_size)
{
- int32 result;+ int32 compressed_size;
/*
* Use int64 to prevent overflow during calculation.
*/
- result = (int32)((int64)raw_slice_size * 9 + 8) / 8;+ compressed_size = (int32) ((int64) rawsize * 9 + 8) / 8;
/*
- * Note that slice compressed size will never be larger than total compressed size.+ * Maximum compressed size can't be larger than total compressed size.
*/
- return result > total_compressed_size ? total_compressed_size: result;+ compressed_size = Min(compressed_size, total_compressed_size);++ return compressed_size;
}
diff --git a/src/include/common/pg_lzcompress.h b/src/include/common/pg_lzcompress.hindex cda3e1d364..e59fe2eaea 100644--- a/src/include/common/pg_lzcompress.h+++ b/src/include/common/pg_lzcompress.h@@ -87,6 +87,6 @@ extern int32 pglz_compress(const char *source, int32 slen, char *dest,
const PGLZ_Strategy *strategy);
extern int32 pglz_decompress(const char *source, int32 slen, char *dest,
int32 rawsize, bool check_complete);
-extern int32 pglz_maximum_compressed_size(int32 raw_slice_size, int32 raw_size);+extern int32 pglz_maximum_compressed_size(int32 rawsize, int32 total_compressed_size);
#endif /* _PG_LZCOMPRESS_H_ */
--
2.20.1
[application/x-sh] random-bench2.sh (1.6K, ../20190706152337.l64zhoj3xs3qkfc4@development/4-random-bench2.sh)
download
[application/vnd.oasis.opendocument.spreadsheet] comparison.ods (48.6K, ../20190706152337.l64zhoj3xs3qkfc4@development/5-comparison.ods)
download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tomas.vondra@2ndquadrant.com, djydewang@gmail.com, x4mmm@yandex-team.ru, pramsey@cleverelephant.ca
Subject: Re: Optimize partial TOAST decompression
In-Reply-To: <20190706152337.l64zhoj3xs3qkfc4@development>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox