agora inbox for [email protected]  

help / color / mirror / Atom feed

[PATCH v11 4/5] fixes
14+ messages / 2 participants
[nested] [flat]

* [PATCH v11 4/5] fixes
@ 2020-02-29 01:20 Justin Pryzby <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Justin Pryzby @ 2020-02-29 01:20 UTC (permalink / raw)

---
 src/backend/catalog/index.c               | 27 +++++++++-----
 src/backend/commands/cluster.c            | 21 ++++++++++-
 src/backend/commands/indexcmds.c          | 43 ++++++++++-------------
 src/backend/commands/vacuum.c             |  3 +-
 src/backend/nodes/copyfuncs.c             |  1 +
 src/backend/nodes/equalfuncs.c            |  1 +
 src/backend/parser/gram.y                 |  3 ++
 src/backend/postmaster/autovacuum.c       |  1 +
 src/test/regress/output/tablespace.source | 11 +++---
 9 files changed, 70 insertions(+), 41 deletions(-)

diff --git a/src/backend/catalog/index.c b/src/backend/catalog/index.c
index e6c06e6a06..05c4b51165 100644
--- a/src/backend/catalog/index.c
+++ b/src/backend/catalog/index.c
@@ -3434,6 +3434,7 @@ reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks,
 	volatile bool skipped_constraint = false;
 	PGRUsage	ru0;
 	bool		progress = (options & REINDEXOPT_REPORT_PROGRESS) != 0;
+	bool		valid_tablespace = OidIsValid(tablespaceOid);
 
 	pg_rusage_init(&ru0);
 
@@ -3473,14 +3474,19 @@ reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks,
 			 RelationGetRelationName(iRel));
 
 	/*
-	 * We cannot support moving mapped relations into different tablespaces.
-	 * (In particular this eliminates all shared catalogs.)
+	 * We don't support moving system relations into different tablespaces.
 	 */
-	if (OidIsValid(tablespaceOid) && RelationIsMapped(iRel))
+	if (OidIsValid(tablespaceOid) &&
+			((IsCatalogRelationOid(indexId) && !allowSystemTableMods)
+			|| RelationIsMapped(iRel)))
+	{
+		check_relation_is_movable(iRel, tablespaceOid);
 		ereport(ERROR,
-				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-				 errmsg("cannot move system relation \"%s\"",
-						RelationGetRelationName(iRel))));
+				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+				 errmsg("permission denied: \"%s\" is a system catalog",
+					 RelationGetRelationName(iRel)),
+				 errdetail("cannot change tablespace of system catalog")));
+	}
 
 	/*
 	 * Don't allow reindex on temp tables of other backends ... their local
@@ -3508,11 +3514,14 @@ reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks,
 	 */
 	CheckTableNotInUse(iRel, "REINDEX INDEX");
 
+	tablespaceOid = (tablespaceOid == MyDatabaseTableSpace) ?
+		InvalidOid : tablespaceOid;
+
 	/*
 	 * Set the new tablespace for the relation.  Do that only in the
 	 * case where the reindex caller wishes to enforce a new tablespace.
 	 */
-	if (OidIsValid(tablespaceOid) &&
+	if (valid_tablespace &&
 		tablespaceOid != iRel->rd_rel->reltablespace)
 	{
 		Relation		pg_class;
@@ -3535,8 +3544,8 @@ reindex_index(Oid indexId, Oid tablespaceOid, bool skip_constraint_checks,
 		RelationDropStorage(iRel);
 
 		/* Update the pg_class row */
-		rd_rel->reltablespace = (tablespaceOid == MyDatabaseTableSpace) ?
-			InvalidOid : tablespaceOid;
+		rd_rel->reltablespace = tablespaceOid;
+
 		CatalogTupleUpdate(pg_class, &tuple->t_self, tuple);
 
 		heap_freetuple(tuple);
diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 794d4be30c..fdc62a1996 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -383,6 +383,18 @@ cluster_rel(Oid tableOid, Oid indexOid, Oid tableSpaceOid, int options)
 				(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
 				 errmsg("cannot cluster a shared catalog")));
 
+	if (!allowSystemTableMods && IsSystemRelation(OldHeap) &&
+		OidIsValid(tableSpaceOid))
+		// tableSpaceOid != OldHeap->rd_rel->reltablespace)
+	{
+		check_relation_is_movable(OldHeap, tableSpaceOid);
+		ereport(ERROR,
+				(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+				 errmsg("permission denied: \"%s\" is a system catalog",
+					 RelationGetRelationName(OldHeap)),
+				errdetail("cannot change tablespace of system catalog")));
+	}
+
 	/*
 	 * Don't process temp tables of other backends ... their local buffer
 	 * manager is not going to cope.
@@ -603,7 +615,7 @@ rebuild_relation(Relation OldHeap, Oid indexOid, Oid NewTableSpaceOid, bool verb
 	TransactionId frozenXid;
 	MultiXactId cutoffMulti;
 
-	/* Use new TeableSpace if passed. */
+	/* Use new TableSpace if passed. */
 	if (OidIsValid(NewTableSpaceOid))
 		tableSpace = NewTableSpaceOid;
 
@@ -1051,6 +1063,13 @@ swap_relation_files(Oid r1, Oid r2, bool target_is_pg_class,
 		 */
 		Assert(!target_is_pg_class);
 
+		if (!allowSystemTableMods && IsSystemClass(r1, relform1) &&
+				relform1->reltablespace != relform2->reltablespace)
+			ereport(ERROR,
+					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+					 errmsg("permission denied: \"%s\" is a system catalog",
+						 get_rel_name(r1))));
+
 		swaptemp = relform1->relfilenode;
 		relform1->relfilenode = relform2->relfilenode;
 		relform2->relfilenode = swaptemp;
diff --git a/src/backend/commands/indexcmds.c b/src/backend/commands/indexcmds.c
index 1d394d5248..448e2f9054 100644
--- a/src/backend/commands/indexcmds.c
+++ b/src/backend/commands/indexcmds.c
@@ -2521,7 +2521,7 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, char
 	ListCell   *l;
 	int			num_keys;
 	bool		concurrent_warning = false;
-	bool		mapped_warning = false;
+	bool		tblspc_warning = false;
 
 	AssertArg(objectName);
 	Assert(objectKind == REINDEX_OBJECT_SCHEMA ||
@@ -2562,17 +2562,8 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, char
 
 	/* Define new tablespaceOid if it is wanted by caller */
 	if (newTableSpaceName)
-	{
 		tablespaceOid = get_tablespace_oid(newTableSpaceName, false);
 
-		/* Can't move a non-shared relation into pg_global */
-		if (tablespaceOid == GLOBALTABLESPACE_OID)
-			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("cannot move non-shared relation to tablespace \"%s\"",
-							newTableSpaceName)));
-	}
-
 	/*
 	 * Create a memory context that will survive forced transaction commits we
 	 * do below.  Since it is a child of PortalContext, it will go away
@@ -2664,18 +2655,18 @@ ReindexMultipleTables(const char *objectName, ReindexObjectType objectKind, char
 		}
 
 		/*
-		 * Skip all mapped relations if TABLESPACE is specified.
-		 * relfilenode == 0 checks after that, similarly to
-		 * RelationIsMapped().
+		 * Skip all system relations if TABLESPACE is specified.
+		 * Skip mapped relations (relfilenode=0) even if allowing system table mods.
 		 */
 		if (OidIsValid(tablespaceOid) &&
-			!OidIsValid(classtuple->relfilenode))
+			((!allowSystemTableMods && IsSystemClass(relid, classtuple)) ||
+			 !OidIsValid(classtuple->relfilenode)))
 		{
-			if (!mapped_warning)
+			if (!tblspc_warning)
 				ereport(WARNING,
 						(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-						 errmsg("cannot change tablespace of indexes for mapped relations, skipping all")));
-			mapped_warning = true;
+						 errmsg("cannot change tablespace of indexes on system catalogs, skipping all")));
+			tblspc_warning = true;
 			continue;
 		}
 
