Re: Changes to stringinfo.c

Hi,

I'm just looking at the changed code in commit
3147acd63e0135aff9a6c4b01d861251925d97d9 and I'm wondering if we should
perhaps test the performance of this before assuming too much that it is an
improvement. I'm a bit concerned that now if there is not enough space in
the buffer that we only now allocate what is needed, whereas before we
would double the buffer's size. I guess this will save memory in many
cases, but I'm a bit worried that we'll see quite a big drop in performance
when we next try to append to the string and have to reallocate space again.

In my patch I ended up only using the value from appendStringInfoVA as a
guide, where I still doubled the buffer if "needed" was less than the
current buffer size, but if it was larger I kept doubling the buffer size
until it was bigger than "needed".

I don't yet have any examples of work loads where I suspect this will slow
down, but I guess any strings that are built using appendStringInfo that go
over 1024 bytes could see a bit of a slow down since it will have to
allocate space for each append after the 1024 byte mark.

Any thoughts on this?

Regards

David

David Rowley <dgrowleyml(at)gmail(dot)com> writes:
> I'm just looking at the changed code in commit
> 3147acd63e0135aff9a6c4b01d861251925d97d9 and I'm wondering if we should
> perhaps test the performance of this before assuming too much that it is an
> improvement. I'm a bit concerned that now if there is not enough space in
> the buffer that we only now allocate what is needed, whereas before we
> would double the buffer's size. I guess this will save memory in many
> cases, but I'm a bit worried that we'll see quite a big drop in performance
> when we next try to append to the string and have to reallocate space again.

Hm? enlargeStringInfo() still enforces the doubling behavior, AFAICS.
I don't see value in doubling the needed-space estimate before that.

regards, tom lane

On Sun, Oct 27, 2013 at 3:04 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:

> David Rowley <dgrowleyml(at)gmail(dot)com> writes:
> > I'm just looking at the changed code in commit
> > 3147acd63e0135aff9a6c4b01d861251925d97d9 and I'm wondering if we should
> > perhaps test the performance of this before assuming too much that it is
> an
> > improvement. I'm a bit concerned that now if there is not enough space in
> > the buffer that we only now allocate what is needed, whereas before we
> > would double the buffer's size. I guess this will save memory in many
> > cases, but I'm a bit worried that we'll see quite a big drop in
> performance
> > when we next try to append to the string and have to reallocate space
> again.
>
> Hm? enlargeStringInfo() still enforces the doubling behavior, AFAICS.
> I don't see value in doubling the needed-space estimate before that.
>
>
Oops, you're right. Sorry for the noise.

> regards, tom lane
>