Thread: Replace PROC_QUEUE / SHM_QUEUE with ilist.h
Hi, In [1] Robert justifiably complained about the use of PROC_QUEUE. I've previously been bothered by this in [2], but didn't get around to finishing the patches. One thing that made me hesitate was the naming of the function for a) checking whether a dlist_node is a list, b) initializing and deleting nodes in a way to allow for a). My patch adds dlist_node_init(), dlist_delete_thoroughly() / dlist_delete_from_thoroughly() / dclist_delete_from_thoroughly() and dlist_node_is_detached(). Thomas proposed dlist_delete_and_reinit() and dlist_node_is_linked() instead. Attached is a revised version of the patches from [2]. I left the naming of the detached / thoroughly as it was before, for now. Another alternative could be to try to just get rid of needing the detached state at all, although that likely would make the code changes bigger. I've switched the PROC_QUEUE uses to dclist, which we only got recently. The prior version of the patchset contained a patch to remove the use of the size field of PROC_QUEUE, as it's only needed in a few places. But it seems easier to just replace it with dclist for now. Robert had previously complained about the ilist.h patch constifying some functions. I don't really understand the complaint in this case - none of the cases should require constifying outside code. It just allows to replace things like SHMQueueEmpty() which were const, because there's a few places that get passed a const PGPROC. There's more that could be constified (removing the need for one unconstify() in predicate.c - but that seems like a lot more work with less clear benefit. Either way, I split the constification into a separate patch. Comments? Greetings, Andres Freund [1] https://www.postgresql.org/message-id/20221117201304.vemjfsxaizmt3zbb%40awork3.anarazel.de [2] https://www.postgresql.org/message-id/20200211042229.msv23badgqljrdg2%40alap3.anarazel.de
Attachment
Hi, On 2022-11-19 21:59:30 -0800, Andres Freund wrote: > In [1] Robert justifiably complained about the use of PROC_QUEUE. I've > previously been bothered by this in [2], but didn't get around to finishing > the patches. > > One thing that made me hesitate was the naming of the function for a) checking > whether a dlist_node is a list, b) initializing and deleting nodes in a way to > allow for a). My patch adds dlist_node_init(), dlist_delete_thoroughly() / > dlist_delete_from_thoroughly() / dclist_delete_from_thoroughly() and > dlist_node_is_detached(). Thomas proposed dlist_delete_and_reinit() and > dlist_node_is_linked() instead. Any comments on these names? Otherwise I'm planning to push ahead with the names as is, lest I forget this patchset for another ~2 years. Greetings, Andres Freund
Hi, On 2022-12-03 10:17:22 -0800, Andres Freund wrote: > On 2022-11-19 21:59:30 -0800, Andres Freund wrote: > > In [1] Robert justifiably complained about the use of PROC_QUEUE. I've > > previously been bothered by this in [2], but didn't get around to finishing > > the patches. > > > > One thing that made me hesitate was the naming of the function for a) checking > > whether a dlist_node is a list, b) initializing and deleting nodes in a way to > > allow for a). My patch adds dlist_node_init(), dlist_delete_thoroughly() / > > dlist_delete_from_thoroughly() / dclist_delete_from_thoroughly() and > > dlist_node_is_detached(). Thomas proposed dlist_delete_and_reinit() and > > dlist_node_is_linked() instead. > > Any comments on these names? Otherwise I'm planning to push ahead with the > names as is, lest I forget this patchset for another ~2 years. Finally pushed, with some fairly minor additional cleanup. No more SHM_QUEUE, yay! Also, predicate.c really needs some love. Greetings, Andres Freund
On Thu, Jan 19, 2023 at 9:58 PM Andres Freund <andres@anarazel.de> wrote: > Finally pushed, with some fairly minor additional cleanup. No more SHM_QUEUE, > yay! Nice. I won't miss it one bit. -- Robert Haas EDB: http://www.enterprisedb.com