@@ -2843,11 +2834,12 @@ ReindexRelationConcurrently(Oid relationOid, Oid tablespaceOid, int options)
 				/* Open relation to get its indexes */
 				heapRelation = table_open(relationOid, ShareUpdateExclusiveLock);
 
-				if (OidIsValid(tablespaceOid) && RelationIsMapped(heapRelation))
+				if (OidIsValid(tablespaceOid) && IsSystemRelation(heapRelation))
 					ereport(ERROR,
-							(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-							 errmsg("cannot change tablespace of indexes for mapped relation \"%s\"",
-									RelationGetRelationName(heapRelation))));
+							(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+							 errmsg("permission denied: \"%s\" is a system catalog",
+								 RelationGetRelationName(heapRelation)),
+							 errdetail("cannot change tablespace of system catalog")));
 
 				/* Add all the valid indexes of relation to list */
 				foreach(lc, RelationGetIndexList(heapRelation))
@@ -3031,11 +3023,12 @@ ReindexRelationConcurrently(Oid relationOid, Oid tablespaceOid, int options)
 		if (indexRel->rd_rel->relpersistence == RELPERSISTENCE_TEMP)
 			elog(ERROR, "cannot reindex a temporary table concurrently");
 
-		if (OidIsValid(tablespaceOid) && RelationIsMapped(heapRel))
+		if (OidIsValid(tablespaceOid) && IsSystemRelation(heapRel))
 			ereport(ERROR,
-					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-					 errmsg("cannot change tablespace of mapped relation \"%s\"",
-							RelationGetRelationName(heapRel))));
+					(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+					 errmsg("permission denied: \"%s\" is a system catalog",
+						 RelationGetRelationName(heapRel)),
+						 errdetail("cannot change tablespace of system catalog")));
 
 		pgstat_progress_start_command(PROGRESS_COMMAND_CREATE_INDEX,
 									  RelationGetRelid(heapRel));
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 6dfdce32eb..095dbbce96 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -1827,7 +1827,8 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
 	/*
 	 * We cannot support moving system relations into different tablespaces.
 	 */
-	if (params->options & VACOPT_FULL && OidIsValid(params->tablespace_oid) && IsSystemRelation(onerel))
+	if (params->options & VACOPT_FULL && OidIsValid(params->tablespace_oid)
+			&& IsSystemRelation(onerel))
 	{
 		ereport(WARNING,
 			(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 7bf9aa7e75..891bc37b08 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -3901,6 +3901,7 @@ _copyVacuumStmt(const VacuumStmt *from)
 	COPY_NODE_FIELD(options);
 	COPY_NODE_FIELD(rels);
 	COPY_SCALAR_FIELD(is_vacuumcmd);
+	COPY_STRING_FIELD(tablespacename);
 
 	return newnode;
 }
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 048c59fd68..da55563ca8 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -1702,6 +1702,7 @@ _equalVacuumStmt(const VacuumStmt *a, const VacuumStmt *b)
 	COMPARE_NODE_FIELD(options);
 	COMPARE_NODE_FIELD(rels);
 	COMPARE_SCALAR_FIELD(is_vacuumcmd);
+	COMPARE_SCALAR_FIELD(tablespacename);
 
 	return true;
 }
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 1f388017be..300e6409f8 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -10615,6 +10615,7 @@ ClusterStmt:
 					n->options = 0;
 					if ($2)
 						n->options |= CLUOPT_VERBOSE;
+					n->tablespacename = NULL;
 					$$ = (Node*)n;
 				}
 			/* kept for pre-8.3 compatibility */
@@ -10718,6 +10719,7 @@ AnalyzeStmt: analyze_keyword opt_verbose opt_vacuum_relation_list
 											 makeDefElem("verbose", NULL, @2));
 					n->rels = $3;
 					n->is_vacuumcmd = false;
+					n->tablespacename = NULL;
 					$$ = (Node *)n;
 				}
 			| analyze_keyword '(' vac_analyze_option_list ')' opt_vacuum_relation_list
@@ -10726,6 +10728,7 @@ AnalyzeStmt: analyze_keyword opt_verbose opt_vacuum_relation_list
 					n->options = $3;
 					n->rels = $5;
 					n->is_vacuumcmd = false;
