Re: dynamic shared memory and locks

(2014/01/22 1:37), Robert Haas wrote:
> On Mon, Jan 20, 2014 at 11:23 PM, KaiGai Kohei <kaigai@ak.jp.nec.com> wrote:
>> I briefly checked the patch. Most of lines are mechanical replacement
>> from LWLockId to LWLock *, and compiler didn't claim anything with
>> -Wall -Werror option.
>>
>> My concern is around LWLockTranche mechanism. Isn't it too complicated
>> towards the purpose?
>> For example, it may make sense to add "char lockname[NAMEDATALEN];" at
>> the tail of LWLock structure if LOCK_DEBUG is enabled. Probably, it
>> also adds an argument of LWLockAssign() to gives the human readable
>> name. Is the additional 64bytes (= NAMEDATALEN) per lock too large
>> for recent hardware?
>
> Well, we'd need it when either LOCK_DEBUG was defined or when
> LWLOCK_STATS was defined or when --enable-dtrace was used, and while
> the first two are probably rarely enough used that that would be OK, I
> think the third case is probably fairly common, and I don't think we
> want to have such a potentially performance-critical difference
> between builds with and without dtrace.
>
> Also... yeah, it's a lot of memory.  If we add an additional 64 bytes
> to the structure, then we're looking at 96 bytes per lwlock instead of
> 32, after padding out to a 32-byte boundary to avoid crossing cache
> lines.  We need 2 lwlocks per buffer, so that's an additional 128
> bytes per 8kB buffer.  For shared_buffers = 8GB, that's an extra 128MB
> for storing lwlocks.  I'm not willing to assume nobody cares about
> that.  And while I agree that this is a bit complex, I don't think
> it's really as bad as all that.  We've gotten by for a long time
> without people being able to put lwlocks in other parts of memory, and
> most extension authors have gotten by with LWLockAssign() just fine
> and can continue doing things that way; only users with particularly
> sophisticated needs should bother to worry about the tranche stuff.
>
Hmm... 1/64 of main memory (if large buffer system) might not be
an ignorable memory consumption.

> One idea I just had is to improve the dsm_toc module so that it can
> optionally set up a tranche of lwlocks for you, and provide some
> analogues of RequestAddinLWLocks and LWLockAssign for that case.  That
> would probably make this quite a bit simpler to use, at least for
> people using it with dynamic shared memory.  But I think that's a
> separate patch.
>
I agree with this idea. It seems to me quite natural to keep properties
of objects held on shared memory (LWLock) on shared memory.
Also, a LWLock once assigned shall not be never released. So, I think
dsm_toc can provide a well suitable storage for them.

Thanks,
-- 
OSS Promotion Center / The PG-Strom Project
KaiGai Kohei <kaigai@ak.jp.nec.com>