public inbox for [email protected]  
help / color / mirror / Atom feed
[PATCH v7 1/1] pg_lfind32(): add "overlap" code for remaining elements
2+ messages / 2 participants
[nested] [flat]

* [PATCH v7 1/1] pg_lfind32(): add "overlap" code for remaining elements
@ 2024-03-20 19:20  Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Nathan Bossart @ 2024-03-20 19:20 UTC (permalink / raw)

---
 src/include/port/pg_lfind.h | 105 ++++++++++++++++++++++++------------
 1 file changed, 72 insertions(+), 33 deletions(-)

diff --git a/src/include/port/pg_lfind.h b/src/include/port/pg_lfind.h
index b8dfa66eef..5830cc7cb3 100644
--- a/src/include/port/pg_lfind.h
+++ b/src/include/port/pg_lfind.h
@@ -80,6 +80,51 @@ pg_lfind8_le(uint8 key, uint8 *base, uint32 nelem)
 	return false;
 }
 
+#ifndef USE_NO_SIMD
+/*
+ * pg_lfind32_helper
+ *
+ * Searches one 4-register-block of integers.  The caller is responsible for
+ * ensuring that there are at least 4-registers-worth of integers remaining.
+ */
+static inline bool
+pg_lfind32_helper(const Vector32 keys, uint32 *base)
+{
+	const uint32 nelem_per_vector = sizeof(Vector32) / sizeof(uint32);
+	Vector32	vals1,
+				vals2,
+				vals3,
+				vals4,
+				result1,
+				result2,
+				result3,
+				result4,
+				tmp1,
+				tmp2,
+				result;
+
+	/* load the next block into 4 registers */
+	vector32_load(&vals1, base);
+	vector32_load(&vals2, &base[nelem_per_vector]);
+	vector32_load(&vals3, &base[nelem_per_vector * 2]);
+	vector32_load(&vals4, &base[nelem_per_vector * 3]);
+
+	/* compare each value to the key */
+	result1 = vector32_eq(keys, vals1);
+	result2 = vector32_eq(keys, vals2);
+	result3 = vector32_eq(keys, vals3);
+	result4 = vector32_eq(keys, vals4);
+
+	/* combine the results into a single variable */
+	tmp1 = vector32_or(result1, result2);
+	tmp2 = vector32_or(result3, result4);
+	result = vector32_or(tmp1, tmp2);
+
+	/* return whether there was a match */
+	return vector32_is_highbit_set(result);
+}
+#endif							/* ! USE_NO_SIMD */
+
 /*
  * pg_lfind32
  *
@@ -119,46 +164,40 @@ pg_lfind32(uint32 key, uint32 *base, uint32 nelem)
 	}
 #endif
 
-	for (i = 0; i < tail_idx; i += nelem_per_iteration)
+	/*
+	 * If there aren't enough elements for the SIMD code, jump to the standard
+	 * one-by-one linear search code.
+	 */
+	if (nelem <= nelem_per_iteration)
+		goto one_by_one;
+
+	/*
+	 * Process as many elements as possible with a block of 4 registers.
+	 */
+	do
 	{
-		Vector32	vals1,
-					vals2,
-					vals3,
-					vals4,
-					result1,
-					result2,
-					result3,
-					result4,
-					tmp1,
-					tmp2,
-					result;
-
-		/* load the next block into 4 registers */
-		vector32_load(&vals1, &base[i]);
-		vector32_load(&vals2, &base[i + nelem_per_vector]);
-		vector32_load(&vals3, &base[i + nelem_per_vector * 2]);
-		vector32_load(&vals4, &base[i + nelem_per_vector * 3]);
-
-		/* compare each value to the key */
-		result1 = vector32_eq(keys, vals1);
-		result2 = vector32_eq(keys, vals2);
-		result3 = vector32_eq(keys, vals3);
-		result4 = vector32_eq(keys, vals4);
-
-		/* combine the results into a single variable */
-		tmp1 = vector32_or(result1, result2);
-		tmp2 = vector32_or(result3, result4);
-		result = vector32_or(tmp1, tmp2);
-
-		/* see if there was a match */
-		if (vector32_is_highbit_set(result))
+		if (pg_lfind32_helper(keys, &base[i]))
 		{
 			Assert(assert_result == true);
 			return true;
 		}
-	}
+
+		i += nelem_per_iteration;
+
+	} while (i < tail_idx);
+
+	/*
+	 * Process the last 'nelem_per_iteration' elements in the array with a
+	 * 4-register block.  This will cause us to check some of the elements
+	 * more than once, but that won't affect correctness, and testing has
+	 * demonstrated that this helps more cases than it harms.
+	 */
+	Assert(assert_result == pg_lfind32_helper(keys, &base[nelem - nelem_per_iteration]));
+	return pg_lfind32_helper(keys, &base[nelem - nelem_per_iteration]);
+
 #endif							/* ! USE_NO_SIMD */
 