+					n->tablespacename = NULL;
 					$$ = (Node *) n;
 				}
 		;
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index da75e755f0..ad538a872f 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2895,6 +2895,7 @@ table_recheck_autovac(Oid relid, HTAB *table_toast_map,
 		tab->at_params.multixact_freeze_table_age = multixact_freeze_table_age;
 		tab->at_params.is_wraparound = wraparound;
 		tab->at_params.log_min_duration = log_min_duration;
+		tab->at_params.tablespace_oid = InvalidOid;
 		tab->at_vacuum_cost_limit = vac_cost_limit;
 		tab->at_vacuum_cost_delay = vac_cost_delay;
 		tab->at_relname = NULL;
diff --git a/src/test/regress/output/tablespace.source b/src/test/regress/output/tablespace.source
index 5634cf20ff..0efe6cf4cc 100644
--- a/src/test/regress/output/tablespace.source
+++ b/src/test/regress/output/tablespace.source
@@ -33,7 +33,8 @@ REINDEX TABLE regress_tblspace_test_tbl TABLESPACE regress_tblspace;
 ROLLBACK;
 BEGIN;
 REINDEX TABLE pg_am TABLESPACE regress_tblspace;
-ERROR:  cannot move system relation "pg_am_name_index"
+ERROR:  permission denied: "pg_am_name_index" is a system catalog
+DETAIL:  cannot change tablespace of system catalog
 ROLLBACK;
 SELECT relname FROM pg_class
 WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace');
@@ -43,9 +44,9 @@ WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspa
 
 -- first, let us reindex and move the entire database, after that return everything back
 REINDEX DATABASE regression TABLESPACE regress_tblspace; -- ok with warning
-WARNING:  cannot change tablespace of indexes for mapped relations, skipping all
+WARNING:  cannot change tablespace of indexes on system catalogs, skipping all
 REINDEX DATABASE regression TABLESPACE pg_default; -- ok with warning
-WARNING:  cannot change tablespace of indexes for mapped relations, skipping all
+WARNING:  cannot change tablespace of indexes on system catalogs, skipping all
 SELECT relname FROM pg_class
 WHERE reltablespace=(SELECT oid FROM pg_tablespace WHERE spcname='regress_tblspace');
  relname 
@@ -58,7 +59,7 @@ REINDEX TABLE regress_tblspace_test_tbl TABLESPACE regress_tblspace; -- ok
 REINDEX TABLE pg_authid TABLESPACE regress_tblspace; -- fail
 ERROR:  cannot move system relation "pg_authid_rolname_index"
 REINDEX SCHEMA pg_catalog TABLESPACE regress_tblspace; -- fail
-WARNING:  cannot change tablespace of indexes for mapped relations, skipping all
+WARNING:  cannot change tablespace of indexes on system catalogs, skipping all
 REINDEX DATABASE postgres TABLESPACE regress_tblspace; -- fail
 ERROR:  can only reindex the currently open database
 REINDEX SYSTEM postgres TABLESPACE regress_tblspace; -- fail
@@ -66,7 +67,7 @@ ERROR:  can only reindex the currently open database
 REINDEX INDEX regress_tblspace_test_tbl_idx TABLESPACE pg_global; -- fail
 ERROR:  cannot move non-shared relation to tablespace "pg_global"
 REINDEX SCHEMA pg_catalog TABLESPACE regress_tblspace; -- fail
-WARNING:  cannot change tablespace of indexes for mapped relations, skipping all
+WARNING:  cannot change tablespace of indexes on system catalogs, skipping all
 REINDEX DATABASE postgres TABLESPACE regress_tblspace; -- fail
 ERROR:  can only reindex the currently open database
 REINDEX SYSTEM postgres TABLESPACE regress_tblspace; -- fail
-- 
2.17.0


--k4f25fnPtRuIRUb3
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
 filename="v11-0005-Specially-handle-toast-relations-during-REINDEX-.patch"



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

* [PATCH v8 3/3] Make use of pg_popcount() in more places.
@ 2026-01-22 17:16 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Nathan Bossart @ 2026-01-22 17:16 UTC (permalink / raw)

This replaces some loops over word-length popcount functions with
calls to our perhaps-over-optimized pg_popcount() function.  Since
pg_popcount() uses a function pointer for inputs with sizes >= a
Bitmapset word, this produces a small regression for the common
one-word case in bms_num_members().  To deal with that, this commit
adds an inlined fast-path for that case.  This fast-path could
arguably go in pg_popcount() itself (with an appropriate alignment
check), but that is left as a future exercise.

Suggested-by: John Naylor <[email protected]>
Reviewed-by: John Naylor <[email protected]>
Discussion: https://postgr.es/m/CANWCAZY7R%2Biy%2Br9YM_sySNydHzNqUirx1xk0tB3ej5HO62GdgQ%40mail.gmail.com
---
 src/backend/nodes/bitmapset.c | 29 +++++++----------------------
 src/include/lib/radixtree.h   |  4 ++--
 2 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index a4765876c31..786f343b3c9 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -553,14 +553,8 @@ bms_member_index(Bitmapset *a, int x)
 	bitnum = BITNUM(x);
 
 	/* count bits in preceding words */
-	for (int i = 0; i < wordnum; i++)
-	{
-		bitmapword	w = a->words[i];
-
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	}
+	result += pg_popcount((const char *) a->words,
+						  wordnum * sizeof(bitmapword));
 
 	/*
 	 * Now add bits of the last word, but only those before the item. We can
@@ -749,26 +743,17 @@ bms_get_singleton_member(const Bitmapset *a, int *member)
 int
 bms_num_members(const Bitmapset *a)
 {
-	int			result = 0;
-	int			nwords;
-	int			wordnum;
-
 	Assert(bms_is_valid_set(a));
 
 	if (a == NULL)
 		return 0;
 
-	nwords = a->nwords;
-	wordnum = 0;
-	do
-	{
-		bitmapword	w = a->words[wordnum];
+	/* fast-path for common case */
+	if (a->nwords == 1)
+		return bmw_popcount(a->words[0]);
 
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	} while (++wordnum < nwords);
-	return result;
+	return pg_popcount((const char *) a->words,
+					   a->nwords * sizeof(bitmapword));
 }
 
 /*
diff --git a/src/include/lib/radixtree.h b/src/include/lib/radixtree.h
index b223ce10a2d..1425654a67c 100644
--- a/src/include/lib/radixtree.h
+++ b/src/include/lib/radixtree.h
@@ -2725,8 +2725,8 @@ RT_VERIFY_NODE(RT_NODE * node)
 
 				/* RT_DUMP_NODE(node); */
 
-				for (int i = 0; i < RT_BM_IDX(RT_NODE_MAX_SLOTS); i++)
-					cnt += bmw_popcount(n256->isset[i]);
+				cnt += pg_popcount((const char *) n256->isset,
+								   RT_NODE_MAX_SLOTS / BITS_PER_BYTE);
 
 				/*
 				 * Check if the number of used chunk matches, accounting for
-- 
2.50.1 (Apple Git-155)


--dIDo5IfS/hVe25hD--





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

* [PATCH v4 1/4] Make use of pg_popcount() in more places.
@ 2026-01-22 17:16 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Nathan Bossart @ 2026-01-22 17:16 UTC (permalink / raw)

---
 src/backend/nodes/bitmapset.c | 27 ++++-----------------------
 src/include/lib/radixtree.h   |  4 ++--
 2 files changed, 6 insertions(+), 25 deletions(-)

diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index a4765876c31..23c91fdb6c9 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -553,14 +553,8 @@ bms_member_index(Bitmapset *a, int x)
 	bitnum = BITNUM(x);
 
 	/* count bits in preceding words */
-	for (int i = 0; i < wordnum; i++)
-	{
-		bitmapword	w = a->words[i];
-
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	}
+	result += pg_popcount((const char *) a->words,
+						  wordnum * sizeof(bitmapword));
 
 	/*
 	 * Now add bits of the last word, but only those before the item. We can
@@ -749,26 +743,13 @@ bms_get_singleton_member(const Bitmapset *a, int *member)
 int
 bms_num_members(const Bitmapset *a)
 {
-	int			result = 0;
-	int			nwords;
-	int			wordnum;
-
 	Assert(bms_is_valid_set(a));
 
 	if (a == NULL)
 		return 0;
 
-	nwords = a->nwords;
-	wordnum = 0;
-	do
-	{
-		bitmapword	w = a->words[wordnum];
-
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	} while (++wordnum < nwords);
-	return result;
+	return pg_popcount((const char *) a->words,
+					   a->nwords * sizeof(bitmapword));
 }
 
 /*
diff --git a/src/include/lib/radixtree.h b/src/include/lib/radixtree.h
index b223ce10a2d..1425654a67c 100644
--- a/src/include/lib/radixtree.h
+++ b/src/include/lib/radixtree.h
@@ -2725,8 +2725,8 @@ RT_VERIFY_NODE(RT_NODE * node)
 
 				/* RT_DUMP_NODE(node); */
 
