public inbox for [email protected]  
help / color / mirror / Atom feed
Re: Injection points: some tools to wait and wake
2+ messages / 2 participants
[nested] [flat]

* Re: Injection points: some tools to wait and wake
@ 2024-02-27 13:39  Bertrand Drouvot <[email protected]>
  0 siblings, 0 replies; 2+ messages in thread

From: Bertrand Drouvot @ 2024-02-27 13:39 UTC (permalink / raw)
  To: Andrey M. Borodin <[email protected]>; +Cc: Michael Paquier <[email protected]>; Postgres hackers <[email protected]>; Ashutosh Bapat <[email protected]>

Hi,

On Tue, Feb 27, 2024 at 04:49:03PM +0500, Andrey M. Borodin wrote:
> Hi,
> 
> > On 27 Feb 2024, at 16:08, Bertrand Drouvot <[email protected]> wrote:
> > 
> > On Tue, Feb 27, 2024 at 11:00:10AM +0500, Andrey M. Borodin wrote:
> >> 
> >> But, AFAICS, the purpose is the same: wait until event happened.
> > 
> > I think it's easier to understand the tests (I mean what the purpose of the
> > injection points are) if we don't use an helper function. While the helper 
> > function would make the test easier to read / cleaner, I think it may make them
> > more difficult to understand as 'await_injection_point' would probably be too
> > generic.
> 
> For the record: I’m fine if there is no such function.
> There will be at least one implementation of this function in every single test with waiting injection points.
> That’s the case where we might want something generic.
> What the specific there might be? The test can check that 
>  - conditions are logged
>  - injection point reached in specific backend (e.g. checkpointer)
> etc
> 
> I doubt that this specific checks worth cleanness of the test. And sacrificing test readability in favour of teaching reader what injection points are sounds strange.

I'm giving a second thought and it does not have to be exclusive, for example in
this specific test we could:

1) check that the injection point is reached with the helper (querying pg_stat_activity
looks good to me) 
And
2) check in the log 

So even if two checks might sound "too much" they both make sense to give 1) better
readability and 2) better understanding of the test.

So, I'm ok with the new helper too.

Regards,

-- 
Bertrand Drouvot
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com






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

* [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


end of thread, other threads:[~2024-03-20 19:20 UTC | newest]

Thread overview: 2+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2024-02-27 13:39 Re: Injection points: some tools to wait and wake Bertrand Drouvot <[email protected]>
2024-03-20 19:20 [PATCH v7 1/1] pg_lfind32(): add "overlap" code for remaining elements Nathan Bossart <[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