Re: Strange Bitmapset manipulation in DiscreteKnapsack()

Hi,

David Rowley <dgrowleyml(at)gmail(dot)com> writes:

> On Tue, 16 Jan 2024 at 16:32, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
>>
>>
>> Now that 00b41463c changed Bitmapset to have NULL be the only valid
>> representation of an empty set, this code no longer makes sense. We
>> may as well just bms_free() the original set and bms_copy() in the new
>> set as the bms_del_members() call will always pfree the set anyway.

I want to know if "user just want to zero out the flags in bitmapset
but keeping the memory allocation" is a valid requirement. Commit
00b41463c makes it is hard IIUC. The user case I have is I want to
keep the detoast datum in slot->tts_values[1] so that any further
access doesn't need to detoast it again, I used a 'Bitmapset' in
TupleTableSlot which shows which attributes is detoast. all of the
detoast values should be pfree-d in ExecClearTuple. However if a
bms_free the bitmapset everytime in ExecClearTuple and allocate the
memory again later makes some noticable performance regression (5%
difference in my workload). That is still a open items for that patch.

> ...

> The functions's header comment mentions "The bitmapsets are all
> pre-initialized with an unused high bit so that memory allocation is
> done only once.".
> NOTICE: DiscreteKnapsack: frees = 110, max_weight = 60, extra = 183.33%
> NOTICE: DiscreteKnapsack: frees = 110, max_weight = 60, extra = 183.33%
>
> and by the looks of the code, the worst case is much worse.
>

Looks like this is another user case of "user just wants to zero out the
flags in bitmapset but keeping the memory allocation".

[1] https://www.postgresql.org/message-id/flat/87il4jrk1l(dot)fsf(at)163(dot)com

--
Best Regards
Andy Fan