-				for (int i = 0; i < RT_BM_IDX(RT_NODE_MAX_SLOTS); i++)
-					cnt += bmw_popcount(n256->isset[i]);
+				cnt += pg_popcount((const char *) n256->isset,
+								   RT_NODE_MAX_SLOTS / BITS_PER_BYTE);
 
 				/*
 				 * Check if the number of used chunk matches, accounting for
-- 
2.50.1 (Apple Git-155)


--u/0bDz5KPuHk2IF+
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v4-0002-Remove-unnecessary-32-bit-optimizations-and-align.patch



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

* [PATCH v11 3/4] Make use of pg_popcount() in more places.
@ 2026-01-22 17:16 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Nathan Bossart @ 2026-01-22 17:16 UTC (permalink / raw)

This replaces some loops over word-length popcount functions with
calls to pg_popcount().  Since pg_popcount() uses a function
pointer for inputs with sizes >= a Bitmapset word, this produces a
small regression for the common one-word case in bms_num_members().
To deal with that, this commit adds an inlined fast-path for that
case.  This fast-path could arguably go in pg_popcount() itself
(with an appropriate alignment check), but that is left as a future
exercise.

Suggested-by: John Naylor <[email protected]>
Reviewed-by: John Naylor <[email protected]>
Discussion: https://postgr.es/m/CANWCAZY7R%2Biy%2Br9YM_sySNydHzNqUirx1xk0tB3ej5HO62GdgQ%40mail.gmail.com
---
 src/backend/nodes/bitmapset.c | 29 +++++++----------------------
 src/include/lib/radixtree.h   |  6 +++---
 2 files changed, 10 insertions(+), 25 deletions(-)

diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index a4765876c31..786f343b3c9 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -553,14 +553,8 @@ bms_member_index(Bitmapset *a, int x)
 	bitnum = BITNUM(x);
 
 	/* count bits in preceding words */
-	for (int i = 0; i < wordnum; i++)
-	{
-		bitmapword	w = a->words[i];
-
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	}
+	result += pg_popcount((const char *) a->words,
+						  wordnum * sizeof(bitmapword));
 
 	/*
 	 * Now add bits of the last word, but only those before the item. We can
@@ -749,26 +743,17 @@ bms_get_singleton_member(const Bitmapset *a, int *member)
 int
 bms_num_members(const Bitmapset *a)
 {
-	int			result = 0;
-	int			nwords;
-	int			wordnum;
-
 	Assert(bms_is_valid_set(a));
 
 	if (a == NULL)
 		return 0;
 
-	nwords = a->nwords;
-	wordnum = 0;
-	do
-	{
-		bitmapword	w = a->words[wordnum];
+	/* fast-path for common case */
+	if (a->nwords == 1)
+		return bmw_popcount(a->words[0]);
 
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	} while (++wordnum < nwords);
-	return result;
+	return pg_popcount((const char *) a->words,
+					   a->nwords * sizeof(bitmapword));
 }
 
 /*
diff --git a/src/include/lib/radixtree.h b/src/include/lib/radixtree.h
index b223ce10a2d..e6c9a591c17 100644
--- a/src/include/lib/radixtree.h
+++ b/src/include/lib/radixtree.h
@@ -2721,12 +2721,12 @@ RT_VERIFY_NODE(RT_NODE * node)
 		case RT_NODE_KIND_256:
 			{
 				RT_NODE_256 *n256 = (RT_NODE_256 *) node;
-				int			cnt = 0;
+				int			cnt;
 
 				/* RT_DUMP_NODE(node); */
 
-				for (int i = 0; i < RT_BM_IDX(RT_NODE_MAX_SLOTS); i++)
-					cnt += bmw_popcount(n256->isset[i]);
+				cnt = pg_popcount((const char *) n256->isset,
+								  RT_NODE_MAX_SLOTS / BITS_PER_BYTE);
 
 				/*
 				 * Check if the number of used chunk matches, accounting for
-- 
2.50.1 (Apple Git-155)


--mih0IfQp6StcW7xc
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v11-0004-Remove-uses-of-popcount-builtins.patch



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

* [PATCH v5 1/3] Make use of pg_popcount() in more places.
@ 2026-01-22 17:16 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Nathan Bossart @ 2026-01-22 17:16 UTC (permalink / raw)

---
 src/backend/nodes/bitmapset.c | 27 ++++-----------------------
 src/include/lib/radixtree.h   |  4 ++--
 2 files changed, 6 insertions(+), 25 deletions(-)

diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index a4765876c31..23c91fdb6c9 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -553,14 +553,8 @@ bms_member_index(Bitmapset *a, int x)
 	bitnum = BITNUM(x);
 
 	/* count bits in preceding words */
-	for (int i = 0; i < wordnum; i++)
-	{
-		bitmapword	w = a->words[i];
-
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	}
+	result += pg_popcount((const char *) a->words,
+						  wordnum * sizeof(bitmapword));
 
 	/*
 	 * Now add bits of the last word, but only those before the item. We can
@@ -749,26 +743,13 @@ bms_get_singleton_member(const Bitmapset *a, int *member)
 int
 bms_num_members(const Bitmapset *a)
 {
-	int			result = 0;
-	int			nwords;
-	int			wordnum;
-
 	Assert(bms_is_valid_set(a));
 
 	if (a == NULL)
 		return 0;
 
-	nwords = a->nwords;
-	wordnum = 0;
-	do
-	{
-		bitmapword	w = a->words[wordnum];
-
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	} while (++wordnum < nwords);
-	return result;
+	return pg_popcount((const char *) a->words,
+					   a->nwords * sizeof(bitmapword));
 }
 
 /*
diff --git a/src/include/lib/radixtree.h b/src/include/lib/radixtree.h
index b223ce10a2d..1425654a67c 100644
--- a/src/include/lib/radixtree.h
+++ b/src/include/lib/radixtree.h
@@ -2725,8 +2725,8 @@ RT_VERIFY_NODE(RT_NODE * node)
 
 				/* RT_DUMP_NODE(node); */
 
-				for (int i = 0; i < RT_BM_IDX(RT_NODE_MAX_SLOTS); i++)
-					cnt += bmw_popcount(n256->isset[i]);
+				cnt += pg_popcount((const char *) n256->isset,
+								   RT_NODE_MAX_SLOTS / BITS_PER_BYTE);
 
 				/*
 				 * Check if the number of used chunk matches, accounting for
-- 
2.50.1 (Apple Git-155)


--a4C7SGgzwrPm67dM
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v5-0002-Remove-unnecessary-32-bit-optimizations-and-align.patch



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

* [PATCH v9 3/3] Make use of pg_popcount() in more places.
@ 2026-01-22 17:16 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Nathan Bossart @ 2026-01-22 17:16 UTC (permalink / raw)

This replaces some loops over word-length popcount functions with
calls to our perhaps-over-optimized pg_popcount() function.  Since
pg_popcount() uses a function pointer for inputs with sizes >= a
Bitmapset word, this produces a small regression for the common
one-word case in bms_num_members().  To deal with that, this commit
adds an inlined fast-path for that case.  This fast-path could
arguably go in pg_popcount() itself (with an appropriate alignment
check), but that is left as a future exercise.

Suggested-by: John Naylor <[email protected]>
Reviewed-by: John Naylor <[email protected]>
Discussion: https://postgr.es/m/CANWCAZY7R%2Biy%2Br9YM_sySNydHzNqUirx1xk0tB3ej5HO62GdgQ%40mail.gmail.com
---
 src/backend/nodes/bitmapset.c | 29 +++++++----------------------
 src/include/lib/radixtree.h   |  4 ++--
 2 files changed, 9 insertions(+), 24 deletions(-)

diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index a4765876c31..786f343b3c9 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -553,14 +553,8 @@ bms_member_index(Bitmapset *a, int x)
 	bitnum = BITNUM(x);
 
 	/* count bits in preceding words */
