On 24.06.2011 11:40, Heikki Linnakangas wrote:
On 21.06.2011 13:08, Alexander Korotkov wrote:
I believe it's due to relatively expensive penalty
method in that opclass.

Hmm, I wonder if it could be optimized. I did a quick test, creating a
gist_trgm_ops index on a list of English words from
/usr/share/dict/words. oprofile shows that with the patch, 60% of the
CPU time is spent in the makesign() function.

I couldn't resist looking into this, and came up with the attached patch. I tested this with:

CREATE TABLE words (word text);
COPY words FROM '/usr/share/dict/words';
CREATE INDEX i_words ON words USING gist (word gist_trgm_ops);

And then ran "REINDEX INDEX i_words" a few times with and without the patch. Without the patch, reindex takes about 4.7 seconds. With the patch, 3.7 seconds. That's a worthwhile gain on its own, but becomes even more important with Alexander's fast GiST build patch, which calls the penalty function more.

I used the attached showsign-debug.patch to verify that the patched makesign function produces the same results as the existing code. I haven't tested the big-endian code, however.

--
  Heikki Linnakangas
  EnterpriseDB   http://www.enterprisedb.com
diff --git a/contrib/pg_trgm/trgm_gist.c b/contrib/pg_trgm/trgm_gist.c
index b328a09..1f3d3e3 100644
--- a/contrib/pg_trgm/trgm_gist.c
+++ b/contrib/pg_trgm/trgm_gist.c
@@ -84,17 +84,88 @@ gtrgm_out(PG_FUNCTION_ARGS)
 static void
 makesign(BITVECP sign, TRGM *a)
 {
-	int4		k,
-				len = ARRNELEM(a);
+	int4		len = ARRNELEM(a);
 	trgm	   *ptr = GETARR(a);
-	int4		tmp = 0;
+	char	   *p;
+	char	   *endptr;
+	uint32		w1,
+				w2,
+				w3;
+	uint32		trg1,
+				trg2,
+				trg3,
+				trg4;
+	uint32	   *p32;
 
 	MemSet((void *) sign, 0, sizeof(BITVEC));
 	SETBIT(sign, SIGLENBIT);	/* set last unused bit */
-	for (k = 0; k < len; k++)
+
+	if (len == 0)
+		return;
+
+	endptr = (char *) (ptr + len);
+	/*------------------------------------------------------------------------
+	 * We have to extract each trigram into a uint32, and calculate the HASH.
+	 * This would be a lot easier if each trigram was aligned at 4-byte
+	 * boundary, but they're not. The simple way would be to copy each
+	 * trigram byte-per-byte, but that is quite slow, and this function is a
+	 * hotspot in penalty calculation.
+	 *
+	 * The first trigram in the array doesn't begin at a 4-byte boundary, the
+	 * flags byte comes first, but the next one does. So we fetch the first
+	 * trigram as a special case, and after that the following four trigrams
+	 * fall onto 4-byte words like this:
+	 *
+	 *  w1   w2   w3
+	 * AAAB BBCC CDDD
+	 *
+	 * As long as there's at least four trigrams left to process, we fetch
+	 * the next three words and extract the trigrams from them with bit
+	 * operations.
+	 *------------------------------------------------------------------------
+	 */
+	p32 = (uint32 *) (((char *) ptr) - 1);
+
+	/* Fetch and extract the initial word */
+	w1 = *(p32++);
+#ifdef WORDS_BIGENDIAN
+	trg1 = w1 << 8;
+#else
+	trg1 = w1 >> 8;
+#endif
+	HASH(sign, trg1);
+
+	while((char *) p32 < endptr - 12)
 	{
-		CPTRGM(((char *) &tmp), ptr + k);
-		HASH(sign, tmp);
+		w1 = *(p32++);
+		w2 = *(p32++);
+		w3 = *(p32++);
+
+#ifdef WORDS_BIGENDIAN
+		trg1 = w1 & 0xFFFFFF00;
+		trg2 = (w1 << 24) | ((w2 & 0xFFFF0000) >> 8);
+		trg3 = ((w2 & 0x0000FFFF) << 16) | ((w3 & 0xFF000000) >> 16);
+		trg4 = w3 << 8;
+#else
+		trg1 = w1 & 0x00FFFFFF;
+		trg2 = (w1 >> 24) | ((w2 & 0x0000FFFF) << 8);
+		trg3 = ((w2 & 0xFFFF0000) >> 16) | ((w3 & 0x000000FF) << 16);
+		trg4 = w3 >> 8;
+#endif
+
+		HASH(sign, trg1);
+		HASH(sign, trg2);
+		HASH(sign, trg3);
+		HASH(sign, trg4);
+	}
+
+	/* Handle remaining 1-3 trigrams the slow way */
+	p = (char *) p32;
+	while (p < endptr)
+	{
+		CPTRGM(((char *) &trg1), p);
+		HASH(sign, trg1);
+		p += 3;
 	}
 }
 
diff --git a/contrib/pg_trgm/trgm_gist.c b/contrib/pg_trgm/trgm_gist.c
index b328a09..b5be800 100644
--- a/contrib/pg_trgm/trgm_gist.c
+++ b/contrib/pg_trgm/trgm_gist.c
@@ -44,6 +44,9 @@ Datum		gtrgm_penalty(PG_FUNCTION_ARGS);
 PG_FUNCTION_INFO_V1(gtrgm_picksplit);
 Datum		gtrgm_picksplit(PG_FUNCTION_ARGS);
 
+PG_FUNCTION_INFO_V1(gtrgm_showsign);
+Datum		gtrgm_showsign(PG_FUNCTION_ARGS);
+
 #define GETENTRY(vec,pos) ((TRGM *) DatumGetPointer((vec)->vector[(pos)].key))
 
 /* Number of one-bits in an unsigned byte */
@@ -98,6 +101,32 @@ makesign(BITVECP sign, TRGM *a)
 	}
 }
 
+static char *
+printsign(BITVECP sign)
+{
+	static char c[200];
+	char *p = c;
+	int i;
+	for(i=0; i < SIGLEN;i++)
+	{
+		p += snprintf(p, 3, "%02x", (unsigned int) (((unsigned char *) sign)[i]));
+	}
+	return c;
+}
+
+Datum
+gtrgm_showsign(PG_FUNCTION_ARGS)
+{
+	text	   *in = PG_GETARG_TEXT_P(0);
+	BITVEC		sign;
+	TRGM	   *trg;
+
+	trg = generate_trgm(VARDATA(in), VARSIZE(in) - VARHDRSZ);
+	makesign(sign, trg);
+
+	PG_RETURN_TEXT_P(cstring_to_text(printsign(sign)));
+}
+
 Datum
 gtrgm_compress(PG_FUNCTION_ARGS)
 {
-- 
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