repeat() function, CHECK_FOR_INTERRUPTS(), and unlikely()

[Joe Conway]

I was doing some memory testing under fractional CPU allocations and it became
painfully obvious that the repeat() function needs CHECK_FOR_INTERRUPTS().

I exchanged a few emails offlist with Tom about it, and (at the risk of putting
words in his mouth) he agreed and felt it was a candidate for backpatching.

Very small patch attached. Quick and dirty performance test:

explain analyze SELECT repeat('A', 300000000);
explain analyze SELECT repeat('A', 300000000);
explain analyze SELECT repeat('A', 300000000);

With an -O2 optimized build:

Without CHECK_FOR_INTERRUPTS

 Planning Time: 1077.238 ms
 Execution Time: 0.016 ms

 Planning Time: 1080.381 ms
 Execution Time: 0.013 ms

 Planning Time: 1072.049 ms
 Execution Time: 0.013 ms

With CHECK_FOR_INTERRUPTS

 Planning Time: 1078.703 ms
 Execution Time: 0.013 ms

 Planning Time: 1077.495 ms
 Execution Time: 0.013 ms

 Planning Time: 1076.793 ms
 Execution Time: 0.013 ms


While discussing the above, Tom also wondered whether we should add unlikely()
to the CHECK_FOR_INTERRUPTS() macro.

Small patch for that also attached. I was not sure about the WIN32 stanza on
that (to do it or not; if so, what about the UNBLOCKED_SIGNAL_QUEUE() test).

I tested as above with unlikely() and did not see any discernible difference,
but the added check might improve other code paths.

Comments or objections?

Thanks,

Joe
-- 
Crunchy Data - http://crunchydata.com
PostgreSQL Support for Secure Enterprises
Consulting, Training, & Open Source Development

diff --git a/src/backend/utils/adt/oracle_compat.c b/src/backend/utils/adt/oracle_compat.c
index 0fdfee5..ecd8268 100644
*** a/src/backend/utils/adt/oracle_compat.c
--- b/src/backend/utils/adt/oracle_compat.c
***************
*** 19,25 ****
  #include "utils/builtins.h"
  #include "utils/formatting.h"
  #include "mb/pg_wchar.h"
! 
  
  static text *dotrim(const char *string, int stringlen,
  					const char *set, int setlen,
--- 19,25 ----
  #include "utils/builtins.h"
  #include "utils/formatting.h"
  #include "mb/pg_wchar.h"
! #include "miscadmin.h"
  
  static text *dotrim(const char *string, int stringlen,
  					const char *set, int setlen,
*************** repeat(PG_FUNCTION_ARGS)
*** 1062,1067 ****
--- 1062,1068 ----
  	{
  		memcpy(cp, sp, slen);
  		cp += slen;
+ 		CHECK_FOR_INTERRUPTS();
  	}
  
  	PG_RETURN_TEXT_P(result);

diff --git a/src/include/miscadmin.h b/src/include/miscadmin.h
index 61a24c2..0467a8e 100644
*** a/src/include/miscadmin.h
--- b/src/include/miscadmin.h
*************** extern void ProcessInterrupts(void);
*** 98,104 ****
  
  #define CHECK_FOR_INTERRUPTS() \
  do { \
! 	if (InterruptPending) \
  		ProcessInterrupts(); \
  } while(0)
  #else							/* WIN32 */
--- 98,104 ----
  
  #define CHECK_FOR_INTERRUPTS() \
  do { \
! 	if (unlikely(InterruptPending)) \
  		ProcessInterrupts(); \
  } while(0)
  #else							/* WIN32 */
*************** do { \
*** 107,113 ****
  do { \
  	if (UNBLOCKED_SIGNAL_QUEUE()) \
  		pgwin32_dispatch_queued_signals(); \
! 	if (InterruptPending) \
  		ProcessInterrupts(); \
  } while(0)
  #endif							/* WIN32 */
--- 107,113 ----
  do { \
  	if (UNBLOCKED_SIGNAL_QUEUE()) \
  		pgwin32_dispatch_queued_signals(); \
! 	if (unlikely(InterruptPending)) \
  		ProcessInterrupts(); \
  } while(0)
  #endif							/* WIN32 */

signature.asc
Description: OpenPGP digital signature