On Mon, Jun 4, 2012 at 9:20 AM, Ants Aasma <a...@cybertec.at> wrote: > On Mon, Jun 4, 2012 at 5:12 PM, Robert Haas <robertmh...@gmail.com> wrote: >> Note sure about the rest of this patch, but this part is definitely bogus: >> >> +#if !defined(pg_atomic_fetch_and_set) >> +#define pg_atomic_fetch_and_set(dst, src, value) \ >> + do { S_LOCK(&dummy_spinlock); \ >> + dst = src; \ >> + src = value; \ >> + S_UNLOCK(&dummy_spinlock); } while (0) >> +#endif >> >> Locking a dummy backend-local spinlock doesn't provide atomicity >> across multiple processes. > > Right, of course. I don't know why I assumed that dummy_spinlock would > be global. In any case, this is very WIP and doesn't even aspire to be > portable yet. The main point was to see if there's any significant > performance to be gained by this method.
yeah -- those are fallback routines in case the compiler primitives don't exist. I think I understand what Ants is doing here: he's reducing the coverage of the free list lock to only cover actually popping a buffer off the free list; it no longer covers the clock sweep. That's a massive win if it works. In order to get away with that he had to decompose all manipulations from the clock sweep to the Strategy to thread safe atomic operations. What happens (in the very unlikely, but possible case?) if another backend races to the buffer you've pointed at with 'victim'? It looks like multiple backends share the clock sweep now, but don't you need to need an extra test to ensure it's still a candidate victim buffer? merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers