While profiling Alexander's patches, I noticed that a lot of time is spent in ginCompareItemPointers:

 47,59%  postmaster  postgres gingetbitmap
 46,73%  postmaster  postgres ginCompareItemPointers
  2,31%  postmaster  postgres FunctionCall8Coll
  1,54%  postmaster  postgres callConsistentFn
  0,79%  postmaster  postgres ginarrayconsistent
  0,63%  postmaster  postgres MemoryContextReset

I think much of the time spent in gingetbitmap has to do with calling ginCompareItemPointers, too.

The query in question is this:

postgres=# explain analyze select * from intarrtbl where ii @> '{1,2,3,4,5,6,7,8,9,10}'::int[]; QUERY PLAN

--------------------------------------------------------------------------------
-------------------------------------------------
Bitmap Heap Scan on intarrtbl (cost=3036.00..3040.01 rows=1 width=61) (actual
time=405.461..405.461 rows=0 loops=1)
   Recheck Cond: (ii @> '{1,2,3,4,5,6,7,8,9,10}'::integer[])
-> Bitmap Index Scan on gin_intarr_master (cost=0.00..3036.00 rows=1 width=
0) (actual time=405.458..405.458 rows=0 loops=1)
         Index Cond: (ii @> '{1,2,3,4,5,6,7,8,9,10}'::integer[])
 Total runtime: 405.482 ms
(5 rows)

Rewriting and inlining ginCompareItemPointers as attached helps a lot. After the patch:

postgres=# explain analyze select * from intarrtbl where ii @> '{1,2,3,4,5,6,7,8,9,10}'::int[]; QUERY PLAN

--------------------------------------------------------------------------------
-------------------------------------------------
Bitmap Heap Scan on intarrtbl (cost=3036.00..3040.01 rows=1 width=61) (actual
time=149.694..149.694 rows=0 loops=1)
   Recheck Cond: (ii @> '{1,2,3,4,5,6,7,8,9,10}'::integer[])
-> Bitmap Index Scan on gin_intarr_master (cost=0.00..3036.00 rows=1 width=
0) (actual time=149.691..149.691 rows=0 loops=1)
         Index Cond: (ii @> '{1,2,3,4,5,6,7,8,9,10}'::integer[])
 Total runtime: 149.716 ms
(5 rows)

This patch seems pretty uncontroversial, so I'll go commit it. Once we get this elephant out of the room, performance testing the rest of the gin patches become much more meaningful. We might want to optimize the ItemPointerCompare function, used elsewhere in the backend, too, but I'll leave that for a separate patch.

- Heikki
diff --git a/src/backend/access/gin/gindatapage.c b/src/backend/access/gin/gindatapage.c
index 13ab448..f017de0 100644
--- a/src/backend/access/gin/gindatapage.c
+++ b/src/backend/access/gin/gindatapage.c
@@ -17,25 +17,6 @@
 #include "access/gin_private.h"
 #include "utils/rel.h"
 
-int
-ginCompareItemPointers(ItemPointer a, ItemPointer b)
-{
-	BlockNumber ba = GinItemPointerGetBlockNumber(a);
-	BlockNumber bb = GinItemPointerGetBlockNumber(b);
-
-	if (ba == bb)
-	{
-		OffsetNumber oa = GinItemPointerGetOffsetNumber(a);
-		OffsetNumber ob = GinItemPointerGetOffsetNumber(b);
-
-		if (oa == ob)
-			return 0;
-		return (oa > ob) ? 1 : -1;
-	}
-
-	return (ba > bb) ? 1 : -1;
-}
-
 /*
  * Merge two ordered arrays of itempointers, eliminating any duplicates.
  * Returns the number of items in the result.
diff --git a/src/include/access/gin_private.h b/src/include/access/gin_private.h
index 1868b77..c603521 100644
--- a/src/include/access/gin_private.h
+++ b/src/include/access/gin_private.h
@@ -530,7 +530,6 @@ extern void ginEntryFillRoot(GinBtree btree, Buffer root, Buffer lbuf, Buffer rb
 extern IndexTuple ginPageGetLinkItup(Buffer buf);
 
 /* gindatapage.c */
-extern int	ginCompareItemPointers(ItemPointer a, ItemPointer b);
 extern uint32 ginMergeItemPointers(ItemPointerData *dst,
 					 ItemPointerData *a, uint32 na,
 					 ItemPointerData *b, uint32 nb);
@@ -724,4 +723,28 @@ extern void ginHeapTupleFastCollect(GinState *ginstate,
 extern void ginInsertCleanup(GinState *ginstate,
 				 bool vac_delay, IndexBulkDeleteResult *stats);
 
+/*
+ * Merging the results of several gin scans compares item pointers a lot,
+ * so we want this to be inlined. But if the compiler doesn't support that,
+ * fall back on the non-inline version from itemptr.c. See STATIC_IF_INLINE in
+ * c.h.
+ */
+#ifdef PG_USE_INLINE
+static inline int
+ginCompareItemPointers(ItemPointer a, ItemPointer b)
+{
+	uint64 ia = (uint64) a->ip_blkid.bi_hi << 32 | (uint64) a->ip_blkid.bi_lo << 16 | a->ip_posid;
+	uint64 ib = (uint64) b->ip_blkid.bi_hi << 32 | (uint64) b->ip_blkid.bi_lo << 16 | b->ip_posid;
+
+	if (ia == ib)
+		return 0;
+	else if (ia > ib)
+		return 1;
+	else
+		return -1;
+}
+#else
+#define ginCompareItemPointers(a, b) ItemPointerCompare(a, b)
+#endif   /* PG_USE_INLINE */
+
 #endif   /* GIN_PRIVATE_H */
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to