Some more experiments

[Schleimer, Ben]

Hi Boud,
   I've been reading yours and Cyrille's emails about QMutex and locking and I had a brainstorm
that maybe it would be better not to use locking at all. Isn't the only times multiple threads are
going to try to modify the different tiles is when long jobs are running on them? Like resizing or
filtering or transforming? Why not divide up all of the target tiles of a job into different
groups and give each group to a thread. Then there won;t be any contention.

If thats not possible, you might want to explore using atomic variables and
atomic-increment-and-check instead of QMutex. QMutex is good if there is A LOT of threads
competing for the same resources at a time but it's overkill if there is only a couple...

Just a thought,
Ben

--- Boudewijn Rempt <boud at valdyas.org> wrote:

> In my experiments, the thumbnailing, dab compution and mask scaling showed up 
> largely, and I've tuned that code a bit. Still... I get the following result 
> with valgrind (using -O2) and thread for composition in KisBrushop::paintAt:
> 
> * computeDab: 40%, of which 25% is spent in removeReader/addReader from 
> KisTiledIterator::fetchTileData.
> 
> * scaleMask: 19%
> 
> * bitBlt: 26%, of which about 26% in createRandomAccessor, 8% in addDirtyRect 
> (QRegion, that is), 20% in KoColorSpace::bitBlt (the real work) and 24% in 
> the destructor of KisRandomConstAccessor.
> 
> Of the 50% time spent in creating and deleting the random accessors, about 30% 
> is spent adding and removing readers in KisTile.
> 
> The big problem here locking on the KisTileManager -- this is, in effect, a 
> global lock that serializes all tile access because it mutexes on the KTM 
> every time the number of readers and writers drops below 1 -- which is 
> always, since having more than one reader on a tile is very rare. So for 
> every tile access we access KisTileManager, which is a globally locked 
> singleton.
> 
> One idea I have is to have a swappable flag on KisDataManager so small, 
> ephemeral paint devices aren't subject to swapping so we can skip the calls 
> to KisTileManager for them. Another is to add a KisPainter::bitBlt() method 
> that takes as source an array of bytes instead of a QImage or QPaintDevice.
> 
> Maybe it would be better to replace the reader counting with timestamp tagging 
> & then mark & sweep collecting of tiles for compression in a separate thread. 
> A tile garbage collector.
> 
> Anyway, I'll commit what I have done soon -- either it's good, which is good, 
> or it's bad, in which case Bart can consider himself prodded :-)
> 
> -- 
> Boudewijn Rempt 
> http://www.valdyas.org/fading/index.cgi
> > _______________________________________________
> kimageshop mailing list
> kimageshop at kde.org
> https://mail.kde.org/mailman/listinfo/kimageshop
>