Re: inefficient loop in StandbyReleaseLockList()
От | Kyotaro Horiguchi |
---|---|
Тема | Re: inefficient loop in StandbyReleaseLockList() |
Дата | |
Msg-id | 20211101.114019.1470121542110379535.horikyota.ntt@gmail.com обсуждение исходный текст |
Ответ на | Re: inefficient loop in StandbyReleaseLockList() (Tom Lane <tgl@sss.pgh.pa.us>) |
Ответы |
Re: inefficient loop in StandbyReleaseLockList()
(Tom Lane <tgl@sss.pgh.pa.us>)
|
Список | pgsql-hackers |
At Sun, 31 Oct 2021 16:55:01 -0400, Tom Lane <tgl@sss.pgh.pa.us> wrote in > 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. I should be missing something, but at the added list_free() there's a case where keysQueue has some elelments. I think the remaining elements are useless but AFAICS all the memory allocated there is freed after createTrgmNFAInternal returnes, before the "next cycle" comes. Do we need to add that list_free there? > 2. I think we almost certainly have a problem in SyncPostCheckpoint. Maybe we want list_delete_first_n() or such to remove the first n elements in a list at once. > 3. Is agg_refill_hash_table a problem? Probably; we've seen cases > with lots of batches. I excluded it since I'm not sure it is in the pattern at a glance. I would want to leave it alone, since changing the logic there seems making things a bit complex and the gain by removing list_delete_first doesn't look so large.. > 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?). From the other side, the elemnts are inserted by lcons, then removed by list_delete_first. It is the worst usage of the current list implementation as a FIFO. Couldn't we construct and iterate over a list in the reverse order? > 5. Not sure about CopyMultiInsertInfoFlush ... how many buffers > could we have there? (I'm not sure..) > 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. Agreed. > 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. Agreed. > 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. If we didn't need the resulting list I'm for changing it but actually it is needed. So I think we won't get so much by changing the function. regards. -- Kyotaro Horiguchi NTT Open Source Software Center
В списке pgsql-hackers по дате отправления:
Следующее
От: Michael PaquierДата:
Сообщение: Re: Opclass parameters of indexes lost after REINDEX CONCURRENTLY[