Re: Strange Bitmapset manipulation in DiscreteKnapsack()

From: Richard Guo <guofenglinux(at)gmail(dot)com>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: PostgreSQL Developers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Strange Bitmapset manipulation in DiscreteKnapsack()
Date: 2024-01-18 02:22:24
Message-ID: CAMbWs4_ka474zs2z7gfG0LiT5F61+HGAZqHYPeGFHKW5JwM4GQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, Jan 18, 2024 at 8:35 AM David Rowley <dgrowleyml(at)gmail(dot)com> wrote:

> 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.".

Ah, I neglected to notice this when reviewing the v1 patch. I guess
it's implemented this way due to performance considerations, right?

> 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.

Do you think we can use 'memcpy(a, b, BITMAPSET_SIZE(b->nwords))'
directly in the new bms_replace_members() instead of copying the
bitmapwords one by one, like:

- i = 0;
- do
- {
- a->words[i] = b->words[i];
- } while (++i < b->nwords);
-
- a->nwords = b->nwords;
+ memcpy(a, b, BITMAPSET_SIZE(b->nwords));

But I'm not sure if this is an improvement or not.

Thanks
Richard

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2024-01-18 03:24:51 Re: Strange Bitmapset manipulation in DiscreteKnapsack()
Previous Message Michael Paquier 2024-01-18 02:20:17 Re: Add system identifier to backup manifest