-	for (int i = 0; i < wordnum; i++)
-	{
-		bitmapword	w = a->words[i];
-
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	}
+	result += pg_popcount((const char *) a->words,
+						  wordnum * sizeof(bitmapword));
 
 	/*
 	 * Now add bits of the last word, but only those before the item. We can
@@ -749,26 +743,17 @@ bms_get_singleton_member(const Bitmapset *a, int *member)
 int
 bms_num_members(const Bitmapset *a)
 {
-	int			result = 0;
-	int			nwords;
-	int			wordnum;
-
 	Assert(bms_is_valid_set(a));
 
 	if (a == NULL)
 		return 0;
 
-	nwords = a->nwords;
-	wordnum = 0;
-	do
-	{
-		bitmapword	w = a->words[wordnum];
+	/* fast-path for common case */
+	if (a->nwords == 1)
+		return bmw_popcount(a->words[0]);
 
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	} while (++wordnum < nwords);
-	return result;
+	return pg_popcount((const char *) a->words,
+					   a->nwords * sizeof(bitmapword));
 }
 
 /*
diff --git a/src/include/lib/radixtree.h b/src/include/lib/radixtree.h
index b223ce10a2d..1425654a67c 100644
--- a/src/include/lib/radixtree.h
+++ b/src/include/lib/radixtree.h
@@ -2725,8 +2725,8 @@ RT_VERIFY_NODE(RT_NODE * node)
 
 				/* RT_DUMP_NODE(node); */
 
-				for (int i = 0; i < RT_BM_IDX(RT_NODE_MAX_SLOTS); i++)
-					cnt += bmw_popcount(n256->isset[i]);
+				cnt += pg_popcount((const char *) n256->isset,
+								   RT_NODE_MAX_SLOTS / BITS_PER_BYTE);
 
 				/*
 				 * Check if the number of used chunk matches, accounting for
-- 
2.50.1 (Apple Git-155)


--oEt0RkbzovU82iL7--





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

* [PATCH v10 3/3] Make use of pg_popcount() in more places.
@ 2026-01-22 17:16 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Nathan Bossart @ 2026-01-22 17:16 UTC (permalink / raw)

This replaces some loops over word-length popcount functions with
calls to our perhaps-over-optimized pg_popcount() function.  Since
pg_popcount() uses a function pointer for inputs with sizes >= a
Bitmapset word, this produces a small regression for the common
one-word case in bms_num_members().  To deal with that, this commit
adds an inlined fast-path for that case.  This fast-path could
arguably go in pg_popcount() itself (with an appropriate alignment
check), but that is left as a future exercise.

Suggested-by: John Naylor <[email protected]>
Reviewed-by: John Naylor <[email protected]>
Discussion: https://postgr.es/m/CANWCAZY7R%2Biy%2Br9YM_sySNydHzNqUirx1xk0tB3ej5HO62GdgQ%40mail.gmail.com
---
 src/backend/nodes/bitmapset.c | 29 +++++++----------------------
 src/include/lib/radixtree.h   |  6 +++---
 2 files changed, 10 insertions(+), 25 deletions(-)

diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index a4765876c31..786f343b3c9 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -553,14 +553,8 @@ bms_member_index(Bitmapset *a, int x)
 	bitnum = BITNUM(x);
 
 	/* count bits in preceding words */
-	for (int i = 0; i < wordnum; i++)
-	{
-		bitmapword	w = a->words[i];
-
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	}
+	result += pg_popcount((const char *) a->words,
+						  wordnum * sizeof(bitmapword));
 
 	/*
 	 * Now add bits of the last word, but only those before the item. We can
@@ -749,26 +743,17 @@ bms_get_singleton_member(const Bitmapset *a, int *member)
 int
 bms_num_members(const Bitmapset *a)
 {
-	int			result = 0;
-	int			nwords;
-	int			wordnum;
-
 	Assert(bms_is_valid_set(a));
 
 	if (a == NULL)
 		return 0;
 
-	nwords = a->nwords;
-	wordnum = 0;
-	do
-	{
-		bitmapword	w = a->words[wordnum];
+	/* fast-path for common case */
+	if (a->nwords == 1)
+		return bmw_popcount(a->words[0]);
 
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	} while (++wordnum < nwords);
-	return result;
+	return pg_popcount((const char *) a->words,
+					   a->nwords * sizeof(bitmapword));
 }
 
 /*
diff --git a/src/include/lib/radixtree.h b/src/include/lib/radixtree.h
index b223ce10a2d..e6c9a591c17 100644
--- a/src/include/lib/radixtree.h
+++ b/src/include/lib/radixtree.h
@@ -2721,12 +2721,12 @@ RT_VERIFY_NODE(RT_NODE * node)
 		case RT_NODE_KIND_256:
 			{
 				RT_NODE_256 *n256 = (RT_NODE_256 *) node;
-				int			cnt = 0;
+				int			cnt;
 
 				/* RT_DUMP_NODE(node); */
 
-				for (int i = 0; i < RT_BM_IDX(RT_NODE_MAX_SLOTS); i++)
-					cnt += bmw_popcount(n256->isset[i]);
+				cnt = pg_popcount((const char *) n256->isset,
+								  RT_NODE_MAX_SLOTS / BITS_PER_BYTE);
 
 				/*
 				 * Check if the number of used chunk matches, accounting for
-- 
2.50.1 (Apple Git-155)


--rlwGoOKBy5dABfM4--





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

* [PATCH v6 1/3] Make use of pg_popcount() in more places.
@ 2026-01-22 17:16 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Nathan Bossart @ 2026-01-22 17:16 UTC (permalink / raw)

---
 src/backend/nodes/bitmapset.c | 27 ++++-----------------------
 src/include/lib/radixtree.h   |  4 ++--
 2 files changed, 6 insertions(+), 25 deletions(-)

diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index a4765876c31..23c91fdb6c9 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -553,14 +553,8 @@ bms_member_index(Bitmapset *a, int x)
 	bitnum = BITNUM(x);
 
 	/* count bits in preceding words */
-	for (int i = 0; i < wordnum; i++)
-	{
-		bitmapword	w = a->words[i];
-
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	}
+	result += pg_popcount((const char *) a->words,
+						  wordnum * sizeof(bitmapword));
 
 	/*
 	 * Now add bits of the last word, but only those before the item. We can
@@ -749,26 +743,13 @@ bms_get_singleton_member(const Bitmapset *a, int *member)
 int
 bms_num_members(const Bitmapset *a)
 {
-	int			result = 0;
-	int			nwords;
-	int			wordnum;
-
 	Assert(bms_is_valid_set(a));
 
 	if (a == NULL)
 		return 0;
 
-	nwords = a->nwords;
-	wordnum = 0;
-	do
-	{
-		bitmapword	w = a->words[wordnum];
-
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	} while (++wordnum < nwords);
-	return result;
+	return pg_popcount((const char *) a->words,
+					   a->nwords * sizeof(bitmapword));
 }
 
 /*
diff --git a/src/include/lib/radixtree.h b/src/include/lib/radixtree.h
index b223ce10a2d..1425654a67c 100644
--- a/src/include/lib/radixtree.h
+++ b/src/include/lib/radixtree.h
@@ -2725,8 +2725,8 @@ RT_VERIFY_NODE(RT_NODE * node)
 
 				/* RT_DUMP_NODE(node); */
 
