Isn't it necessary to have an interface to initialize LWLock structure being
allocated on a dynamic shared memory segment?
Even though LWLock structure is exposed at lwlock.h, we have no common
way to initialize it.
How about to have a following function?
void
InitLWLock(LWLock *lock)
{
SpinLockInit(&lock->lock.mutex);
lock->lock.releaseOK = true;
lock->lock.exclusive = 0;
lock->lock.shared = 0;
lock->lock.head = NULL;
lock->lock.tail = NULL;
}
Thanks,
2014/1/22 KaiGai Kohei <kai...@ak.jp.nec.com>:
> (2014/01/22 1:37), Robert Haas wrote:
>>
>> On Mon, Jan 20, 2014 at 11:23 PM, KaiGai Kohei <kai...@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 <kai...@ak.jp.nec.com>
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
--
KaiGai Kohei <kai...@kaigai.gr.jp>
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
