Patch: TODO highlighting in comments

Milian Wolff mail at milianw.de
Sat Nov 13 18:27:11 UTC 2010


On Saturday 13 November 2010 18:09:18 David Nolden wrote:
> I think it was too early to apply this on master. There are some
> severe efficiency problems. First of all, there were some very heavy
> QString/QByteArray operations on every single comment encountered
> during parsing, which is evil, since we already before spent a lot of
> time processing comments. I have somewhat fixed these efficiency
> problems by adding an _efficient_ pre-check for the todo keywords
> (based on fast IndexedString token comparison).

I'll benchmark it, or do you have some callgrinds around to show us where the 
bottleneck actually exists? And I clearly disagree with the "should not have 
been merged". Noone, esp. not you, would have tested it and hence it would 
just have bitrotten. This way we could try it out (it worked fine for me), and 
you could find these problems.

> The other big problem is (as Dmitry already noted) the way the
> problems are collected from the top-contexts when the scope goes over
> the whole project. There doesn't seem to be a proper updating scheme
> (see BackgroundParser::parseJobFinished for a possible entry point
> into updating), and the whole TopDUContexts of the whole project are
> processed whenever the items are collected.
> 
> As first, it has to be made sure that the problem-reporter doesn't do
> any significant work at all when the problem-report view is not
> visible. Then, it needs to be made sure that, when one parse-job has
> finished, only the problems from the corresponding TopDUContext are
> updated, and all other problems remain untouched.

Doesn't sound that hard to fix.

> For the slightly farrer future, it would be better to not use the
> duchain directly, for example to build the TODO lists, but to transfer
> the TODOs of the whole project into a separate ItemRepository, where
> they can be stored and accessed persistently and efficiently (this
> repository would need to be partially updated only after each
> parseJobFinished).

If this *really* is that big a problem, then we could do this, sure. It would 
only store IndexedString + array of todo ranges, right? A simple Mutex to make 
it threadsafe and done.

Bye
-- 
Milian Wolff
mail at milianw.de
http://milianw.de
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 198 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20101113/a962916d/attachment.sig>


More information about the KDevelop-devel mailing list