[Nepomuk] High mutex contention in Nepomuk2::Resource

[David Faure]

On Thursday 14 March 2013 23:04:24 Simeon Bird wrote:
> Hi,
> 
> I read through the patches - 0002, 0004 and 0005 are good. For 0001,
> can you not instead of this:
> 
>     lock.unlock();
>     QMutexLocker rmlock(&m_rm->mutex); // for updateKickOffLists, but
> must be locked first
>     lock.relock(); // we must respect the lock ordering!
> 
>      const QString newNaoIdentifier =
> m_cache.value(NAO::identifier()).toString();
>      const QUrl newNieUrl = m_cache.value(NIE::url()).toUrl();
>      updateIdentifierLists( oldNaoIdentifier, newNaoIdentifier );
> 
> do this:
> 
>      const QString newNaoIdentifier =
> m_cache.value(NAO::identifier()).toString();
>      const QUrl newNieUrl = m_cache.value(NIE::url()).toUrl();
> 
>     lock.unlock();
>     QMutexLocker rmlock(&m_rm->mutex); // for updateKickOffLists, but
> must be locked first
> 
>      updateIdentifierLists( oldNaoIdentifier, newNaoIdentifier );
> 
> No rule says you can't take the mutexes independently, only that if
> you take them both you must take the rmmutex first, no?

True, good point.
 
> For patch 3, how about the attached two patches instead?

I learned recently that a QReadWriteLock is actually *more* expensive than a 
mutex: it has to lock+unlock an internal mutex in lock(), and again 
lock+unlock in unlock().
So a QReadWriteLock is only worth it if
* there are many more read operations than write (this seems to be the case)
* but also: if the read operation takes long enough. If it's just about 
checking a bool or two, then it's faster to use a single mutex around that, 
than a QReadWriteLock.

About PATCH 1/5:
1) write a one-line summary when doing git commit, you can see why in the 
attached-as-email format for it.
2) don't forget to switch to DBusConnectionPool, too

> +         * As addResource, but if the added resource is the last first one,

The "last first" one? Typo, or am I missing something?

> I pushed a feature branch: feature/resourcemanagercleanup with both
> these patch series in them. It's on top of KDE/4.10, and your patches
> are on top of mine. Does it make sense to you? Vishesh, what do you
> think?

Great, thanks for doing this.

I took a look at all the commits in the branch, and apart from the fact that 
some of them revert some others (could be cleaned up before merging), it all 
looks quite good. I'll test it in kmail now.

-- 
David Faure, faure at kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5