[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-core-devel
Subject:    Re: Review Request: Add spinlocks lock type, based on GCC intrisincs
From:       Vadim Zhukov <persgray () gmail ! com>
Date:       2012-08-28 8:28:24
Message-ID: CAMy=nGGjXuVcC35mtF5jLEn6f=S_VZP_vCU9sgdE28X+Q7MkTQ () mail ! gmail ! com
[Download RAW message or body]

28.08.2012 12:01 =D0=BF=D0=BE=D0=BB=D1=8C=D0=B7=D0=BE=D0=B2=D0=B0=D1=82=D0=
=B5=D0=BB=D1=8C "Thiago Macieira" <thiago@kde.org> =D0=BD=D0=B0=D0=BF=D0=B8=
=D1=81=D0=B0=D0=BB:
> On segunda-feira, 27 de agosto de 2012 20.29.52, Michael Pyne wrote:
> > On Monday, August 27, 2012 20:18:34 Michael Pyne wrote:
> > > On Tuesday, August 28, 2012 00:41:16 Thiago Macieira wrote:
> > > > QBasicAtomicInt are permitted in unions. Besides, why do you want
it in
> > > > a
> > > > union in the first place? You should not access the data that it
holds
> > > > *except* via the QBasicAtomicInt functions.
> > >
> > > That would be the idea, yes (to use the public QBAI functions).
> > >
> > > The problem with having it in a union was that it's a non-POD type
> > > according to C++ 03 rules (or at least, that seemed to be the issue
when
> > > I had tried that initially).
> >
> > Actually I take that back. I was using QAtomicInt, which had that
problem.
> > QBasicAtomicInt works just fine in the union... yay!
>
> That's the whole point of QBasicAtomicInt: it's POD.
>
> Anyway, you haven't explained why you need it in a union with something
else.
> Are you accessing the data outside of QBasicAtomicInt? If so, that's
wrong. if
> you're not, you probably don't need the union.

See the definition of SharedLock structure in kshareddatacache_p.h.
Actually, other union members will not be accessed simultaneously with
spinlock, but compiler doesn't know about that.

Other way would be to get rid of using shared memory for locks at all, but
then there'll be no way for real timeouts, see file-based locking patch
nearby. (sorry for lack of links, I'm from mobile)

Personally I'd prefer not to introduce compiler-related hacks. And there is
no need to worry about code effectiveness: actual access to data in cache
is much, much longer than time spent on acquiring locks, even lockf()-based
ones.

[Attachment #3 (text/html)]

<p>28.08.2012 12:01 пользователь &quot;Thiago Macieira&quot; &lt;<a \
href="mailto:thiago@kde.org">thiago@kde.org</a>&gt; написал:<br> &gt; On \
segunda-feira, 27 de agosto de 2012 20.29.52, Michael Pyne wrote:<br> &gt; &gt; On \
Monday, August 27, 2012 20:18:34 Michael Pyne wrote:<br> &gt; &gt; &gt; On Tuesday, \
August 28, 2012 00:41:16 Thiago Macieira wrote:<br> &gt; &gt; &gt; &gt; \
QBasicAtomicInt are permitted in unions. Besides, why do you want it in<br> &gt; &gt; \
&gt; &gt; a<br> &gt; &gt; &gt; &gt; union in the first place? You should not access \
the data that it holds<br> &gt; &gt; &gt; &gt; *except* via the QBasicAtomicInt \
functions.<br> &gt; &gt; &gt;<br>
&gt; &gt; &gt; That would be the idea, yes (to use the public QBAI functions).<br>
&gt; &gt; &gt;<br>
&gt; &gt; &gt; The problem with having it in a union was that it&#39;s a non-POD \
type<br> &gt; &gt; &gt; according to C++ 03 rules (or at least, that seemed to be the \
issue when<br> &gt; &gt; &gt; I had tried that initially).<br>
&gt; &gt;<br>
&gt; &gt; Actually I take that back. I was using QAtomicInt, which had that \
problem.<br> &gt; &gt; QBasicAtomicInt works just fine in the union... yay!<br>
&gt;<br>
&gt; That&#39;s the whole point of QBasicAtomicInt: it&#39;s POD.<br>
&gt;<br>
&gt; Anyway, you haven&#39;t explained why you need it in a union with something \
else.<br> &gt; Are you accessing the data outside of QBasicAtomicInt? If so, \
that&#39;s wrong. if<br> &gt; you&#39;re not, you probably don&#39;t need the \
union.</p> <p>See the definition of SharedLock structure in kshareddatacache_p.h. \
Actually, other union members will not be accessed simultaneously with spinlock, but \
compiler doesn&#39;t know about that.</p> <p>Other way would be to get rid of using \
shared memory for locks at all, but then there&#39;ll be no way for real timeouts, \
see file-based locking patch nearby. (sorry for lack of links, I&#39;m from \
mobile)</p> <p>Personally I&#39;d prefer not to introduce compiler-related hacks. And \
there is no need to worry about code effectiveness: actual access to data in cache is \
much, much longer than time spent on acquiring locks, even lockf()-based ones.</p>



[prev in list] [next in list] [prev in thread] [next in thread] 

Configure | About | News | Add a list | Sponsored by KoreLogic