Get rid of PgStat_BackendFunctionEntry

Started by Drouvot, Bertrandalmost 3 years ago4 messages
#1Drouvot, Bertrand
bertranddrouvot.pg@gmail.com
1 attachment(s)

Hi hackers,

Please find attached a patch proposal to $SUBJECT: it would allow to simplify
even more the work done to generate pg_stat_get_xact*() functions with Macros.

This is a follow up for [1]/messages/by-id/20230210214619.bdpbd5wvxcpx27rw@awork3.anarazel.de where it has been suggested
to get rid of PgStat_BackendFunctionEntry in a separate patch.

Looking forward to your feedback,

Regards

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

[1]: /messages/by-id/20230210214619.bdpbd5wvxcpx27rw@awork3.anarazel.de

Attachments:

v1-0001-get_rid_PgStat_BackendFunctionEntry.patchtext/plain; charset=UTF-8; name=v1-0001-get_rid_PgStat_BackendFunctionEntry.patchDownload
diff --git a/src/backend/utils/activity/pgstat.c b/src/backend/utils/activity/pgstat.c
index 60fc4e761f..b125802b21 100644
--- a/src/backend/utils/activity/pgstat.c
+++ b/src/backend/utils/activity/pgstat.c
@@ -292,7 +292,7 @@ static const PgStat_KindInfo pgstat_kind_infos[PGSTAT_NUM_KINDS] = {
 		.shared_size = sizeof(PgStatShared_Function),
 		.shared_data_off = offsetof(PgStatShared_Function, stats),
 		.shared_data_len = sizeof(((PgStatShared_Function *) 0)->stats),
-		.pending_size = sizeof(PgStat_BackendFunctionEntry),
+		.pending_size = sizeof(PgStat_FunctionCounts),
 
 		.flush_pending_cb = pgstat_function_flush_cb,
 	},
