From: | Merlin Moncure <mmoncure(at)gmail(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: Bad COMPACT_ALLOC_CHUNK size in tsearch/spell.c? |
Date: | 2011-04-26 21:19:02 |
Message-ID: | BANLkTinVoSGF1yii6hBtdD9ZGmxiBzxyeQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
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
Attachment | Content-Type | Size |
---|---|---|
chunk.diff | application/octet-stream | 1.9 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Tomáš Pospíšil | 2011-04-26 21:21:08 | GSoC greetings and little question at begining |
Previous Message | Robert Haas | 2011-04-26 21:17:58 | Re: Proposal - asynchronous functions |