Hi, On 2022-04-04 19:03:13 -0700, David G. Johnston wrote: > > > (if this is true...but given this is an optimization category I'm > > thinking > > > maybe it doesn't actually matter...) > > > > It is true. Not sure what you mean with "optimization category"? > > > > > I mean that distinguishing between stats that are fixed and those that are > variable implies that fixed kinds have a better performance (speed, memory) > characteristic than variable kinds (at least in part due to the presence of > changecount). If fixed kinds did not have a performance benefit then > having the variable kind implementation simply handle fixed kinds as well > (using the common struct header and storage in a hash table) would make the > implementation simpler since all statistics would report through the same > API.
Yes, fixed-numbered stats are faster. > Coming back to this: > """ > + /* cluster-scoped object stats having a variable number of entries */ > + PGSTAT_KIND_REPLSLOT = 1, /* per-slot statistics */ > + PGSTAT_KIND_SUBSCRIPTION, /* per-subscription statistics */ > + PGSTAT_KIND_DATABASE, /* database-wide statistics */ (I moved this to 3rd > spot to be closer to the database-scoped options) > + > + /* database-scoped object stats having a variable number of entries */ > + PGSTAT_KIND_RELATION, /* per-table statistics */ > + PGSTAT_KIND_FUNCTION, /* per-function statistics */ > + > + /* cluster-scoped stats having a fixed number of entries */ (maybe these > should go first, the variable following?) > + PGSTAT_KIND_ARCHIVER, > + PGSTAT_KIND_BGWRITER, > + PGSTAT_KIND_CHECKPOINTER, > + PGSTAT_KIND_SLRU, > + PGSTAT_KIND_WAL, > """ > > I see three "KIND_GROUP" categories here: > PGSTAT_KIND_CLUSTER (open to a different word here though...) > PGSTAT_KIND_DATABASE (we seem to agree on this above) > PGSTAT_KIND_GLOBAL (already used in the code) > > This single enum can replace the two booleans that, in combination, would > define 4 unique groups (of which only three are interesting - > database+fixed doesn't seem interesting and so is not given a name/value > here). The more I think about it, the less I think a split like that makes sense. The difference between PGSTAT_KIND_CLUSTER / PGSTAT_KIND_DATABASE is tiny. Nearly all code just deals with both together. I think all this is going to achieve is to making code more complicated. There is a *single* non-assert use of accessed_across_databases and now a single assertion involving it. What would having PGSTAT_KIND_CLUSTER and PGSTAT_KIND_DATABASE achieve? Greetings, Andres Freund
