Merge pgstat_count_io_op_n() and pgstat_count_io_op()

michaelpq · michaelpq · commit f0bf7857befe · 2025-01-10T09:57:27.000+09:00
The pgstat_count_io_op() function, which counts a single I/O operation, wraps pgstat_count_io_op_n() with a counter value of 1. The latter is declared in pgstat.h and used nowhere in the code, so let's remove it in favor of the former. This change makes also the code more symmetric with pgstat_count_io_op_time(), that already uses a similar set of arguments, except that it counts also the I/O time. This will ease a bit the integration of a follow-up patch that adds byte-level tracking in pg_stat_io for some of its attributes, lifting the current restriction based on BLCKSZ as all I/O operations are assumed to be block-based. Author: Nazir Bilal Yavuz Reviewed-by: Bertrand Drouvot Discussion: https://postgr.es/m/CAN55FZ32ze812=yjyZg1QeXhKvACUM_Nu0_gyPQcUKKuVHL5xA@mail.gmail.com
diff --git a/src/backend/storage/buffer/bufmgr.c b/src/backend/storage/buffer/bufmgr.c
@@ -1165,7 +1165,7 @@ PinBufferForBlock(Relation rel,
 	}
 	if (*foundPtr)
 	{
-		pgstat_count_io_op(io_object, io_context, IOOP_HIT);
+		pgstat_count_io_op(io_object, io_context, IOOP_HIT, 1);
 		if (VacuumCostActive)
 			VacuumCostBalance += VacuumCostPageHit;
 
@@ -2073,7 +2073,7 @@ GetVictimBuffer(BufferAccessStrategy strategy, IOContext io_context)
 		 * pinners or erroring out.
 		 */
 		pgstat_count_io_op(IOOBJECT_RELATION, io_context,
-						   from_ring ? IOOP_REUSE : IOOP_EVICT);
+						   from_ring ? IOOP_REUSE : IOOP_EVICT, 1);
 	}
 
 	/*
diff --git a/src/backend/storage/buffer/localbuf.c b/src/backend/storage/buffer/localbuf.c
@@ -279,7 +279,7 @@ GetLocalVictimBuffer(void)
 		ClearBufferTag(&bufHdr->tag);
 		buf_state &= ~(BUF_FLAG_MASK | BUF_USAGECOUNT_MASK);
 		pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state);
-		pgstat_count_io_op(IOOBJECT_TEMP_RELATION, IOCONTEXT_NORMAL, IOOP_EVICT);
+		pgstat_count_io_op(IOOBJECT_TEMP_RELATION, IOCONTEXT_NORMAL, IOOP_EVICT, 1);
 	}
 
 	return BufferDescriptorGetBuffer(bufHdr);
diff --git a/src/backend/utils/activity/pgstat_io.c b/src/backend/utils/activity/pgstat_io.c
@@ -66,13 +66,7 @@ pgstat_bktype_io_stats_valid(PgStat_BktypeIO *backend_io,
 }
 
 void
-pgstat_count_io_op(IOObject io_object, IOContext io_context, IOOp io_op)
-{
-	pgstat_count_io_op_n(io_object, io_context, io_op, 1);
-}
-
-void
-pgstat_count_io_op_n(IOObject io_object, IOContext io_context, IOOp io_op, uint32 cnt)
+pgstat_count_io_op(IOObject io_object, IOContext io_context, IOOp io_op, uint32 cnt)
 {
 	Assert((unsigned int) io_object < IOOBJECT_NUM_TYPES);
 	Assert((unsigned int) io_context < IOCONTEXT_NUM_TYPES);
@@ -116,7 +110,7 @@ pgstat_prepare_io_time(bool track_io_guc)
 }
 
 /*
- * Like pgstat_count_io_op_n() except it also accumulates time.
+ * Like pgstat_count_io_op() except it also accumulates time.
  */
 void
 pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op,
@@ -159,7 +153,7 @@ pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op,
 		}
 	}
 
-	pgstat_count_io_op_n(io_object, io_context, io_op, cnt);
+	pgstat_count_io_op(io_object, io_context, io_op, cnt);
 }
 
 PgStat_IO *
diff --git a/src/include/pgstat.h b/src/include/pgstat.h
@@ -603,8 +603,8 @@ extern PgStat_CheckpointerStats *pgstat_fetch_stat_checkpointer(void);
 
 extern bool pgstat_bktype_io_stats_valid(PgStat_BktypeIO *backend_io,
 										 BackendType bktype);
