Review Request 108914: Thread for OutputModel contents filtering.

[Kevin Funk]

> On Feb. 11, 2013, 11:26 p.m., Andreas Pakulat wrote:
> > outputview/outputmodel.cpp, line 58
> > <http://git.reviewboard.kde.org/r/108914/diff/1/?file=113723#file113723line58>
> >
> >     Why this timer for processing? Whats wrong with blocking the event loop until a set of lines is done?

The idea is that the cache is more likely to be filled with lines, in turn reducing the number of calls to beginInsertRow() later on.


> On Feb. 11, 2013, 11:26 p.m., Andreas Pakulat wrote:
> > outputview/outputmodel.cpp, line 59
> > <http://git.reviewboard.kde.org/r/108914/diff/1/?file=113723#file113723line59>
> >
> >     This contradicts the interval-thing, please decide wether you want a single-shot timer that is restarted manually or one that runs in an interval.

Why is that an issue? Interval sets the timeout delay of the timer, single shot defines whether it is restarted or not. They are not mutually exclusive.


> On Feb. 11, 2013, 11:26 p.m., Andreas Pakulat wrote:
> > outputview/outputmodel.cpp, line 180
> > <http://git.reviewboard.kde.org/r/108914/diff/1/?file=113723#file113723line180>
> >
> >     I can't find anything about this in the Qt docs at the moment, but does QThread actually support "restarting" the thread? I thought I once read that this is not really supported (might work or not), but maybe my recollection is wrong.

Seems to be fine, and worksforme. Related: http://www.qtcentre.org/threads/6548-Multiple-start()-on-same-QThread-Object.


> On Feb. 11, 2013, 11:26 p.m., Andreas Pakulat wrote:
> > outputview/outputmodel.cpp, line 199
> > <http://git.reviewboard.kde.org/r/108914/diff/1/?file=113723#file113723line199>
> >
> >     Hmm, what happens if the timer times out, but there are still queued lines in the thread? i.e. the thread is currently working on some lines when its event loop is being exited. Since the event-queue is local in QThread::run() (IIRC) all the still-queued things would be lost in this case.
> >     
> >     So I guess whats needed here is for the outputmodel to keep track of how many lines that where sent to the thread have been finished and if all are done, then start the timer to shut down the thread.

Not an issue. ParseWorker::process() does not return to the event-loop until *all* items were processed.


- Kevin


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


On Feb. 11, 2013, 10:11 p.m., Kevin Funk wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108914/
> -----------------------------------------------------------
> 
> (Updated Feb. 11, 2013, 10:11 p.m.)
> 
> 
> Review request for KDevelop.
> 
> 
> Description
> -------
> 
> Thread for OutputModel contents filtering.
> 
> Fixes performance issues in the OutputView tool view when inserting (and filtering) a large number of lines.
> 
> As discussed in IRC.
> 
> The patch consists of 6 commits which were pulled from the wip/filter_output_threaded branch.
> 
> 
> Diffs
> -----
> 
>   outputview/filtereditem.h 8eab5e9d6dbefefc203884291f6894ca2a1411a9 
>   outputview/outputexecutejob.cpp 56167e6267d5d60caf5378bc1ea994a8a731e789 
>   outputview/outputmodel.h 55920beab6ee16643ef449d816d210ba343046e6 
>   outputview/outputmodel.cpp c482222c2448fe75d97911d10aaa7b89fa15bfbf 
> 
> 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/92fbe22b/attachment.html>