Author Topic: CalculateCommonTopLevelPath and commits 7307 and 7460  (Read 8999 times)

Offline Jenna

  • Administrator
  • Lives here!
  • *****
  • Posts: 7255
CalculateCommonTopLevelPath and commits 7307 and 7460
« on: October 29, 2011, 10:44:46 am »
In commit 7307 the ProjectDirTraverser was introducued, it should fiyx a problem with cmake generated projects, because they always have "/" as common toplevel path.
This leads to a long delay if the project is loaded, because the codecompletion-plugin checks the whole harddisk for includes.
See: http://forums.codeblocks.org/index.php/topic,15023.msg100822.html#msg100822

The commit had an error and leads to a toplevel path, that was different to the toplevel path calculated in older revisions, and therefore leads to an incorrect layout, if the projectfile was in a folder below it.
http://forums.codeblocks.org/index.php/topic,15023.msg101286.html#msg101286

I thought the issue is fixed and it's all good now.

But we got reports about very slow loading of large projects due to the ProjectDirTraverser:
http://forums.codeblocks.org/index.php/topic,15423.msg103528.html#msg103528

I looked into it deeper and what I found is, that it seems nobody really thought about the previous two commits.

In my opinion, we should switch back to the way we didi it before the ProjectDirTraverser was introduced, because as far as I know, it has always worked correct.

For the cmake-generated projects "/" is the correct toplevel path for all project files, because cmake puts files below /usr/src into the project, if the user has it's own files below /home/username, the common toplevelpath is in fact "/".
And if that's slows down codecompletion, it's not a bug of cc or C::b but of cmake.
And thirdparty bugs should be fixed there and not by changing our software.

I tried the 7307 changes without the DirTraverser and got the same results.
The only thing needed was to forbid a common toplevel path that has no subdirs:
Quote
if (tmpbaseF.GetDirCount() && tmpbaseF.GetDirCount() < base.GetDirCount())

This would surely speedup the loadtime a lot, but it would still be incorrect for some projects (like the cmake projects), so the former behaviour would be the best.
What we could do is check whether the drive letter is the same for a projects-file and the base-path (not common toplevel path) of a project, and only use files on the same drive for calculating.
That's only a windows problem and only in the unusual case, that users have a project with files on different drives, but it can surely happen.

I attach a patch, that also checks the volume (aka as drive-letter) and measures the time needed for CalculateCommonToplevelPath.
The stopwatch part is just to see whether it increasye the speed for the large projects and would be completeley removed before a possible commit.

Please test and give feedback.

Note the delay due to cc parsing the whole harddisk is there again, but as said before it should be fixed by cmake, or probably handled by cc itself, but it should not break the way the common toplevel path is calculated.

Offline killerbot

  • Administrator
  • Lives here!
  • *****
  • Posts: 5491
Re: CalculateCommonTopLevelPath and commits 7307 and 7460
« Reply #1 on: October 30, 2011, 11:35:50 pm »
for my big workspace I had been using and I am using the debugger branch. I hadn't updated for a while on that branch.
Now I did (7550), and indeed the workspace loading is very slow :-(
(on linux, no clue on windows)

Offline cbnewbie

  • Multiple posting newcomer
  • *
  • Posts: 25
Re: CalculateCommonTopLevelPath and commits 7307 and 7460
« Reply #2 on: October 31, 2011, 09:16:56 am »
I just downloaded c::b (r7550), applied your patch - and everything seems to be fine now. Loading my "slow" workspace is again possible within 2 seconds! Great!

Offline Jenna

  • Administrator
  • Lives here!
  • *****
  • Posts: 7255
Re: CalculateCommonTopLevelPath and commits 7307 and 7460
« Reply #3 on: November 03, 2011, 06:47:19 am »
Are there any others (especially devs) that test the patch/revert ?

If there are no objections or answers I will commit it soon. so we have the state we had before the commit, with a small change, that checks the validity of the new path (without the DirTraverser).

The change will handle the common toplevel path of the cmake generated project correctly and lead to a slowdown of CC, because the path is always "/" on linux, but as I wrote before, it's a cmake-issue and not a C::B issue.
And we should not work around it, in my opinion.
« Last Edit: November 03, 2011, 06:49:06 am by jens »

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
Re: CalculateCommonTopLevelPath and commits 7307 and 7460
« Reply #4 on: November 03, 2011, 11:26:42 am »
Are there any others (especially devs) that test the patch/revert ?
Yes, me. Works fine here, so feel free to go ahead.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: https://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: https://wiki.codeblocks.org/index.php?title=FAQ

Offline killerbot

  • Administrator
  • Lives here!
  • *****
  • Posts: 5491
Re: CalculateCommonTopLevelPath and commits 7307 and 7460
« Reply #5 on: November 06, 2011, 05:25:04 pm »
when you apply it, could you do a merge to debugger branch too ?

Offline rickg22

  • Lives here!
  • ****
  • Posts: 2283
Re: CalculateCommonTopLevelPath and commits 7307 and 7460
« Reply #6 on: October 16, 2012, 05:35:55 am »
I hope I'm not reviving an old thread, but I'm reporting horribly slow loading project times. I'd like to know if this patch could be applied soon... or at least add another configuration option like "use extensive tree traversing for project loading (use when the project layout is incorrect)" and have it disabled by default...

EDIT: I've tested revisions 8248 and the Oct 7 2012 nightly, and they keep loading slow... with 50% CPU usage (on a dual core).
« Last Edit: October 16, 2012, 05:40:50 am by rickg22 »

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
Re: CalculateCommonTopLevelPath and commits 7307 and 7460
« Reply #7 on: October 16, 2012, 07:15:13 am »
I'd like to know if this patch could be applied soon...
Isn't this applied already since ages? This DirTraverser is no longer present in cbProject.
Compiler logging: Settings->Compiler & Debugger->tab "Other"->Compiler logging="Full command line"
C::B Manual: https://www.codeblocks.org/docs/main_codeblocks_en.html
C::B FAQ: https://wiki.codeblocks.org/index.php?title=FAQ

Offline rickg22

  • Lives here!
  • ****
  • Posts: 2283
Re: CalculateCommonTopLevelPath and commits 7307 and 7460
« Reply #8 on: October 16, 2012, 02:02:32 pm »
OK, then I hit a new bug. I'll report it ASAP.