speed improvements (patch)

[Ovidiu Gheorghioiu]

Thanks for the warm welcome guys!

I was getting ready to split the patch into multiple files like Mark
suggested, but it seems it's no longer necessary. Yippee, first
contribution to Amarok :)

Next up -- while looking at AtomicString out of curiosity, I've
discovered that it wasn't really thread safe, for a number of reasons.
First up, the mutex was not static, like the set it was protecting.
Second, some refs and derefs were done un-protected. So I wrote a
small program to unit test it, that simply allocates and deallocates
some strings in two threads (without communicating explicitly, but
their contents match a lot so they trigger the AtomicString
single-store optimization). Then I spent a chunk of time making it not
crash.

It harder than I expected, mainly because of QString's inherent thread
unsafety -- it tries not to reallocate on assignment, but without
locks, which was a surprise to me. So we have to store deep copies in
the AtomicString store and deep copy them when we take them out.

I've attached a patch (including also the unittest that crashed it
before, 100% on my dual-core) that makes it work well in multiple
threads. It's not optimal, in that the single global lock, which is
also held over refs / derefs, might some day become a performance
problem (I've verified that right now it isn't) but at least it's
safe. And while QString is also unsafe, at least it's only unsafe when
an instance is explicitly used between threads, but AtomicString was
unsafe even on unrelated instances, like in my unit test, that just
happen to use the same text.

If anybody has a better solution, that doesn't crash the unit test and
does not involve a mutex per string, I'd be happy to see it.

Regards,
Ovy

On 10/4/06, Alexandre Oliveira <aleprjlists at gmail.com> wrote:
> I commited it, as the changes indeed made a lot of sense.
> I just replaced the qFatal by a warning (we don't use qFatal ever),
> and fixed the check that would lead to it.
> _______________________________________________
> Amarok mailing list
> Amarok at kde.org
> https://mail.kde.org/mailman/listinfo/amarok
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: atomicstring-fix.patch
Type: application/octet-stream
Size: 7297 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/amarok/attachments/20061004/2786178f/attachment.obj>