-				for (int i = 0; i < RT_BM_IDX(RT_NODE_MAX_SLOTS); i++)
-					cnt += bmw_popcount(n256->isset[i]);
+				cnt += pg_popcount((const char *) n256->isset,
+								   RT_NODE_MAX_SLOTS / BITS_PER_BYTE);
 
 				/*
 				 * Check if the number of used chunk matches, accounting for
-- 
2.50.1 (Apple Git-155)


--vgXyZptCXHDBThGI
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v6-0002-Remove-unnecessary-32-bit-optimizations-and-align.patch



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

* [PATCH v3 1/2] Make use of pg_popcount() in more places.
@ 2026-01-22 17:16 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Nathan Bossart @ 2026-01-22 17:16 UTC (permalink / raw)

---
 src/backend/nodes/bitmapset.c | 27 ++++-----------------------
 src/include/lib/radixtree.h   |  4 ++--
 2 files changed, 6 insertions(+), 25 deletions(-)

diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index a4765876c31..23c91fdb6c9 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -553,14 +553,8 @@ bms_member_index(Bitmapset *a, int x)
 	bitnum = BITNUM(x);
 
 	/* count bits in preceding words */
-	for (int i = 0; i < wordnum; i++)
-	{
-		bitmapword	w = a->words[i];
-
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	}
+	result += pg_popcount((const char *) a->words,
+						  wordnum * sizeof(bitmapword));
 
 	/*
 	 * Now add bits of the last word, but only those before the item. We can
@@ -749,26 +743,13 @@ bms_get_singleton_member(const Bitmapset *a, int *member)
 int
 bms_num_members(const Bitmapset *a)
 {
-	int			result = 0;
-	int			nwords;
-	int			wordnum;
-
 	Assert(bms_is_valid_set(a));
 
 	if (a == NULL)
 		return 0;
 
-	nwords = a->nwords;
-	wordnum = 0;
-	do
-	{
-		bitmapword	w = a->words[wordnum];
-
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	} while (++wordnum < nwords);
-	return result;
+	return pg_popcount((const char *) a->words,
+					   a->nwords * sizeof(bitmapword));
 }
 
 /*
diff --git a/src/include/lib/radixtree.h b/src/include/lib/radixtree.h
index b223ce10a2d..1425654a67c 100644
--- a/src/include/lib/radixtree.h
+++ b/src/include/lib/radixtree.h
@@ -2725,8 +2725,8 @@ RT_VERIFY_NODE(RT_NODE * node)
 
 				/* RT_DUMP_NODE(node); */
 
-				for (int i = 0; i < RT_BM_IDX(RT_NODE_MAX_SLOTS); i++)
-					cnt += bmw_popcount(n256->isset[i]);
+				cnt += pg_popcount((const char *) n256->isset,
+								   RT_NODE_MAX_SLOTS / BITS_PER_BYTE);
 
 				/*
 				 * Check if the number of used chunk matches, accounting for
-- 
2.50.1 (Apple Git-155)


--1Cwfhs/B30EIRjWN
Content-Type: text/plain; charset=us-ascii
Content-Disposition: attachment;
	filename=v3-0002-Remove-unnecessary-32-bit-optimizations-and-align.patch



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

* [PATCH v14 2/2] Make use of pg_popcount() in more places.
@ 2026-02-10 18:40 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Nathan Bossart @ 2026-02-10 18:40 UTC (permalink / raw)

This replaces some loops over word-length popcount functions with
calls to pg_popcount().  Since pg_popcount() uses a function
pointer for inputs with sizes >= a Bitmapset word, this produces a
small regression for the common one-word case in bms_num_members().
To deal with that, this commit adds an inlined fast-path for that
case.  This fast-path could arguably go in pg_popcount() itself
(with an appropriate alignment check), but that is left for future
study.

Suggested-by: John Naylor <[email protected]>
Reviewed-by: John Naylor <[email protected]>
Discussion: https://postgr.es/m/CANWCAZY7R%2Biy%2Br9YM_sySNydHzNqUirx1xk0tB3ej5HO62GdgQ%40mail.gmail.com
---
 src/backend/nodes/bitmapset.c | 29 +++++++----------------------
 src/include/lib/radixtree.h   |  6 +++---
 2 files changed, 10 insertions(+), 25 deletions(-)

diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index a4765876c31..786f343b3c9 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -553,14 +553,8 @@ bms_member_index(Bitmapset *a, int x)
 	bitnum = BITNUM(x);
 
 	/* count bits in preceding words */
-	for (int i = 0; i < wordnum; i++)
-	{
-		bitmapword	w = a->words[i];
-
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	}
+	result += pg_popcount((const char *) a->words,
+						  wordnum * sizeof(bitmapword));
 
 	/*
 	 * Now add bits of the last word, but only those before the item. We can
@@ -749,26 +743,17 @@ bms_get_singleton_member(const Bitmapset *a, int *member)
 int
 bms_num_members(const Bitmapset *a)
 {
-	int			result = 0;
-	int			nwords;
-	int			wordnum;
-
 	Assert(bms_is_valid_set(a));
 
 	if (a == NULL)
 		return 0;
 
-	nwords = a->nwords;
-	wordnum = 0;
-	do
-	{
-		bitmapword	w = a->words[wordnum];
+	/* fast-path for common case */
+	if (a->nwords == 1)
+		return bmw_popcount(a->words[0]);
 
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	} while (++wordnum < nwords);
-	return result;
+	return pg_popcount((const char *) a->words,
+					   a->nwords * sizeof(bitmapword));
 }
 
 /*
diff --git a/src/include/lib/radixtree.h b/src/include/lib/radixtree.h
index b223ce10a2d..e6c9a591c17 100644
--- a/src/include/lib/radixtree.h
+++ b/src/include/lib/radixtree.h
@@ -2721,12 +2721,12 @@ RT_VERIFY_NODE(RT_NODE * node)
 		case RT_NODE_KIND_256:
 			{
 				RT_NODE_256 *n256 = (RT_NODE_256 *) node;
-				int			cnt = 0;
+				int			cnt;
 
 				/* RT_DUMP_NODE(node); */
 
-				for (int i = 0; i < RT_BM_IDX(RT_NODE_MAX_SLOTS); i++)
-					cnt += bmw_popcount(n256->isset[i]);
+				cnt = pg_popcount((const char *) n256->isset,
+								  RT_NODE_MAX_SLOTS / BITS_PER_BYTE);
 
 				/*
 				 * Check if the number of used chunk matches, accounting for
-- 
2.50.1 (Apple Git-155)


--rjsRYmGG7r/fFdxn--





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

* [PATCH v16 2/2] Make use of pg_popcount() in more places.
@ 2026-02-10 18:40 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Nathan Bossart @ 2026-02-10 18:40 UTC (permalink / raw)

This replaces some loops over word-length popcount functions with
calls to pg_popcount().  Since pg_popcount() uses a function
pointer for inputs with sizes >= a Bitmapset word, this produces a
small regression for the common one-word case in bms_num_members().
To deal with that, this commit adds an inlined fast-path for that
case.  This fast-path could arguably go in pg_popcount() itself
(with an appropriate alignment check), but that is left for future
study.

Suggested-by: John Naylor <[email protected]>
Reviewed-by: John Naylor <[email protected]>
Discussion: https://postgr.es/m/CANWCAZY7R%2Biy%2Br9YM_sySNydHzNqUirx1xk0tB3ej5HO62GdgQ%40mail.gmail.com
---
 src/backend/nodes/bitmapset.c | 29 +++++++----------------------
 src/include/lib/radixtree.h   |  6 +++---
 2 files changed, 10 insertions(+), 25 deletions(-)

diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index a4765876c31..786f343b3c9 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -553,14 +553,8 @@ bms_member_index(Bitmapset *a, int x)
 	bitnum = BITNUM(x);
 
 	/* count bits in preceding words */
