Re: Bad COMPACT_ALLOC_CHUNK size in tsearch/spell.c?

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Merlin Moncure <mmoncure(at)gmail(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Bad COMPACT_ALLOC_CHUNK size in tsearch/spell.c?
Date: 2011-04-26 16:46:02
Message-ID: 8819.1303836362@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

I wrote:
> Hmm ... the idea behind that comment is evidently that it's best if the
> blocks allocated by this code form independent malloc blocks in aset.c;
> but right offhand I can't see why that'd be important. It's obviously
> not functionally necessary, since the code works as-is, and I don't
> immediately see a reason to think that it'd be more efficient. If
> anything it might be best the way it is, with the allocation
> corresponding to the largest allowable small-chunk size.

> I'm thinking the comment is just brain fade ... which is annoying
> because I have a feeling it's my own comment :-( ... but I'm darned
> if I can reconstruct the reasoning for it now.

After a bit longer, the reasoning came back to me ...

The key point here is that alloc requests larger than ALLOC_CHUNK_LIMIT
don't affect aset.c's other allocation behavior: it hands you out a
block gotten directly from malloc, end of story. However, if you keep
on asking for chunks <= ALLOC_CHUNK_LIMIT, those will come out of larger
and larger malloc'd blocks. That means larger and larger potential
wastage when you stop asking for chunks. In normal usage this doesn't
matter a whole lot because if you stop asking for space in a context,
you're probably going to release it not long after that. But ispell
is building a dictionary that will likely live for the duration of the
process, so if it wastes space comparable to the useful size of the
dictionary, that's a tad more annoying.

In short: the property suggested by the comment is meant to minimize
wasted memory, and we're not doing it right to achieve that.

Now on the other hand, it probably takes more cycles to build it using
a number of individual malloc calls than the way we're doing it now.
But since it's a once-per-process overhead, I'm thinking that's probably
a good tradeoff.

So now I think the comment is right and we need to bump up the value of
the constant.

regards, tom lane

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Merlin Moncure 2011-04-26 16:58:03 Re: Bad COMPACT_ALLOC_CHUNK size in tsearch/spell.c?
Previous Message Leonardo Sápiras 2011-04-26 16:39:39 GSoC 2011 - New phpPgAdmin Plugin Architecture