+one_by_one:
 	/* Process the remaining elements one at a time. */
 	for (; i < nelem; i++)
 	{
-- 
2.25.1


--sm4nu43k4a2Rpi4c--





^ permalink  raw  reply  [nested|flat] 2+ messages in thread

* Re: Initial COPY of Logical Replication is too slow
@ 2026-02-25 21:29  Matheus Alcantara <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Matheus Alcantara @ 2026-02-25 21:29 UTC (permalink / raw)
  To: Masahiko Sawada <[email protected]>; Marcos Pegoraro <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>

Hi

On Wed Feb 25, 2026 at 4:03 PM -03, Masahiko Sawada wrote:
>> After more investigation of slowness, it seems that the
>> list_concat_unique_oid() called below is quite slow when the database
>> has a lot of tables to publish:
>>
>>     relids = GetPublicationRelations(pub_elem->oid,
>>                                      pub_elem->pubviaroot ?
>>                                      PUBLICATION_PART_ROOT :
>>                                      PUBLICATION_PART_LEAF);
>>     schemarelids = GetAllSchemaPublicationRelations(pub_elem->oid,
>>                                                      pub_elem->pubviaroot ?
>>                                                      PUBLICATION_PART_ROOT :
>>                                                      PUBLICATION_PART_LEAF);
>>     pub_elem_tables = list_concat_unique_oid(relids, schemarelids);
>>
>> This is simply because it's O(n^2), where n is the number of oids in
>> schemarelids in the test case. A simple change would be to do sort &
>> dedup instead. With the attached experimental patch, the
>> pg_get_publication_tables() execution time gets halved in my
>> environment (796ms -> 430ms with 50k tables). If the number of tables
>> is not large, this method might be slower than today but it's not a
>> huge regression.
>>
>> In the initial tablesync cases, it could be optimized further in a way
>> that we introduce a new SQL function that gets the column list and
>> expr of the specific table. This way, we can filter the result by
>> relid at an early stage instead of getting all information and
>> filtering by relid as the tablesync worker does today, avoiding
>> overheads of gathering system catalog scan results.
>
> I've drafted this idea and I find it looks like a better approach. The
> patch introduces the pg_get_publication_table_info() SQL function that
> returns the column list and row filter expression like
> pg_get_publication_tables() returns but it checks only the specific
> table unlike pg_get_publication_tables(). On my env, the tablesync
> worker's query in question becomes 0.6ms from 288 ms with 50k tables
> in one publication. Feedback is very welcome.
>

Thanks for patch. I did a review and here are my comments:

+	values[0] = ObjectIdGetDatum(pub->oid);
+	values[1] = ObjectIdGetDatum(relid);
+
+	values[0] = ObjectIdGetDatum(pub->oid);
+	values[1] = ObjectIdGetDatum(relid);

Duplicated assignments?

--------------

+		  /* ALL TALBES publication */

Typo on TALBES

--------------

+ * Common routine for pg_get_publication_tables() and
+ * pg_get_publication_table_info() to construct the result tuple.
+ */
+static HeapTuple
+construct_published_rel_tuple(published_rel *table_info, TupleDesc tuple_desc)

construct_published_rel_tuple is only being used on
pg_get_publication_table_info(). Perhaps it can also be used on "if
(funcctx->call_cntr < list_length(table_infos))" block on
pg_get_publication_tables()?

--------------

Is new regression tests needed or the current ones already cover the new
function engouth? The code of pg_get_publication_table_info seems well
coveraged.

--------------

It seems that pgindent is missing on src/backend/catalog/pg_publication.c

--
Matheus Alcantara
EDB: https://www.enterprisedb.com






^ permalink  raw  reply  [nested|flat] 2+ messages in thread


end of thread, other threads:[~2026-02-25 21:29 UTC | newest]

Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-03-20 19:20 [PATCH v7 1/1] pg_lfind32(): add "overlap" code for remaining elements Nathan Bossart <[email protected]>
2026-02-25 21:29 Re: Initial COPY of Logical Replication is too slow Matheus Alcantara <[email protected]>

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox