Hi, On 2021-08-02 12:57:36 -0400, Tom Lane wrote: > Andres Freund <and...@anarazel.de> writes: > > I think there's quite a few different issues around this - here I'm just > > trying to tackle a few of the most glaring (to me): > > No opinion yet about most of this, but I did want to react to > > > In fact, I think there's a good argument to be made that we should > > entirely remove the concept of BackendIds and just use pgprocnos. We > > have a fair number of places like SignalVirtualTransaction() that need > > to search the procarray just to find the proc to signal based on the > > BackendId. If we used pgprocno instead, that'd not be needed. > > If I understand what you're suggesting, it'd result in unused slots > in sinvaladt.c's procState[] array, which could create an annoying > drag on performance.
Yea, I was looking into that, which is why I don't yet have a patch. I think it might be reasonable to address this by making pgprocnos more dense. We right now actually have a kind of maximally bad allocation pattern for pgprocnos - InitProcGlobal puts them onto the free lists in reverse order. Which means that ProcArrayAdd() will have to move all procs... But, as you say: > However, I think it might be reasonable to switch other things over to > using pgprocnos, with an eye to eventually making BackendIds private > to the sinval mechanism. There's certainly no strong reason why > sinval's array indexes need to be used as identifiers by other > modules. I think it'd entirely be reasonable to switch over at least backend_status.c, procsignal.c to pgprocnos without doing anything about the density of allocation. We'd likely would want to do that as independent steps anyway, even if we were to switch over sinval as well. Another approach to deal with this could be to simply not do the O(n) work in SIInsertDataEntries(). It's not obvious that ->hasMessages is actually necessary - we could atomically read maxMsgNum without acquiring a lock instead of needing the per-backend ->hasMessages. I don't the density would be a relevant factor in SICleanupQueue(). Greetings, Andres Freund
