Re: inefficient loop in StandbyReleaseLockList()
Tom Lane <tgl@sss.pgh.pa.us>
From: Tom Lane <tgl@sss.pgh.pa.us>
To: "Bossart, Nathan" <bossartn@amazon.com>
Cc: Michael Paquier <michael@paquier.xyz>, Andres Freund <andres@anarazel.de>,
Kyotaro Horiguchi <horikyota.ntt@gmail.com>,
"sulamul@gmail.com" <sulamul@gmail.com>,
"pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2021-10-31T20:55:01Z
Lists: pgsql-hackers
Attachments
- avoid-list-delete-first-in-pg-trgm.patch (text/x-diff) patch
I wrote: > Pushed the patch as given. I've not yet reviewed other list_delete_first > callers, but I'll take a look. (I seem to remember that I did survey > them while writing 1cff1b95a, but I evidently missed that this code > could be dealing with a list long enough to be problematic.) I looked at the remaining list_delete_first callers. 1. Attached is a proposed patch to get rid of the calls in trgm_regexp.c. I'm not certain that those lists could get long enough to be a problem, given the existing complexity limits in that file (MAX_EXPANDED_STATES etc). But I'm not certain they can't, either, and it's easy enough to fix along the same lines as in StandbyReleaseLockList. 2. I think we almost certainly have a problem in SyncPostCheckpoint. 3. Is agg_refill_hash_table a problem? Probably; we've seen cases with lots of batches. 4. I'm a bit worried about the uses in access/gist/, but I don't know that code well enough to want to mess with it. It's possible the list lengths are bounded by the index tree height, in which case it likely doesn't matter. The logic in gistFindPath looks like a mess anyway since it's appending to both ends of the "fifo" list in different places (is that really necessary?). 5. Not sure about CopyMultiInsertInfoFlush ... how many buffers could we have there? 6. llvm_release_context may not have a long enough list to be a problem, but on the other hand, it looks easy to fix. 7. The list lengths in the parser and dependency.c, ruleutils.c, explain.c are bounded by subquery nesting depth or plan tree depth, so I doubt it's worth worrying about. 8. The uses in namespace.c don't seem like an issue either -- for instance, GetOverrideSearchPath can't iterate more than twice, and the overrideStack list shouldn't get very deep. regards, tom lane
Commits
-
Doc: add some notes about performance of the List functions.
- 27ef132a805c 15.0 landed
-
Avoid O(N^2) behavior in SyncPostCheckpoint().
- 65c6cab1365a 15.0 landed
- 08cfa5981e17 14.1 landed
- 0151af40cd4e 13.5 landed
-
Avoid some other O(N^2) hazards in list manipulation.
- e477642a1ba8 13.5 landed
- ad87bf355214 14.1 landed
- e9d9ba2a4ddc 15.0 landed
-
Avoid O(N^2) behavior when the standby process releases many locks.
- df238aed1090 13.5 landed
- 8424dfced790 14.1 landed
- 6301c3adabd9 15.0 landed