Compressed TOAST Slicing

classic Classic list List threaded Threaded
8 messages Options
Reply | Threaded
Open this post in threaded view
|

Compressed TOAST Slicing

Paul Ramsey
Currently, PG_DETOAST_DATUM_SLICE when run on a compressed TOAST entry will first decompress the whole object, then extract the relevant slice. 

When the desired slice is at or near the front of the object, this is obviously non-optimal. 

The attached patch adds in a code path to do a partial decompression of the TOAST entry, when the requested slice is at the start of the object.

For an example of the improvement possible, this trivial example:

    create table slicingtest (
      id serial primary key,
      a text
    );

    insert into slicingtest (a) select repeat('xyz123', 10000) as a from generate_series(1,10000);
    \timing
    select sum(length(substr(a, 0, 20))) from slicingtest;

On master, in the current state on my wee laptop, I get

    Time: 1426.737 ms (00:01.427)

With the patch, on my wee laptop, I get

    Time: 46.886 ms

As usual, doing less work is faster.

Interesting note to motivate a follow-on patch: the substr() function does attempt to slice, but the left() function does not. So, if this patch is accepted, next patch will be to left() to add slicing behaviour.

If nobody lights me on fire, I'll submit to commitfest shortly.

P.

compressed-datum-slicing-20190101a.patch (7K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Compressed TOAST Slicing

Stephen Frost
Greetings,

* Paul Ramsey ([hidden email]) wrote:
> The attached patch adds in a code path to do a partial decompression of the
> TOAST entry, when the requested slice is at the start of the object.

Neat!

> As usual, doing less work is faster.

Definitely.

> Interesting note to motivate a follow-on patch: the substr() function does
> attempt to slice, but the left() function does not. So, if this patch is
> accepted, next patch will be to left() to add slicing behaviour.

Makes sense to me.

There two things that I wonder about in the patch- if it would be of any
use to try and allocate on a need basis instead of just allocating the
whole chunk up to the toast size, and secondly, why we wouldn't consider
handling a non-zero offset.  A non-zero offset would, of course, still
require decompressing from the start and then just throwing away what we
skip over, but we're going to be doing that anyway, aren't we?  Why not
stop when we get to the end, at least, and save ourselves the trouble of
decompressing the rest and then throwing it away.

> If nobody lights me on fire, I'll submit to commitfest shortly.

Sounds like a good idea to me.

Thanks!

Stephen

signature.asc (836 bytes) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Compressed TOAST Slicing

Paul Ramsey


On Thu, Nov 1, 2018 at 2:29 PM Stephen Frost <[hidden email]> wrote:
Greetings,

* Paul Ramsey ([hidden email]) wrote:
> The attached patch adds in a code path to do a partial decompression of the
> TOAST entry, when the requested slice is at the start of the object.

There two things that I wonder about in the patch- if it would be of any
use to try and allocate on a need basis instead of just allocating the
whole chunk up to the toast size,

I'm not sure what I was thinking when I rejected allocating the slice size in favour of the whole uncompressed size... I cannot see why that wouldn't work.
 
and secondly, why we wouldn't consider
handling a non-zero offset.  A non-zero offset would, of course, still
require decompressing from the start and then just throwing away what we
skip over, but we're going to be doing that anyway, aren't we?  Why not
stop when we get to the end, at least, and save ourselves the trouble of
decompressing the rest and then throwing it away.

I was worried about changing the pg_lz code too much because it scared me, but debugging some stuff made me read it more closely so I fear it less now, and doing interior slices seems not unreasonable, so I will give it a try.
 
P

Reply | Threaded
Open this post in threaded view
|

Re: Compressed TOAST Slicing

Tom Lane-2
Paul Ramsey <[hidden email]> writes:
> On Thu, Nov 1, 2018 at 2:29 PM Stephen Frost <[hidden email]> wrote:
>> and secondly, why we wouldn't consider
>> handling a non-zero offset.  A non-zero offset would, of course, still
>> require decompressing from the start and then just throwing away what we
>> skip over, but we're going to be doing that anyway, aren't we?  Why not
>> stop when we get to the end, at least, and save ourselves the trouble of
>> decompressing the rest and then throwing it away.

> I was worried about changing the pg_lz code too much because it scared me,
> but debugging some stuff made me read it more closely so I fear it less
> now, and doing interior slices seems not unreasonable, so I will give it a
> try.

I think Stephen was just envisioning decompressing from offset 0 up to
the end of what's needed, and then discarding any data before the start
of what's needed; at least, that's what'd occurred to me.  It sounds like
you were thinking about hacking pg_lz to not write the leading data
anywhere.  While that'd likely be a win for cases where there was leading
data to discard, I'm worried about adding any cycles to the inner loops
of the decompressor.  We don't want to pessimize every other use of pg_lz
to buy a little bit for these cases.

                        regards, tom lane

Reply | Threaded
Open this post in threaded view
|

Re: Compressed TOAST Slicing

Paul Ramsey


On Thu, Nov 1, 2018 at 4:02 PM Tom Lane <[hidden email]> wrote:
Paul Ramsey <[hidden email]> writes:
> On Thu, Nov 1, 2018 at 2:29 PM Stephen Frost <[hidden email]> wrote:
>> and secondly, why we wouldn't consider
>> handling a non-zero offset.  A non-zero offset would, of course, still
>> require decompressing from the start and then just throwing away what we
>> skip over, but we're going to be doing that anyway, aren't we?  Why not
>> stop when we get to the end, at least, and save ourselves the trouble of
>> decompressing the rest and then throwing it away.

> I was worried about changing the pg_lz code too much because it scared me,
> but debugging some stuff made me read it more closely so I fear it less
> now, and doing interior slices seems not unreasonable, so I will give it a
> try.

I think Stephen was just envisioning decompressing from offset 0 up to
the end of what's needed, and then discarding any data before the start
of what's needed; at least, that's what'd occurred to me. 

Understood, that makes lots of sense and is a very small change, it turns out :)
Allocating just what is needed also makes things faster yet, which is nice, and no big surprise.
Some light testing seems to show no obvious regression in speed of decompression for the usual "decompress it all" case.

