Re: Strange Bitmapset manipulation in DiscreteKnapsack()

Hi,

David Rowley <dgrowleyml@gmail.com> writes:

> Given that the code original code was written in a very deliberate way
> to avoid reallocations, I now think that we should maintain that.
>
> I've attached a patch which adds bms_replace_members(). It's basically
> like bms_copy() but attempts to reuse the member from another set. I
> considered if the new function should be called bms_copy_inplace(),
> but left it as bms_replace_members() for now.

I find the following code in DiscreteKnapsack is weird.


	for (i = 0; i <= max_weight; ++i)
	{
		values[i] = 0;

** memory allocation here, and the num_items bit is removed later **
	
		sets[i] = bms_make_singleton(num_items);
	}


        ** num_items bit is removed here **
	result = bms_del_member(bms_copy(sets[max_weight]), num_items);

I can't access the github.com now so I can't test my idea, but basiclly
I think we may need some improvement here. like 'sets[i] = NULL;' at the
first and remove the bms_del_member later.

-- 
Best Regards
Andy Fan