-	for (int i = 0; i < wordnum; i++)
-	{
-		bitmapword	w = a->words[i];
-
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	}
+	result += pg_popcount((const char *) a->words,
+						  wordnum * sizeof(bitmapword));
 
 	/*
 	 * Now add bits of the last word, but only those before the item. We can
@@ -749,26 +743,17 @@ bms_get_singleton_member(const Bitmapset *a, int *member)
 int
 bms_num_members(const Bitmapset *a)
 {
-	int			result = 0;
-	int			nwords;
-	int			wordnum;
-
 	Assert(bms_is_valid_set(a));
 
 	if (a == NULL)
 		return 0;
 
-	nwords = a->nwords;
-	wordnum = 0;
-	do
-	{
-		bitmapword	w = a->words[wordnum];
+	/* fast-path for common case */
+	if (a->nwords == 1)
+		return bmw_popcount(a->words[0]);
 
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	} while (++wordnum < nwords);
-	return result;
+	return pg_popcount((const char *) a->words,
+					   a->nwords * sizeof(bitmapword));
 }
 
 /*
diff --git a/src/include/lib/radixtree.h b/src/include/lib/radixtree.h
index b223ce10a2d..e6c9a591c17 100644
--- a/src/include/lib/radixtree.h
+++ b/src/include/lib/radixtree.h
@@ -2721,12 +2721,12 @@ RT_VERIFY_NODE(RT_NODE * node)
 		case RT_NODE_KIND_256:
 			{
 				RT_NODE_256 *n256 = (RT_NODE_256 *) node;
-				int			cnt = 0;
+				int			cnt;
 
 				/* RT_DUMP_NODE(node); */
 
-				for (int i = 0; i < RT_BM_IDX(RT_NODE_MAX_SLOTS); i++)
-					cnt += bmw_popcount(n256->isset[i]);
+				cnt = pg_popcount((const char *) n256->isset,
+								  RT_NODE_MAX_SLOTS / BITS_PER_BYTE);
 
 				/*
 				 * Check if the number of used chunk matches, accounting for
-- 
2.50.1 (Apple Git-155)


--/1paUjurV8mGDyTY--





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

* [PATCH v12 4/4] Make use of pg_popcount() in more places.
@ 2026-02-10 18:40 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Nathan Bossart @ 2026-02-10 18:40 UTC (permalink / raw)

This replaces some loops over word-length popcount functions with
calls to pg_popcount().  Since pg_popcount() uses a function
pointer for inputs with sizes >= a Bitmapset word, this produces a
small regression for the common one-word case in bms_num_members().
To deal with that, this commit adds an inlined fast-path for that
case.  This fast-path could arguably go in pg_popcount() itself
(with an appropriate alignment check), but that is left for future
study.

Suggested-by: John Naylor <[email protected]>
Reviewed-by: John Naylor <[email protected]>
Discussion: https://postgr.es/m/CANWCAZY7R%2Biy%2Br9YM_sySNydHzNqUirx1xk0tB3ej5HO62GdgQ%40mail.gmail.com
---
 src/backend/nodes/bitmapset.c | 29 +++++++----------------------
 src/include/lib/radixtree.h   |  6 +++---
 2 files changed, 10 insertions(+), 25 deletions(-)

diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index a4765876c31..786f343b3c9 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -553,14 +553,8 @@ bms_member_index(Bitmapset *a, int x)
 	bitnum = BITNUM(x);
 
 	/* count bits in preceding words */
-	for (int i = 0; i < wordnum; i++)
-	{
-		bitmapword	w = a->words[i];
-
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	}
+	result += pg_popcount((const char *) a->words,
+						  wordnum * sizeof(bitmapword));
 
 	/*
 	 * Now add bits of the last word, but only those before the item. We can
@@ -749,26 +743,17 @@ bms_get_singleton_member(const Bitmapset *a, int *member)
 int
 bms_num_members(const Bitmapset *a)
 {
-	int			result = 0;
-	int			nwords;
-	int			wordnum;
-
 	Assert(bms_is_valid_set(a));
 
 	if (a == NULL)
 		return 0;
 
-	nwords = a->nwords;
-	wordnum = 0;
-	do
-	{
-		bitmapword	w = a->words[wordnum];
+	/* fast-path for common case */
+	if (a->nwords == 1)
+		return bmw_popcount(a->words[0]);
 
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	} while (++wordnum < nwords);
-	return result;
+	return pg_popcount((const char *) a->words,
+					   a->nwords * sizeof(bitmapword));
 }
 
 /*
diff --git a/src/include/lib/radixtree.h b/src/include/lib/radixtree.h
index b223ce10a2d..e6c9a591c17 100644
--- a/src/include/lib/radixtree.h
+++ b/src/include/lib/radixtree.h
@@ -2721,12 +2721,12 @@ RT_VERIFY_NODE(RT_NODE * node)
 		case RT_NODE_KIND_256:
 			{
 				RT_NODE_256 *n256 = (RT_NODE_256 *) node;
-				int			cnt = 0;
+				int			cnt;
 
 				/* RT_DUMP_NODE(node); */
 
-				for (int i = 0; i < RT_BM_IDX(RT_NODE_MAX_SLOTS); i++)
-					cnt += bmw_popcount(n256->isset[i]);
+				cnt = pg_popcount((const char *) n256->isset,
+								  RT_NODE_MAX_SLOTS / BITS_PER_BYTE);
 
 				/*
 				 * Check if the number of used chunk matches, accounting for
-- 
2.50.1 (Apple Git-155)


--AFLuD1w+kMRlv6Zk--





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

* [PATCH v13 5/5] Make use of pg_popcount() in more places.
@ 2026-02-10 18:40 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Nathan Bossart @ 2026-02-10 18:40 UTC (permalink / raw)

This replaces some loops over word-length popcount functions with
calls to pg_popcount().  Since pg_popcount() uses a function
pointer for inputs with sizes >= a Bitmapset word, this produces a
small regression for the common one-word case in bms_num_members().
To deal with that, this commit adds an inlined fast-path for that
case.  This fast-path could arguably go in pg_popcount() itself
(with an appropriate alignment check), but that is left for future
study.

Suggested-by: John Naylor <[email protected]>
Reviewed-by: John Naylor <[email protected]>
Discussion: https://postgr.es/m/CANWCAZY7R%2Biy%2Br9YM_sySNydHzNqUirx1xk0tB3ej5HO62GdgQ%40mail.gmail.com
---
 src/backend/nodes/bitmapset.c | 29 +++++++----------------------
 src/include/lib/radixtree.h   |  6 +++---
 2 files changed, 10 insertions(+), 25 deletions(-)

diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index a4765876c31..786f343b3c9 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -553,14 +553,8 @@ bms_member_index(Bitmapset *a, int x)
 	bitnum = BITNUM(x);
 
 	/* count bits in preceding words */
