On Thu, Jul 23, 2015 at 3:01 PM, Ildus Kurbangaliev <i.kurbangal...@postgrespro.ru> wrote: > Hello. > I’ve changed the previous patch. `group` field in LWLock is removed, so the > size of LWLock will not increase. > Instead of the `group` field I've created new tranches for LWLocks from > MainLWLocksArray. This allowed to remove a loop > from the previous version of the patch. > Now the names for LWLocks that are not individual is obtained from > corresponding tranches.
I think using tranches for this is good, but I'm not really keen on using two bytes for this. I think using one byte is just fine. It's OK to assume that anything up to a 4-byte word can be read atomically if it's read using a read of the same width that was used to write it. But it's not OK to assume that if somebody might do, say, a memcpy of a structure containing that 4-byte word, as pgstat_read_current_status does. That, I think, might get torn, because it's reading using 1-byte reads. You'll notice that pgstat_beshutdown_hook() uses the changecount protocol even though it's only changing a 4-byte word. There's no real need for two bytes here, so let's not do that. Just use offsets intelligently to pack it into a single byte. Also, the patch should not invent a new array similar but not quite identical to LockTagTypeNames[]. This is goofy: + if (tranche_id > 0) + result->tranche = tranche_id; + else + result->tranche = LW_USERDEFINED; If we're going to make everybody pass a tranche ID when they call LWLockAssign(), then they should have to do that always, not sometimes pass a tranche ID and otherwise pass 0, which is actually a valid tranche ID but to which you've given a weird special meaning here. I'd also name the constants differently, like LWLOCK_TRANCHE_XXX or something. LW_ is a somewhat ambiguous prefix. The idea of tranches is really that each tranche is an array of items each of which contains 1 or more lwlocks. Here you are intermingling different tranches. I guess that won't really break anything but it seems ugly. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
