From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | Andres Freund <andres(at)2ndquadrant(dot)com> |
Cc: | pgsql-hackers(at)postgreSQL(dot)org |
Subject: | Re: ilist.h is not useful as-is |
Date: | 2013-07-24 21:50:52 |
Message-ID: | 5558.1374702652@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Andres Freund <andres(at)2ndquadrant(dot)com> writes:
> The first attached patch adds slist_delete_current(), updates the
> comments addressing your points and converts the bgworker code to pass
> the iterator around (it's more efficient which might actually matter
> with a few hundred bgworkers).
Applied with fixes. The slist_delete_current() logic wouldn't actually
work for deleting multiple adjacent list elements, because the foreach
macro would advance prev to point at the just-deleted element. Compare
the places where we use list_delete_cell() in a loop --- we're careful
to advance prev only when we *don't* delete the current element.
I dealt with that by making slist_delete_current() back up the
iterator's "cur" pointer to equal "prev", so that the loop macro's next
copying of "cur" to "prev" is a no-op. This means callers can't rely on
"cur" anymore after the deletion, but in typical usage they'd have
computed a pointer to their struct already so this shouldn't be a
problem. Another fine point is you can't use both slist_delete and
slist_delete_current within the same loop, since the former wouldn't
apply this correction.
Also, I fixed the slist_foreach_modify macro to not doubly evaluate
its lhead argument, since I think we agreed that was a bad idea,
and did some more work on the comments.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Michael Paquier | 2013-07-24 23:02:11 | Re: dynamic background workers, round two |
Previous Message | Andrew Gierth | 2013-07-24 21:38:15 | Re: Review: UNNEST (and other functions) WITH ORDINALITY |