-	for (int i = 0; i < wordnum; i++)
-	{
-		bitmapword	w = a->words[i];
-
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	}
+	result += pg_popcount((const char *) a->words,
+						  wordnum * sizeof(bitmapword));
 
 	/*
 	 * Now add bits of the last word, but only those before the item. We can
@@ -749,26 +743,17 @@ bms_get_singleton_member(const Bitmapset *a, int *member)
 int
 bms_num_members(const Bitmapset *a)
 {
-	int			result = 0;
-	int			nwords;
-	int			wordnum;
-
 	Assert(bms_is_valid_set(a));
 
 	if (a == NULL)
 		return 0;
 
-	nwords = a->nwords;
-	wordnum = 0;
-	do
-	{
-		bitmapword	w = a->words[wordnum];
+	/* fast-path for common case */
+	if (a->nwords == 1)
+		return bmw_popcount(a->words[0]);
 
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	} while (++wordnum < nwords);
-	return result;
+	return pg_popcount((const char *) a->words,
+					   a->nwords * sizeof(bitmapword));
 }
 
 /*
diff --git a/src/include/lib/radixtree.h b/src/include/lib/radixtree.h
index b223ce10a2d..e6c9a591c17 100644
--- a/src/include/lib/radixtree.h
+++ b/src/include/lib/radixtree.h
@@ -2721,12 +2721,12 @@ RT_VERIFY_NODE(RT_NODE * node)
 		case RT_NODE_KIND_256:
 			{
 				RT_NODE_256 *n256 = (RT_NODE_256 *) node;
-				int			cnt = 0;
+				int			cnt;
 
 				/* RT_DUMP_NODE(node); */
 
-				for (int i = 0; i < RT_BM_IDX(RT_NODE_MAX_SLOTS); i++)
-					cnt += bmw_popcount(n256->isset[i]);
+				cnt = pg_popcount((const char *) n256->isset,
+								  RT_NODE_MAX_SLOTS / BITS_PER_BYTE);
 
 				/*
 				 * Check if the number of used chunk matches, accounting for
-- 
2.50.1 (Apple Git-155)


--5NNwvjHYUnRaoEDo--





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

* [PATCH v15 2/2] Make use of pg_popcount() in more places.
@ 2026-02-10 18:40 Nathan Bossart <[email protected]>
  0 siblings, 0 replies; 14+ messages in thread

From: Nathan Bossart @ 2026-02-10 18:40 UTC (permalink / raw)

This replaces some loops over word-length popcount functions with
calls to pg_popcount().  Since pg_popcount() uses a function
pointer for inputs with sizes >= a Bitmapset word, this produces a
small regression for the common one-word case in bms_num_members().
To deal with that, this commit adds an inlined fast-path for that
case.  This fast-path could arguably go in pg_popcount() itself
(with an appropriate alignment check), but that is left for future
study.

Suggested-by: John Naylor <[email protected]>
Reviewed-by: John Naylor <[email protected]>
Discussion: https://postgr.es/m/CANWCAZY7R%2Biy%2Br9YM_sySNydHzNqUirx1xk0tB3ej5HO62GdgQ%40mail.gmail.com
---
 src/backend/nodes/bitmapset.c | 29 +++++++----------------------
 src/include/lib/radixtree.h   |  6 +++---
 2 files changed, 10 insertions(+), 25 deletions(-)

diff --git a/src/backend/nodes/bitmapset.c b/src/backend/nodes/bitmapset.c
index a4765876c31..786f343b3c9 100644
--- a/src/backend/nodes/bitmapset.c
+++ b/src/backend/nodes/bitmapset.c
@@ -553,14 +553,8 @@ bms_member_index(Bitmapset *a, int x)
 	bitnum = BITNUM(x);
 
 	/* count bits in preceding words */
-	for (int i = 0; i < wordnum; i++)
-	{
-		bitmapword	w = a->words[i];
-
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	}
+	result += pg_popcount((const char *) a->words,
+						  wordnum * sizeof(bitmapword));
 
 	/*
 	 * Now add bits of the last word, but only those before the item. We can
@@ -749,26 +743,17 @@ bms_get_singleton_member(const Bitmapset *a, int *member)
 int
 bms_num_members(const Bitmapset *a)
 {
-	int			result = 0;
-	int			nwords;
-	int			wordnum;
-
 	Assert(bms_is_valid_set(a));
 
 	if (a == NULL)
 		return 0;
 
-	nwords = a->nwords;
-	wordnum = 0;
-	do
-	{
-		bitmapword	w = a->words[wordnum];
+	/* fast-path for common case */
+	if (a->nwords == 1)
+		return bmw_popcount(a->words[0]);
 
-		/* No need to count the bits in a zero word */
-		if (w != 0)
-			result += bmw_popcount(w);
-	} while (++wordnum < nwords);
-	return result;
+	return pg_popcount((const char *) a->words,
+					   a->nwords * sizeof(bitmapword));
 }
 
 /*
diff --git a/src/include/lib/radixtree.h b/src/include/lib/radixtree.h
index b223ce10a2d..e6c9a591c17 100644
--- a/src/include/lib/radixtree.h
+++ b/src/include/lib/radixtree.h
@@ -2721,12 +2721,12 @@ RT_VERIFY_NODE(RT_NODE * node)
 		case RT_NODE_KIND_256:
 			{
 				RT_NODE_256 *n256 = (RT_NODE_256 *) node;
-				int			cnt = 0;
+				int			cnt;
 
 				/* RT_DUMP_NODE(node); */
 
-				for (int i = 0; i < RT_BM_IDX(RT_NODE_MAX_SLOTS); i++)
-					cnt += bmw_popcount(n256->isset[i]);
+				cnt = pg_popcount((const char *) n256->isset,
+								  RT_NODE_MAX_SLOTS / BITS_PER_BYTE);
 
 				/*
 				 * Check if the number of used chunk matches, accounting for
-- 
2.50.1 (Apple Git-155)


--PPBl4GgF05zqfOFU--





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

end of thread, other threads:[~2026-02-10 18:40 UTC | newest]

Thread overview: 14+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2020-02-29 01:20 [PATCH v11 4/5] fixes Justin Pryzby <[email protected]>
2026-01-22 17:16 [PATCH v8 3/3] Make use of pg_popcount() in more places. Nathan Bossart <[email protected]>
2026-01-22 17:16 [PATCH v4 1/4] Make use of pg_popcount() in more places. Nathan Bossart <[email protected]>
2026-01-22 17:16 [PATCH v11 3/4] Make use of pg_popcount() in more places. Nathan Bossart <[email protected]>
2026-01-22 17:16 [PATCH v5 1/3] Make use of pg_popcount() in more places. Nathan Bossart <[email protected]>
2026-01-22 17:16 [PATCH v9 3/3] Make use of pg_popcount() in more places. Nathan Bossart <[email protected]>
2026-01-22 17:16 [PATCH v10 3/3] Make use of pg_popcount() in more places. Nathan Bossart <[email protected]>
2026-01-22 17:16 [PATCH v6 1/3] Make use of pg_popcount() in more places. Nathan Bossart <[email protected]>
2026-01-22 17:16 [PATCH v3 1/2] Make use of pg_popcount() in more places. Nathan Bossart <[email protected]>
2026-02-10 18:40 [PATCH v14 2/2] Make use of pg_popcount() in more places. Nathan Bossart <[email protected]>
2026-02-10 18:40 [PATCH v16 2/2] Make use of pg_popcount() in more places. Nathan Bossart <[email protected]>
2026-02-10 18:40 [PATCH v12 4/4] Make use of pg_popcount() in more places. Nathan Bossart <[email protected]>
2026-02-10 18:40 [PATCH v13 5/5] Make use of pg_popcount() in more places. Nathan Bossart <[email protected]>
2026-02-10 18:40 [PATCH v15 2/2] Make use of pg_popcount() in more places. 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