-extern void pgstat_count_io_op(IOObject io_object, IOContext io_context, IOOp io_op);
-extern void pgstat_count_io_op_n(IOObject io_object, IOContext io_context, IOOp io_op, uint32 cnt);
+extern void pgstat_count_io_op(IOObject io_object, IOContext io_context,
+							   IOOp io_op, uint32 cnt);
 extern instr_time pgstat_prepare_io_time(bool track_io_guc);
 extern void pgstat_count_io_op_time(IOObject io_object, IOContext io_context,
 									IOOp io_op, instr_time start_time, uint32 cnt);

Original file line number	Diff line number	Diff line change
`@@ -1165,7 +1165,7 @@ PinBufferForBlock(Relation rel,`
`1165`	`1165`	`}`
`1166`	`1166`	`if (*foundPtr)`
`1167`	`1167`	`{`
`1168`		`- pgstat_count_io_op(io_object, io_context, IOOP_HIT);`
	`1168`	`+ pgstat_count_io_op(io_object, io_context, IOOP_HIT, 1);`
`1169`	`1169`	`if (VacuumCostActive)`
`1170`	`1170`	`VacuumCostBalance += VacuumCostPageHit;`
`1171`	`1171`
`@@ -2073,7 +2073,7 @@ GetVictimBuffer(BufferAccessStrategy strategy, IOContext io_context)`
`2073`	`2073`	`* pinners or erroring out.`
`2074`	`2074`	`*/`
`2075`	`2075`	`pgstat_count_io_op(IOOBJECT_RELATION, io_context,`
`2076`		`- from_ring ? IOOP_REUSE : IOOP_EVICT);`
	`2076`	`+ from_ring ? IOOP_REUSE : IOOP_EVICT, 1);`
`2077`	`2077`	`}`
`2078`	`2078`
`2079`	`2079`	`/*`
Original file line number	Diff line number	Diff line change
`@@ -279,7 +279,7 @@ GetLocalVictimBuffer(void)`
`279`	`279`	`ClearBufferTag(&bufHdr->tag);`
`280`	`280`	`buf_state &= ~(BUF_FLAG_MASK \| BUF_USAGECOUNT_MASK);`
`281`	`281`	`pg_atomic_unlocked_write_u32(&bufHdr->state, buf_state);`
`282`		`- pgstat_count_io_op(IOOBJECT_TEMP_RELATION, IOCONTEXT_NORMAL, IOOP_EVICT);`
	`282`	`+ pgstat_count_io_op(IOOBJECT_TEMP_RELATION, IOCONTEXT_NORMAL, IOOP_EVICT, 1);`
`283`	`283`	`}`
`284`	`284`
`285`	`285`	`return BufferDescriptorGetBuffer(bufHdr);`
Original file line number	Diff line number	Diff line change
`@@ -66,13 +66,7 @@ pgstat_bktype_io_stats_valid(PgStat_BktypeIO *backend_io,`
`66`	`66`	`}`
`67`	`67`
`68`	`68`	`void`
`69`		`-pgstat_count_io_op(IOObject io_object, IOContext io_context, IOOp io_op)`
`70`		`-{`
`71`		`- pgstat_count_io_op_n(io_object, io_context, io_op, 1);`
`72`		`-}`
`73`		`-`
`74`		`-void`
`75`		`-pgstat_count_io_op_n(IOObject io_object, IOContext io_context, IOOp io_op, uint32 cnt)`
	`69`	`+pgstat_count_io_op(IOObject io_object, IOContext io_context, IOOp io_op, uint32 cnt)`
`76`	`70`	`{`
`77`	`71`	`Assert((unsigned int) io_object < IOOBJECT_NUM_TYPES);`
`78`	`72`	`Assert((unsigned int) io_context < IOCONTEXT_NUM_TYPES);`
`@@ -116,7 +110,7 @@ pgstat_prepare_io_time(bool track_io_guc)`
`116`	`110`	`}`
`117`	`111`
`118`	`112`	`/*`
`119`		`- * Like pgstat_count_io_op_n() except it also accumulates time.`
	`113`	`+ * Like pgstat_count_io_op() except it also accumulates time.`
`120`	`114`	`*/`
`121`	`115`	`void`
`122`	`116`	`pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op,`
`@@ -159,7 +153,7 @@ pgstat_count_io_op_time(IOObject io_object, IOContext io_context, IOOp io_op,`
`159`	`153`	`}`
`160`	`154`	`}`
`161`	`155`
`162`		`- pgstat_count_io_op_n(io_object, io_context, io_op, cnt);`
	`156`	`+ pgstat_count_io_op(io_object, io_context, io_op, cnt);`
`163`	`157`	`}`
`164`	`158`
`165`	`159`	`PgStat_IO *`