Make Fault Injection sites cheaper, when no faults have been activated.

Fault injection is expected to be *very* cheap, we even enable it on
production builds. That's why I was very surprised when I saw 'perf' report
that FaultInjector_InjectFaultIfSet() was consuming about 10% of CPU time
in a performance test I was running on my laptop. I tracked it to the
FaultInjector_InjectFaultIfSet() call in standard_ExecutorRun(). It gets
called for every tuple between 10000 and 1000000, on every segment.

Why is FaultInjector_InjectFaultIfSet() so expensive? It has a quick exit
in it, when no faults have been activated, but before reaching the quick
exit it calls strlen() on the arguments. That's not cheap. And the function
call isn't completely negligible on hot code paths, either.

To fix, turn FaultInjector_InjectFaultIfSet() into a macro that's only
few instructions long in the fast path. That should be cheap enough.
Reviewed-by: NAshwin Agrawal <aashwin@vmware.com>
Reviewed-by: NJesse Zhang <jzhang@pivotal.io>
Reviewed-by: NAsim R P <pasim@vmware.com>

src/backend/access/transam/test/xlog_test.c

@@ -8,13 +8,6 @@
 static void
 KeepLogSeg_wrapper(XLogRecPtr recptr, XLogSegNo *logSegNo)
 {
-#ifdef FAULT_INJECTOR
-	expect_value(FaultInjector_InjectFaultIfSet, faultName, "keep_log_seg");
-	expect_value(FaultInjector_InjectFaultIfSet, ddlStatement, DDLNotSpecified);
-	expect_value(FaultInjector_InjectFaultIfSet, databaseName, "");
-	expect_value(FaultInjector_InjectFaultIfSet, tableName, "");
-	will_be_called(FaultInjector_InjectFaultIfSet);
-#endif
 	KeepLogSeg(recptr, logSegNo);
 }
 

src/backend/utils/misc/faultinjector.c

@@ -61,6 +61,14 @@ bool am_faulthandler = false;
 
 static	FaultInjectorShmem_s *faultInjectorShmem = NULL;
 
+/*
+ * faultInjectorSlots_ptr points to this until shmem is initialized. Just to
+ * keep any FaultInjector_InjectFaultIfSet calls from crashing.
+ */
+static int dummyslots = 0;
+
+int *faultInjectorSlots_ptr = &dummyslots;
+
 static void FiLockAcquire(void);
 static void FiLockRelease(void);
 
@@ -200,6 +208,8 @@ FaultInjector_ShmemInit(void)
 				 (errmsg("not enough shared memory for fault injector"))));
 	}	
 
+	faultInjectorSlots_ptr = &faultInjectorShmem->faultInjectorSlots;
+	
 	if (! foundPtr) 
 	{
 		MemSet(faultInjectorShmem, 0, sizeof(FaultInjectorShmem_s));
@@ -230,7 +240,7 @@ FaultInjector_ShmemInit(void)
 }
 
 FaultInjectorType_e
-FaultInjector_InjectFaultIfSet(
+FaultInjector_InjectFaultIfSet_out_of_line(
 							   const char*				 faultName,
 							   DDLStatement_e			 ddlStatement,
 							   const char*				 databaseName,

src/backend/utils/mmgr/test/runaway_cleaner_test.c

@@ -149,14 +149,6 @@ test__RunawayCleaner_StartCleanup__StartsPrimaryCleanupIfPossible(void **state)
 	gp_command_count = 1;
 	will_return(IsTransactionState, true);
 
-#ifdef FAULT_INJECTOR
-	expect_value(FaultInjector_InjectFaultIfSet, faultName, "runaway_cleanup");
-	expect_value(FaultInjector_InjectFaultIfSet, ddlStatement, DDLNotSpecified);
-	expect_value(FaultInjector_InjectFaultIfSet, databaseName, "");
-	expect_value(FaultInjector_InjectFaultIfSet, tableName, "");
-	will_be_called(FaultInjector_InjectFaultIfSet);
-#endif
-
 	EXPECT_EREPORT(ERROR);
 
 	PG_TRY();
@@ -216,14 +208,6 @@ test__RunawayCleaner_StartCleanup__StartsSecondaryCleanupIfPossible(void **state
 	will_return(superuser, false);
 	will_return(IsTransactionState, true);
 
-#ifdef FAULT_INJECTOR
-	expect_value(FaultInjector_InjectFaultIfSet, faultName, "runaway_cleanup");
-	expect_value(FaultInjector_InjectFaultIfSet, ddlStatement, DDLNotSpecified);
-	expect_value(FaultInjector_InjectFaultIfSet, databaseName, "");
-	expect_value(FaultInjector_InjectFaultIfSet, tableName, "");
-	will_be_called(FaultInjector_InjectFaultIfSet);
-#endif
-
 	EXPECT_EREPORT(ERROR);
 
 	PG_TRY();

src/include/utils/faultinjector.h

@@ -85,12 +85,25 @@ extern Size FaultInjector_ShmemSize(void);
 
 extern void FaultInjector_ShmemInit(void);
 
-extern FaultInjectorType_e FaultInjector_InjectFaultIfSet(
+/*
+ * To check if a fault has been injected, use FaultInjector_InjectFaultIfSet().
+ * It is designed to fall through as quickly as possible, when no faults are
+ * activated.
+ */
+extern FaultInjectorType_e FaultInjector_InjectFaultIfSet_out_of_line(
 							   const char*				 faultName,
 							   DDLStatement_e			 ddlStatement,
 							   const char*				 databaseName,
 							   const char*				 tableName);
 
+#define FaultInjector_InjectFaultIfSet(faultName, ddlStatement, databaseName, tableName) \
+	(((*faultInjectorSlots_ptr) > 0) ? \
+	 FaultInjector_InjectFaultIfSet_out_of_line(faultName, ddlStatement, databaseName, tableName) : \
+	 FaultInjectorTypeNotSpecified)
+
+extern int *faultInjectorSlots_ptr;
+
+
 extern char *InjectFault(
 	char *faultName, char *type, char *ddlStatement, char *databaseName,
 	char *tableName, int startOccurrence, int endOccurrence, int extraArg);