Review Request 108914: Quit thread after some time

Kevin Funk krf at gmx.de
Tue Feb 12 02:22:26 UTC 2013


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

(Updated Feb. 12, 2013, 2:22 a.m.)


Review request for KDevelop.


Changes
-------

Updated diff.

I'll add some documentation for the thread, etc. later on. I'm also going to set the sleep timer timeout to 30 seconds.
The solution with the sleep timer is not ideal in general (as Milian already pointed out), but I don't see a good alternative at the moment.


Summary (updated)
-----------------

Quit thread after some time


Description (updated)
-------

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 (updated)
-----

  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/20130212/b7a36c35/attachment-0001.html>


More information about the KDevelop-devel mailing list