[Development] Maintainer changes to review: QMutex optimisations

[Olivier Goffart]

On Sunday 26 August 2012 12:12:18 lars.knoll at nokia.com wrote:
> On Aug 26, 2012, at 12:26 PM, ext Thiago Macieira 
<thiago.macieira at intel.com> wrote:
> > On domingo, 26 de agosto de 2012 11.10.10, Olivier Goffart wrote:
> >> I was in vacation last week without proper internet access.
> >> Many of the changes already had a +2 from me before, but it went away as
> >> you rebased or refreshed the changes.  This show a problem with the
> >> gerrit tool and a major annoyance of the "topics" since it keeps you
> >> from already merging the stand alone patches that have already been
> >> approved.
> > 
> > The earlier ones that had +2 kept them. It's only the later ones that lost
> > the +2 due to rebasing, because Gerrit doesn't know whether the change
> > depended on something from the previous version.
> > 
> > Even though it told me "no files changed, just rebased" when I pushed.
> > 
> >> Regarding the 2 patches which I have not approved: I personally think
> >> they
> >> are not giving anything usefull, just adding complexity.
> >> I think QBasicMutex and QMutex should be the same, only one is POD. And
> >> am
> >> not in favor of all the patches that goes against that.  But I realize
> >> that
> >> argument is not enough to veto or even put -1.
> 
> I don't quite get why they have to be the same, and actually don't like that
> QMutex inherits QBasicMutex, esp given that the basic mutex is internal.
> 
> I don't see why they can't be separate classes that share quite a bit of the
> implementation behind the scenes. This should not make the design any more
> complex then it is now, but give us more flexibility.

The idea was to follow the QAtomic/QBasicAtomic pattern, no more no less.
QMutex is just QBasicMutex with a constructor and destructor.

But if QBasicMutex is different than QMutex, there is indeed no reason to have 
one inheriting from the other.
but then, you need to re-define the goals of QBasicMutex, and probably find 
another name.

> > The problem is that QBasicMutex being POD -- actually, being zero
> > initialised -- is imposing constraints in how we can optimise the much
> > more used QMutex. QMutex construction happens once, so it makes sense to
> > keep the lock & timing structure, instead of having to find it from the
> > freelist on every contended lock and unlock.

The whole idea is that mutexes do not allocate any private data on the heap. 
(or any other ressources). They use almost no memory and the cost of 
construction/destryction is basically free. So you can use a lot of mutexes in 
order to have really granular locking. That is for every QMutex, not only 
QBasicMutex.

The freelist is O(1) and only used when we actually block. The freelist should 
only become a bottleneck if it is itself contented. Which only happen in a 
benchmark which does nothing but locking and unlocking mutexes.  Any code that 
does something usefull will put much less constraint on the freelist.

-- 
Olivier

Woboq - Qt services and support - http://woboq.com