Review Request: Enforce parse-jobs to be processed in order of their priorities

[Sven Brauch]

> On Feb. 26, 2012, 11:40 p.m., David Nolden wrote:
> > I see two problems:
> > 1. With this patch, a user editing the source-code (at least with the "sequential" flag enabled), would prevent any real background-parsing from happening, because the direct editing would repeatedly create a good-priority parsejob.
> > 2. Thinking of it, the background-parse does not seem to be the right place for handling exact parsing-sequences, because it merges multiple parse-requests for the same document together. For example, if the background-parse once gets a request for file A with priority 0 and full processing-level, and additionally a request for the same file with priority 10 and partial processing-level, then the second request will be completely merged into the first, because it is (or at least seems) redundant.
> > 
> > Probably the right place to handle all these things would be a layer on top of the background-parser which would wait for finished parse-jobs, and schedule re-parses where required accordingly.
> 
> Sven Brauch wrote:
>     Hi David,
>     
>     Point 1 is not of concern, because that flag is per-job. I only set it on those jobs which need it: if I encounter an include, both the job which should parse the include and the job which should reparse the document get the flags, and all other jobs don't have them set. The latter includes the parsejob for the currently active document, that one will never be affected by this change in any way (unless a new unresolved include statement was added since the last edit, in which case it makes sense).
>     
>     For point 2, I'm not sure if I understand what you mean. The strictest flags and the best priority are picked when the jobs are merged. Thus, both requests will always see their requirements fulfilled -- or am I overlooking something here?
>     
>     Cheers
> 
> Milian Wolff wrote:
>     I also think that the points you raise here are actually not of concern. But I think this really shows we need some kind of proof. Sven, lets try to write unit tests for this! It should actually be rather simple, all you need is just some custom TestParseJob and some list/vector to track the order in which the job(s) where run. Then you can create some jobs in a test method and verify, after they have finished, that the order was correct and nothing hanged.
>     
>     if you need help, I can probably get this thing started this week.
>     
>     bye

Hm yeah, we could probably use the QObject* notifyWhenReady stuff for this, and then add some sample files.
Still, that would more likely be a python unit test, wouldn't it? KDevelop has no default languages using this feature.

Also, could we maybe get this merged first (given David has no further objections)? It really fixes a long-standing bug for python support, and it'll be a while until the next kdevplatform release, so if it *really* turns out to cause problems, we can still improve the implementation. I don't have much time currently (exam stuff) and I don't want this to starve somewhere in the depths of the review board.

As an example it actually works, I have a test directory with four files, where first imports second, second imports third, and third imports fourth. This is what happens if you open the "first" file in the editor:
http://paste.kde.org/430328/
As you can see, the jobs are indeed well-ordered, and I have tested this quite a few times.
I also copied the same hierarchy into a large mixed-language project (konversation) and let the background parser do its work while editing a python file all the time in the foreground (thus nearly all the time a high-priority parse job was running). This is the result:
http://paste.kde.org/430334/
As you can see, the parser randomly started with the "third" file, then descended to the "fourth" one with a lower priority, thus again chosing the correct ordering.

I know this won't persuade anyone that this is a good idea, but it's an example that it *can* work. :)


- Sven


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


On Feb. 25, 2012, 5:48 p.m., Sven Brauch wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/104019/
> -----------------------------------------------------------
> 
> (Updated Feb. 25, 2012, 5:48 p.m.)
> 
> 
> Review request for KDevelop, Milian Wolff and David Nolden.
> 
> 
> Description
> -------
> 
> As recently discussed on the mailing list, it is currently unnecessarily difficult to handle the following scenario:
> File A does something like "import B"; you want A to be parsed with the top-context for B available.
> Various language plugins have various solutions for this problem, but none of them were considered optimal by their creators (as far as I understood).
> 
> This patch aims to adress that problem. It changes the parsejob creation algorithm by enforcing it to wait with creating a job with a worse priority as long as jobs with a better priority are still running. Example: Three documents A, B and C are scheduled for parsing, A and B with priority 0 and C with priority -1. Assuming two parse jobs are available, the old function would create two parsejobs for C and A or B (let's say A), then wait for one of them to finish, then create a third job for B. The new function will create a parsejob for C and wait until that one is finished, and then create two jobs for A and B (still simultaneously, because they have the same priority). In other words: It's guaranteed that all parse jobs running at any specific time have the same priority. (*)
> 
> Why is that useful? Because parsejob priorities can be used to adress the above problem now: Let priority(x) be the priority of the parse-job for document x. You can then parse A, and as soon as you encounter the "import B", you can schedule B with priority(A)-1 and schedule A (again) with priority(A). That's now guaranteed to first parse B and then re-parse A with the top-context for B being available.
> Oh, this patch also adds a function to get a parseJobs ownPriority(), that wasn't available before.
> 
> Please tell me what you think.
> 
> Greetings,
> Sven
> ________
> (*) I'm aware of the fact that this will decrease performance by a little bit. However, I'm pretty sure it's not relevant. If the general concept of this is accepted, I'll test it.
> 
> 
> Diffs
> -----
> 
>   language/backgroundparser/backgroundparser.h 954ee17 
>   language/backgroundparser/backgroundparser.cpp 7210254 
>   language/backgroundparser/parsejob.h 135319c 
>   language/backgroundparser/parsejob.cpp 552ef68 
> 
> Diff: http://git.reviewboard.kde.org/r/104019/diff/
> 
> 
> Testing
> -------
> 
> 
> Thanks,
> 
> Sven Brauch
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kdevelop-devel/attachments/20120227/49d27041/attachment.html>