diff --git a/src/backend/utils/activity/pgstat_function.c b/src/backend/utils/activity/pgstat_function.c
index 6139a27fee..0fdcefb783 100644
--- a/src/backend/utils/activity/pgstat_function.c
+++ b/src/backend/utils/activity/pgstat_function.c
@@ -73,7 +73,7 @@ pgstat_init_function_usage(FunctionCallInfo fcinfo,
 						   PgStat_FunctionCallUsage *fcu)
 {
 	PgStat_EntryRef *entry_ref;
-	PgStat_BackendFunctionEntry *pending;
+	PgStat_FunctionCounts *pending;
 	bool		created_entry;
 
 	if (pgstat_track_functions <= fcinfo->flinfo->fn_stats)
@@ -121,10 +121,10 @@ pgstat_init_function_usage(FunctionCallInfo fcinfo,
 
 	pending = entry_ref->pending;
 
-	fcu->fs = &pending->f_counts;
+	fcu->fs = pending;
 
 	/* save stats for this function, later used to compensate for recursion */
-	fcu->save_f_total_time = pending->f_counts.f_total_time;
+	fcu->save_f_total_time = pending->f_total_time;
 
 	/* save current backend-wide total time */
 	fcu->save_total = total_func_time;
@@ -192,10 +192,10 @@ pgstat_end_function_usage(PgStat_FunctionCallUsage *fcu, bool finalize)
 bool
 pgstat_function_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
 {
-	PgStat_BackendFunctionEntry *localent;
+	PgStat_FunctionCounts *localent;
 	PgStatShared_Function *shfuncent;
 
-	localent = (PgStat_BackendFunctionEntry *) entry_ref->pending;
+	localent = (PgStat_FunctionCounts *) entry_ref->pending;
 	shfuncent = (PgStatShared_Function *) entry_ref->shared_stats;
 
 	/* localent always has non-zero content */
@@ -203,11 +203,11 @@ pgstat_function_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
 	if (!pgstat_lock_entry(entry_ref, nowait))
 		return false;
 
-	shfuncent->stats.f_numcalls += localent->f_counts.f_numcalls;
+	shfuncent->stats.f_numcalls += localent->f_numcalls;
 	shfuncent->stats.f_total_time +=
-		INSTR_TIME_GET_MICROSEC(localent->f_counts.f_total_time);
+		INSTR_TIME_GET_MICROSEC(localent->f_total_time);
 	shfuncent->stats.f_self_time +=
-		INSTR_TIME_GET_MICROSEC(localent->f_counts.f_self_time);
+		INSTR_TIME_GET_MICROSEC(localent->f_self_time);
 
 	pgstat_unlock_entry(entry_ref);
 
@@ -215,11 +215,11 @@ pgstat_function_flush_cb(PgStat_EntryRef *entry_ref, bool nowait)
 }
 
 /*
- * find any existing PgStat_BackendFunctionEntry entry for specified function
+ * find any existing PgStat_FunctionCounts entry for specified function
  *
  * If no entry, return NULL, don't create a new one
  */
-PgStat_BackendFunctionEntry *
+PgStat_FunctionCounts *
 find_funcstat_entry(Oid func_id)
 {
 	PgStat_EntryRef *entry_ref;
diff --git a/src/backend/utils/adt/pgstatfuncs.c b/src/backend/utils/adt/pgstatfuncs.c
index 9d707c3521..9819afa462 100644
--- a/src/backend/utils/adt/pgstatfuncs.c
+++ b/src/backend/utils/adt/pgstatfuncs.c
@@ -1654,33 +1654,33 @@ Datum
 pg_stat_get_xact_function_calls(PG_FUNCTION_ARGS)
 {
 	Oid			funcid = PG_GETARG_OID(0);
-	PgStat_BackendFunctionEntry *funcentry;
+	PgStat_FunctionCounts *funcentry;
 
 	if ((funcentry = find_funcstat_entry(funcid)) == NULL)
 		PG_RETURN_NULL();
-	PG_RETURN_INT64(funcentry->f_counts.f_numcalls);
+	PG_RETURN_INT64(funcentry->f_numcalls);
 }
 
 Datum
 pg_stat_get_xact_function_total_time(PG_FUNCTION_ARGS)
 {
 	Oid			funcid = PG_GETARG_OID(0);
-	PgStat_BackendFunctionEntry *funcentry;
+	PgStat_FunctionCounts *funcentry;
 
 	if ((funcentry = find_funcstat_entry(funcid)) == NULL)
 		PG_RETURN_NULL();
-	PG_RETURN_FLOAT8(INSTR_TIME_GET_MILLISEC(funcentry->f_counts.f_total_time));
+	PG_RETURN_FLOAT8(INSTR_TIME_GET_MILLISEC(funcentry->f_total_time));
 }
 
 Datum
 pg_stat_get_xact_function_self_time(PG_FUNCTION_ARGS)
 {
 	Oid			funcid = PG_GETARG_OID(0);
-	PgStat_BackendFunctionEntry *funcentry;
+	PgStat_FunctionCounts *funcentry;
 
 	if ((funcentry = find_funcstat_entry(funcid)) == NULL)
 		PG_RETURN_NULL();
-	PG_RETURN_FLOAT8(INSTR_TIME_GET_MILLISEC(funcentry->f_counts.f_self_time));
+	PG_RETURN_FLOAT8(INSTR_TIME_GET_MILLISEC(funcentry->f_self_time));
 }
 
 
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
index db9675884f..c10e4da65f 100644
--- a/src/include/pgstat.h
+++ b/src/include/pgstat.h
@@ -111,15 +111,6 @@ typedef struct PgStat_FunctionCounts
 	instr_time	f_self_time;
 } PgStat_FunctionCounts;
 
-/* ----------
- * PgStat_BackendFunctionEntry	Non-flushed function stats.
- * ----------
- */
-typedef struct PgStat_BackendFunctionEntry
-{
-	PgStat_FunctionCounts f_counts;
-} PgStat_BackendFunctionEntry;
-
 /*
  * Working state needed to accumulate per-function-call timing statistics.
  */
@@ -559,7 +550,7 @@ extern void pgstat_end_function_usage(PgStat_FunctionCallUsage *fcu,
 									  bool finalize);
 
 extern PgStat_StatFuncEntry *pgstat_fetch_stat_funcentry(Oid func_id);
-extern PgStat_BackendFunctionEntry *find_funcstat_entry(Oid func_id);
+extern PgStat_FunctionCounts *find_funcstat_entry(Oid func_id);
 
 
 /*
#2Nathan Bossart
nathandbossart@gmail.com
In reply to: Drouvot, Bertrand (#1)
Re: Get rid of PgStat_BackendFunctionEntry

On Mon, Feb 13, 2023 at 10:06:27AM +0100, Drouvot, Bertrand wrote:

Please find attached a patch proposal to $SUBJECT: it would allow to simplify
even more the work done to generate pg_stat_get_xact*() functions with Macros.

This is a follow up for [1] where it has been suggested
to get rid of PgStat_BackendFunctionEntry in a separate patch.

Looks reasonable to me.

--
Nathan Bossart
Amazon Web Services: https://aws.amazon.com

#3Michael Paquier
michael@paquier.xyz
In reply to: Nathan Bossart (#2)
Re: Get rid of PgStat_BackendFunctionEntry

On Wed, Mar 08, 2023 at 01:38:38PM -0800, Nathan Bossart wrote:

Looks reasonable to me.

I have been catching up with this thread and the other thread, and
indeed it looks like this is going to help in refactoring
pgstatfuncs.c to have more macros for all these mostly-duplicated
functions. So, I have applied this bit.
--
Michael

#4Drouvot, Bertrand
bertranddrouvot.pg@gmail.com
In reply to: Michael Paquier (#3)
Re: Get rid of PgStat_BackendFunctionEntry

Hi,

On 3/16/23 6:25 AM, Michael Paquier wrote:

On Wed, Mar 08, 2023 at 01:38:38PM -0800, Nathan Bossart wrote:

Looks reasonable to me.

I have been catching up with this thread and the other thread, and
indeed it looks like this is going to help in refactoring
pgstatfuncs.c to have more macros for all these mostly-duplicated
functions. So, I have applied this bit.

Thanks!

Regards,

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