P
 

compressed-datum-slicing-20190102a.patch (7K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Compressed TOAST Slicing

Paul Ramsey
As threatened, I have also added a patch to left() to also use sliced access.

compressed-datum-slicing-20190102a.patch (7K) Download Attachment
compressed-datum-slicing-left-20190102a.patch (1K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: Compressed TOAST Slicing

rafia.sabih
On Fri, Nov 2, 2018 at 11:55 PM Paul Ramsey <[hidden email]> wrote:
>
> As threatened, I have also added a patch to left() to also use sliced access.

Hi Paul,

The idea looks good and believing your performance evaluation it seems
like a practical one too.

I had a look at this patch and here are my initial comments,
1.
- if (dp != destend || sp != srcend)
+ if (!is_slice && (dp != destend || sp != srcend))
  return -1;

A comment explaining how this check differs for is_slice case would be helpful.
2.
- int len = VARSIZE_ANY_EXHDR(str);
- int n = PG_GETARG_INT32(1);
- int rlen;
+ int n = PG_GETARG_INT32(1);

Looks like PG indentation is not followed here for n.

--
Regards,
Rafia Sabih
EnterpriseDB: http://www.enterprisedb.com/

Reply | Threaded
Open this post in threaded view
|

Re: Compressed TOAST Slicing

Paul Ramsey
On Sun, Dec 2, 2018 at 7:03 AM Rafia Sabih <[hidden email]> wrote:
>
> The idea looks good and believing your performance evaluation it seems
> like a practical one too.

Thank you kindly for the review!

> A comment explaining how this check differs for is_slice case would be helpful.
> Looks like PG indentation is not followed here for n.

I have attached updated patches that add the comment and adhere to the Pg variable declaration indentation styles,
ATB!
P

--
Paul Ramsey


compressed-datum-slicing-left-20190103a.patch (1K) Download Attachment
compressed-datum-slicing-20190103a.patch (7K) Download Attachment