Re: [HACKERS] Improve code in tidbitmap.c

2013-11-15 Thread Tom Lane 
Etsuro Fujita fujita.ets...@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


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

Re: [HACKERS] Improve code in tidbitmap.c

2013-11-14 Thread Etsuro Fujita 
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



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

Re: [HACKERS] Improve code in tidbitmap.c

2013-11-14 Thread David Rowley 
On Fri, Nov 15, 2013 at 12:50 AM, Etsuro Fujita fujita.ets...@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

[HACKERS] Improve code in tidbitmap.c

2013-11-08 Thread Etsuro Fujita 
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

diff --git a/src/backend/nodes/tidbitmap.c b/src/backend/nodes/tidbitmap.c
index 3ef0112..43628ac 100644
--- a/src/backend/nodes/tidbitmap.c
+++ b/src/backend/nodes/tidbitmap.c
@@ -361,7 +361,7 @@ tbm_union_page(TIDBitmap *a, const PagetableEntry *bpage)
if (bpage-ischunk)
{
/* Scan b's chunk, mark each indicated page lossy in a */
-   for (wordnum = 0; wordnum  WORDS_PER_PAGE; wordnum++)
+   for (wordnum = 0; wordnum  WORDS_PER_CHUNK; wordnum++)
{
bitmapword  w = bpage-words[wordnum];

@@ -473,7 +473,7 @@ tbm_intersect_page(TIDBitmap *a, PagetableEntry *apage, 
const TIDBitmap *b)
/* Scan each bit in chunk, try to clear */
boolcandelete = true;

-   for (wordnum = 0; wordnum  WORDS_PER_PAGE; wordnum++)
+   for (wordnum = 0; wordnum  WORDS_PER_CHUNK; wordnum++)
{
bitmapword  w = apage-words[wordnum];

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