| Lists: | pgsql-hackers |
|---|
| From: | "Etsuro Fujita" <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> |
|---|---|
| To: | "'pgsql-hackers'" <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Improve code in tidbitmap.c |
| Date: | 2013-11-08 09:23:37 |
| Message-ID: | 004d01cedc64$34722820$9d567860$@etsuro@lab.ntt.co.jp |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Lists: | pgsql-hackers |
Hi,
ISTM the code in tidbitmap.c should be improved. Patch attached. I think
this patch increases the efficiency a bit.
Thanks,
Best regards,
Etsuro Fujita
| Attachment | Content-Type | Size |
|---|---|---|
| tidbitmap-20131108.patch.txt | text/plain | 1.0 KB |
| From: | "Etsuro Fujita" <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> |
|---|---|
| To: | "'Etsuro Fujita'" <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp>, "'pgsql-hackers'" <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Improve code in tidbitmap.c |
| Date: | 2013-11-14 11:50:45 |
| Message-ID: | 00a201cee12f$c0d04430$4270cc90$@etsuro@lab.ntt.co.jp |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Lists: | pgsql-hackers |
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
| From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
|---|---|
| To: | "Etsuro Fujita" <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)jp> |
| Cc: | "'pgsql-hackers'" <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Improve code in tidbitmap.c |
| Date: | 2013-11-15 23:38:00 |
| Message-ID: | 30708.1384558680@sss.pgh.pa.us |
| Views: | Raw Message | Whole Thread | Download mbox | Resend email |
| Lists: | pgsql-hackers |
"Etsuro Fujita" <fujita(dot)etsuro(at)lab(dot)ntt(dot)co(dot)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