Review Request 108914: Thread for OutputModel contents filtering.

[Milian Wolff]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108914/#review27396
-----------------------------------------------------------


I will discard this review request and instead work on the code in the branch.

Reasons:
- we should only have one thread
- we should not repeatedly spin a thread up/down
- the timer stuff is just too complicated

I'll write a mail to the kdev-devel list when I'm done.

- Milian Wolff


On Feb. 12, 2013, 2:24 a.m., Kevin Funk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108914/
> -----------------------------------------------------------
> 
> (Updated Feb. 12, 2013, 2:24 a.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Description
> -------
> 
> Quit thread after some time
> 
> Since the OutputModel instance may live for a long time, try not to keep
> a thread running that may never be used again.
> 
> Further improve the background thread
> 
> Cache the lines a certain amount of time before trying to process them.
> This will guarantee that the batch is more likely in a filled state,
> hence reducing the number of calls to beginInsertRow() at the end.
> 
> Also add some debug output for time measurements.
> 
> Fixup/cleanup threaded output line classification.
> 
> - use a QVector instead of a QList for efficiency,
>   reserve/squeeze batch sized buffer
> - use a foreach loop instead of a broken for loop
> - don't use shared pointers for objects that are not
>   shared. here we can simply rely on QObject parent-child
>   relation ship
> - don't use a setupParser init function, use the ctor of the
>   OutputModelPrivate for that
> - use Q_PRIVATE_SLOT to not taint our public interface
> - use invokeMethod to push data to worker instead of signal
>   in public interface
> - properly close and wait for the thread when we destroy the
>   OutputModelPrivate
> - cleanup #include list of outputmodel.h
> 
> Mark FitleredItem as movable and make it possible to sore it in QVector
> 
> 
> Move parsing of lines to a background thread
> 
> This moves the parsing of the added lines to a background thread to avoid
> blocking the UI while the QRegEx'es are being matched. The current
> implementation has a major flaw though, deleting an OutputModel causes
> a crash because the thread cannot be stopped properly (at least thats what
> I think is causing the crash). This might be avoidable by dropping the
> manual usage of QThread and instead do this through QtConcurrent, but then
> we again need manual batching with a invokeMethod to ensure that not all
> lines are added at once to the view.
> 
> Removes 'removeLastLines' since the only user was the ninja-plugin and that
> one should handle its special needs in its own code instead of requiring
> API changes that nobody else needs.
> 
> Removes 'flushLineBuffer' since the idea of that function goes completely
> against the idea of doing batches in the first place so it shouldn't exist.
> If code needs to know when all items are available in the model it'll have
> to connect to the model signals and verify wether the number of lines in
> the model matches the expected final amount. This function is used in the
> ctest-related code in kdevelop, so will need a change there too.
> 
> 
> Diffs
> -----
> 
>   outputview/outputmodel.h 55920beab6ee16643ef449d816d210ba343046e6 
>   outputview/outputmodel.cpp c482222c2448fe75d97911d10aaa7b89fa15bfbf 
>   outputview/outputexecutejob.cpp 56167e6267d5d60caf5378bc1ea994a8a731e789 
>   outputview/filtereditem.h 8eab5e9d6dbefefc203884291f6894ca2a1411a9 
> 
> Diff: http://git.reviewboard.kde.org/r/108914/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Kevin Funk
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20130213/3664b457/attachment.html>