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

On Tue, Apr 26, 2011 at 3:19 PM, Merlin Moncure <mmoncure(at)gmail(dot)com> wrote:
> On Tue, Apr 26, 2011 at 1:48 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>> I wrote:
>>> A different angle of attack on the issue is that aset.c's use of
>>> exact-power-of-2 sizes for both malloc requests and the available space
>>> in chunks is inefficient when maxBlockSize is constrained to be not much
>>> larger than common chunk request sizes.  Maybe we should try to fix that
>>> instead of asking other code to choose request sizes to dodge the
>>> inefficiency.
>>
>> After chewing on that thought for a bit, it seems like an easy fix is to
>> modify AllocSetContextCreate (around line 390 in HEAD's aset.c) so that
>> allocChunkLimit is not just constrained to be less than maxBlockSize,
>> but significantly less than maxBlockSize --- say an eighth or so.  When
>> using ALLOCSET_SMALL_MAXSIZE this would result in requests of 1K or more
>> being treated as separate malloc blocks, in turn guaranteeing that not
>> more than 1K of each 8K request block could go to waste in the face of a
>> stream of allocChunkLimit-sized requests.
>>
>> Of course, this might just result in the fragmentation problem getting
>> tossed over the fence into malloc's playground ... but offhand it seems
>> that it would remove the issue that spell.c is concerned about.
>
> well, +1 on any solution that doesn't push having to make assumptions
> about the allocator from the outside.  your fix seems to nail it
> without having to tinker around with the api which is nice. (plus you
> could just remove the comment).
>
> Some perfunctory probing didn't turn up any other cases like this.

patch attached -- I did no testing beyond make check though. I
suppose changes to the allocator are not to be take lightly and this
should really be tested in some allocation heavy scenarios.

merlin