Re: Some memory allocations in gin fastupdate code are a bit brain dead - Mailing list pgsql-hackers

From David Rowley
Subject Re: Some memory allocations in gin fastupdate code are a bit brain dead
Date
Msg-id CAKJS1f-5YgdaCH3SyhPW1WM+d+1gnVibdZRdDdNfP7kZxa88Nw@mail.gmail.com
Whole thread Raw
In response to Re: Some memory allocations in gin fastupdate code are a bit brain dead  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: Some memory allocations in gin fastupdate code are a bit brain dead
List pgsql-hackers
On Wed, 19 Dec 2018 at 04:24, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> David Rowley <david.rowley@2ndquadrant.com> writes:
> > I recently stumbled upon the following code in ginfast.c:
> > while (collector->ntuples + nentries > collector->lentuples)
> > {
> >     collector->lentuples *= 2;
> >     collector->tuples = (IndexTuple *) repalloc(collector->tuples,
> >       sizeof(IndexTuple) * collector->lentuples);
> > }
>
> I agree that's pretty stupid, but I wonder whether we should also try
> harder in the initial-allocation path.  Right now, if the initial
> pass through this code sees say 3 tuples to insert, it will then work
> with 3 * 2^n entries in subsequent enlargements, which is likely to
> be pretty inefficient.  Should we try to force the initial allocation
> to be a power of 2?

Yeah, I think that's a good idea.

> Also, do we need to worry about integer overflow?

Good point.  I didn't think of it before, but the previous code would
have ensured that we got an ERROR before any overflow could have
occurred as the repalloc() would fail once the allocation request went
beyond MaxAllocSize. However, I think an overflow is sufficiently
unlikely that we don't need to do any batching, but we should keep it
an error and not a crash.

I propose the attached.  If we hit the overflow case, we'll still
attempt the repalloc(), but it'll fail, just as it would have before,
just without the needless calls.

-- 
 David Rowley                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services

Attachment

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: insensitive collations
Next
From: Tom Lane
Date:
Subject: Re: Use an enum for RELKIND_*?