Postgres Pro
Downloads
Home   >   mailing lists

Thread: Improve code in tidbitmap.c

Improve code in tidbitmap.c

From
"Etsuro Fujita"
Date:
<div class="WordSection1"><p class="MsoNormal"><span lang="EN-US" style="font-size:10.0pt">Hi,</span><p
class="MsoNormal"><spanlang="EN-US" style="font-size:10.0pt"> </span><p class="MsoNormal"><span lang="EN-US"
style="font-size:10.0pt">ISTMthe code in tidbitmap.c should be improved.  Patch attached.  I think this patch increases
theefficiency a bit.</span><p class="MsoNormal"><span lang="EN-US" style="font-size:10.0pt"> </span><p
class="MsoNormal"><spanlang="EN-US">Thanks,</span><p class="MsoNormal"><span lang="EN-US"> </span><p
class="MsoNormal"><spanlang="EN-US">Best regards,</span><p class="MsoNormal"><span lang="EN-US">Etsuro
Fujita</span></div>

Re: Improve code in tidbitmap.c

From
"Etsuro Fujita"
Date:
I'd like to do the complementary explanation of this.

In tbm_union_page() and tbm_intersect_page() in tidbitmap.c, WORDS_PER_PAGE
is used in the scan of a lossy chunk, instead of WORDS_PER_CHUNK, where
these macros are defined as:

/* number of active words for an exact page: */
#define WORDS_PER_PAGE  ((MAX_TUPLES_PER_PAGE - 1) / BITS_PER_BITMAPWORD +
1)
/* number of active words for a lossy chunk: */
#define WORDS_PER_CHUNK  ((PAGES_PER_CHUNK - 1) / BITS_PER_BITMAPWORD + 1)

Though the use of WORDS_PER_PAGE in the scan of a lossy chunk is logically
correct since these macros implicitly satisfy that WORDS_PER_CHUNK <
WORDS_PER_PAGE, I think WORDS_PER_CHUNK should be used in the scan of a
lossy chunk for code readability and maintenance.  So, I submitted a patch
working in such a way in an earlier email.

I think that, as a secondary effect of the patch, the scan would be
performed a bit efficiently.

I'll add the patch to the 2013-11 CF.  Any comments are welcome.

Thanks,

Best regards,
Etsuro Fujita

Re: Improve code in tidbitmap.c

From
David Rowley
Date:
On Fri, Nov 15, 2013 at 12:50 AM, Etsuro Fujita <fujita.etsuro@lab.ntt.co.jp> wrote:

I think that, as a secondary effect of the patch, the scan would be
performed a bit efficiently.


Hi Etsuro,

Would you be able to supply some benchmarks of a work load which benefits from this change?
Something which compares patched and unpatched versions of head would be really good.

Regards

David Rowley
 
I'll add the patch to the 2013-11 CF.  Any comments are welcome.

Thanks,

Best regards,
Etsuro Fujita



--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Re: Improve code in tidbitmap.c

From
Tom Lane
Date:
"Etsuro Fujita" <fujita.etsuro@lab.ntt.co.jp> writes:
> I'd like to do the complementary explanation of this.
> In tbm_union_page() and tbm_intersect_page() in tidbitmap.c, WORDS_PER_PAGE
> is used in the scan of a lossy chunk, instead of WORDS_PER_CHUNK, where
> these macros are defined as:

> /* number of active words for an exact page: */
> #define WORDS_PER_PAGE  ((MAX_TUPLES_PER_PAGE - 1) / BITS_PER_BITMAPWORD +
> 1)
> /* number of active words for a lossy chunk: */
> #define WORDS_PER_CHUNK  ((PAGES_PER_CHUNK - 1) / BITS_PER_BITMAPWORD + 1)

> Though the use of WORDS_PER_PAGE in the scan of a lossy chunk is logically
> correct since these macros implicitly satisfy that WORDS_PER_CHUNK <
> WORDS_PER_PAGE, I think WORDS_PER_CHUNK should be used in the scan of a
> lossy chunk for code readability and maintenance.  So, I submitted a patch
> working in such a way in an earlier email.

This is a bug fix, not a performance improvement (any improvement would
be welcome, but that's not the point).  There's absolutely nothing
guaranteeing that WORDS_PER_CHUNK is less than WORDS_PER_PAGE, and if
it were the other way around, the code would be outright broken.  It's
pure luck that it was merely inefficient.

Committed, thanks for finding it!
        regards, tom lane