On 1/6/14, 2:59 PM, Robert Haas wrote:
> On Mon, Jan 6, 2014 at 3:57 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Robert Haas <robertmhaas@gmail.com> writes:
>>> On Mon, Jan 6, 2014 at 3:32 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>> I agree it'd be nicer if we had some better way than mere manual
>>>> inspection to enforce proper use of spinlocks; but this change doesn't
>>>> seem to me to move the ball downfield by any meaningful distance.
>>
>>> Well, my thought was mainly that, while it may be a bad idea to take
>>> another spinlock while holding a spinlock under any circumstances,
>>> somebody might do it and it might appear to work just fine. The most
>>> likely sequences seems to me to be something like SpinLockAcquire(...)
>>> followed by LWLockConditionalAcquire(), thinking that things are OK
>>> because the lock acquisition is conditional - but in fact the
>>> conditional acquire still takes the spinlock unconditionally.
>>
>> The point I'm making is that no such code should get past review,
>> whether it's got an obvious performance problem or not.
>
> Sure, I agree, but we all make mistakes. It's just a judgement call
> as to how likely you think it is that someone might make this
> particular mistake, a topic upon which opinions may vary.
There's been discussions in the past about the overhead added by testing and having more than one level of test
capabilitiesso that facilities like the buildfarm can run more expensive testing without burdening developers with
that.ISTM this is another case of that (presumably Tom's concern is the performance hit of adding an assert in a
criticalpath).
If we had a more aggressive form of assert (assert_anal? :)) we could use that here and let the buildfarm bore the
bruntof that cost.
--
Jim C. Nasby, Data Architect jim@nasby.net
512.569.9461 (cell) http://jim.nasby.net