Code::Blocks Forums

I found this bug this week. At first I thought it had to do with directory traversal (old bug), but it seems it's not the case.

https://developer.berlios.de/bugs/?func=detailbug&bug_id=18755&group_id=5358

QuoteOriginal Submission:
I have a PHP project under a WAMP directory (with a symlink on c:\htdocs for easy access) under Windows 7 32 bit.

Build info:

Build: Oct 13, 2012 18:50:41 - wx 2.8.12 (Windows, unicode) - 32 bit
"Version: svn build rev 8455
SDK Version: 1.13.11"

I noticed that when I open this particular project, Code::Blocks takes 22 seconds to open the project. Even when I add a file, it's painfully slow.

While this project is loaded, loading even trivial projects (like a hello world located elsewhere) takes 17 seconds.

It happened with the 8248 nightly, but also with the October 7 nightly 8438 and with 8455 (latest available at the time of this report). I can't replicate the bug at work, so it might be a rare case. I'll attach some files to add more info.

It's a long PHP project, with 2121 files including several javascript toolkits in it (ExtJS, jQuery, etc). Notice that I set up a different directory for the "executable" under build options to not disturb the source tree (maybe that's it, but I can't be sure, I'm attaching the cbp file just in case).

The last thing I saw was
"Project data set for C:\htdocs\SIG\SGPG\js\app\common\commonfuncs.js
Top Editor: C:\htdocs\SIG\SGPG\js\app\common\commonfuncs.js "

After this, I get the 20 sec hang, and finally, the environment variables are loaded.
A way to solve this would be to add more debug messages on each stage of project loading (or whatever post-loading stuff is done).

After loaded, adding a file takes 7 seconds (I'm in the middle of refactoring an urgent project, this is a royal pain in the arse guys and my job might be at stake :( Please help! )

I can share the full project to individual developers on request, if needed.

(EDIT: Unattached project file for confidentiality)

I can't attach the full project in public, it's confidential stuff (doh) and over 64 megs...

the easiest thing would be to add debugging breadcrumbs to print messages on the debug window to see where the hang happens... (I might need to be given a patched build for this so I can test)

Well, I'm off to work. Keep me updated.

Attaching my default.conf file... (this one's at home)

...In wonder if you can pin-point the hang to a particular file. What happens, if you remove all but the ones mentioned in the log?

- Also, are there any non *.js files? What, if you remove files of certain type(s) step-by-step?
- Also, does it change if you remove the *.layout file?
- Also, is it generally related to the number of files? I.e. if you remove "half" of the files, is it loaded twice as fast?

BTW: I just opened the project file - works fine and fast here... I'm afraid this is of no help.

Why don't you build a debug build of C::B and use the debugger to break it when it is hanged?
I'm sure you'll pinpoint the slow function/code.

As to the timing, isn't it rather expectable that loading a 64 MB project will take some time? Smaller projects seem to load enough quick. My "big projects" are about 1.5 MB of source code and about 300 source files (thus, not so big :) ) and I cannot complain.

If you move everything out of the WAMP directory, does the problem go away?

Quote from: Radek on October 16, 2012, 03:59:28 PM
As to the timing, isn't it rather expectable that loading a 64 MB project will take some time? Smaller projects seem to load enough quick. My "big projects" are about 1.5 MB of source code and about 300 source files (thus, not so big :) ) and I cannot complain.

Well, most of the megabytes are in .gif, .jpg and .pdf files. The real issue is that I have no problems opening this project at work (or with previous versions of C::B).

Wait... maybe "debugging" isn't the right word. What we need is a profiler. After profiling, I'm sure I can pinpoint the cause of the slowdown. :D How can I profile C::B?

Did you compare the default.conf's from at work and the system with the lag ?
Do you have any network shares, that might be involved ?
Did you try it with the exact same copy of C::B at both systems (packed here and unpacked there in a clean folder) ?
Are the related paths the same ?
Are there any probaly related (outdated) libs in the search path ?

btw, it takes about 8s to open the C::B cbp on a pretty beefy Win 7, 64 bit system with no open editors. (CC on/off makes no difference.) Is that normal?

Quote from: jens on October 16, 2012, 10:26:23 PM
Did you compare the default.conf's from at work and the system with the lag ?

Not yet, I'm still at work.

QuoteDo you have any network shares, that might be involved ?

Not that I know of.

QuoteDid you try it with the exact same copy of C::B at both systems (packed here and unpacked there in a clean folder) ?

The one at work seems to have another wxWidgets.dll, and I think I copied it to the system folder (at work). The C::B folder was zipped and unzipped pristine from c:\cb at work, to c:\cb at home. I had to download the wxWidgets dll separately (at home I used the one you provided), so that might be a candidate.

QuoteAre the related paths the same ?

At home I'm using a WAMP, at work I'm using a XAMPP. Both have a long url before the www, and both are shortcutted with a symlink (using Window's mklink command) to c:\htdocs. Note that there's no difference between opening the original path and the symlinked one, both take the same to open.
[/quote]

QuoteAre there any probaly related (outdated) libs in the search path ?

Okay, I did not check that one. Are you saying home's C::B is using another lib? Is there a way to find out which libraries (dlls I suppose) C::B is using at runtime? (Is there a plugin for that, for example?)

Quote from: rickg22 on October 16, 2012, 11:41:06 PM

QuoteAre there any probaly related (outdated) libs in the search path ?

Okay, I did not check that one. Are you saying home's C::B is using another lib? Is there a way to find out which libraries (dlls I suppose) C::B is using at runtime? (Is there a plugin for that, for example?)

You can run C::B through dependency walker: http://www.dependencywalker.com/ (http://www.dependencywalker.com/) .

OK, before anything, thank you so much for your support guys. Your suggestions and interest are really a relief.

So, I'm compiling wxWidgets right now to compile C::B from source. I'll keep you posted.

BTW, opening the C::B project itself only took around 5 seconds delay.

Quote from: rickg22 on October 17, 2012, 04:41:55 AM
OK, before anything, thank you so much for your support guys. Your suggestions and interest are really a relief.

So, I'm compiling wxWidgets right now to compile C::B from source. I'll keep you posted.

BTW, opening the C::B project itself only took around 5 seconds delay.

I did a little profiling of the time it takes to load the C::B project on my Win7 PC. It looked like most of the time was taken on the rebuilding the project manager tree view. Loading the project file and creating the project instance was very fast. Loading the C::B unix project (including building the tree view) takes less than a second on my lesser spec'd Ubuntu machine at home (actually it looks almost instantaneous after the first time).

EDIT: This could be relevant http://forums.wxwidgets.org/viewtopic.php?f=1&t=35118 but we only open a few nodes at most... Maybe this too http://forums.wxwidgets.org/viewtopic.php?t=16124&p=71104

Okay, I compiled C::B and started debugging...

Here's some info so far:

projectmanager.cpp:786: EndLoadingProject(result) takes around 40 seconds (in debug mode);
projectmanager.cpp:794: SetProject(result,true) takes from 20 to 30 seconds.

I'll run a detailed walktthrough in the next pass.

EDIT:

Okay, more fine-grained stuff:

projectmanager.cpp: 3101 project->BuildTree(m_pTree, m_TreeRoot, m_TreeVisualState, m_pFileGroups);

This takes most of the running time.

Something puzzled me, however. This builds the tree. Why is it then that in a later stage, in ProjectManager::SetProject (lines 482-483), the tree is rebuilt AGAIN? We just built it! :( (Oh, nevermind. It's the workspace tree, not the project tree)

Anyway, most of the delay is in project->BuildTree. I'll go into more detail tomorrow, it's getting late.

Okay, I've been thinking....

If Life gives you lemons, don't make lemonade...  :P

Now, seriously. The following lines seem to make a lot of noise:

       
       wxFileName nodefile = f->file;
       nodefile.MakeRelativeTo(m_CommonTopLevelPath);
       wxString nodetext = nodefile.GetFullPath();
       FileTreeData::FileTreeDataKind folders_kind = FileTreeData::ftdkFolder;


(I haven't been able to profile them yet, how do you profile that part?)

If I recall correctly, wxFileName does depend on some Operating System stuff to do its calculation. So, here's an idea: Why do we have to recalculate all these relative paths over and over for the files belonging to the same directory?

Why not storing the resulting relative filenames in a hash table? Even better, why not do the calculation first for all the directories, and THEN use the obtained values for the rest of the files? We can know if two files belong to the same directory, right? It would be matter of assigning a numeric id to each directory, and ta-da!

(This is, if most of the time spent is really in the path calculation and not in the actual widget building)


EDIT: Oh, I forgot to post the dll dependencies. Here they are.

From        To          Syms Read   Shared Object Library
0x77b01000  0x77c65d1c  Yes (*)     C:\Windows\system32\ntdll.dll
0x75561000  0x7565bd58  Yes (*)     C:\Windows\syswow64\kernel32.dll
0x76ca1000  0x76ce6a18  Yes (*)     C:\Windows\syswow64\KernelBase.dll
0x76f81000  0x7702b2c4  Yes (*)     C:\Windows\syswow64\msvcrt.dll
0x75d11000  0x76959898  Yes (*)     C:\Windows\syswow64\shell32.dll
0x75831000  0x75886b60  Yes (*)     C:\Windows\syswow64\shlwapi.dll
0x75bb1000  0x75c2292c  Yes (*)     C:\Windows\syswow64\gdi32.dll
0x758c1000  0x759a4198  Yes (*)     C:\Windows\syswow64\user32.dll
0x75ae1000  0x75b7f04c  Yes (*)     C:\Windows\syswow64\advapi32.dll
0x76b21000  0x76b38ed8  Yes (*)     C:\Windows\SysWOW64\sechost.dll
0x77151000  0x77225e04  Yes (*)     C:\Windows\syswow64\rpcrt4.dll
0x751e1000  0x752221f0  Yes (*)     C:\Windows\syswow64\sspicli.dll
0x751d1000  0x751db474  Yes (*)     C:\Windows\syswow64\cryptbase.dll
0x75ba1000  0x75ba92f8  Yes (*)     C:\Windows\syswow64\lpk.dll
0x75c41000  0x75cdc9fc  Yes (*)     C:\Windows\syswow64\usp10.dll
0x751a1000  0x751a40f0  Yes (*)     C:\Windows\SysWOW64\shfolder.dll
0x6e941000  0x6e96373c  Yes (*)     C:\MinGW\bin\libgcc_s_dw2-1.dll
0x6fc41000  0x6fd3495c  Yes (*)     C:\MinGW\bin\libstdc++-6.dll
0x62701000  0x62ddc6c0  Yes         C:\projects\wxWidgets\wxMSW-2.8.12\lib\gcc_dll\wxmsw28u_gcc_custom.dll
0x72b31000  0x72ccd18c  Yes (*)     C:\Windows\WinSxS\x86_microsoft.windows.common-controls_6595b64144ccf1df_6.0.7601.17514_none_41e6975e2bd6f2b2\comctl32.dll
0x76b91000  0x76c0a39c  Yes (*)     C:\Windows\syswow64\comdlg32.dll
0x76cf1000  0x76e4b0cc  Yes (*)     C:\Windows\syswow64\ole32.dll
0x756b1000  0x7573e644  Yes (*)     C:\Windows\syswow64\oleaut32.dll
0x73a71000  0x73aa1264  Yes (*)     C:\Windows\SysWOW64\winmm.dll
0x736f1000  0x737407a0  Yes (*)     C:\Windows\SysWOW64\winspool.drv
0x73951000  0x73956108  Yes (*)     C:\Windows\SysWOW64\wsock32.dll
0x75671000  0x756a4784  Yes (*)     C:\Windows\syswow64\ws2_32.dll
0x77ad1000  0x77ad5058  Yes (*)     C:\Windows\syswow64\nsi.dll
0x00de1000  0x01336718  Yes         c:\projects\cb\src\devel\codeblocks.dll
0x6e441000  0x6e4f93c0  Yes         c:\projects\cb\src\devel\wxpropgrid.dll
0x75361000  0x753a1ce0  Yes (*)     C:\Windows\SysWOW64\imm32.dll
0x76eb1000  0x76f7bebc  Yes (*)     C:\Windows\syswow64\msctf.dll
0x73811000  0x73872a7c  Yes (*)     C:\Windows\SysWOW64\uxtheme.dll
0x6fe71000  0x6fe8299c  Yes (*)     C:\Windows\SysWOW64\dwmapi.dll
0x736d1000  0x736e57b2  Yes (*)     C:\Windows\SysWOW64\cryptsp.dll
0x73691000  0x736ca244  Yes (*)     C:\Windows\SysWOW64\rsaenh.dll
0x72811000  0x7281d7ac  Yes (*)     C:\Windows\SysWOW64\RpcRtRemote.dll
0x743d1000  0x74463f78  Yes (*)     C:\Windows\SysWOW64\msftedit.dll
0x6bd81000  0x6bd99ba0  Yes         c:\projects\cb\src\devel\share\codeblocks\plugins\abbreviations.dll
0x712c1000  0x71312cb4  Yes         c:\projects\cb\src\devel\share\codeblocks\plugins\astyle.dll
0x64381000  0x64395a68  Yes         c:\projects\cb\src\devel\share\codeblocks\plugins\autosave.dll
0x6af01000  0x6af1df1c  Yes         c:\projects\cb\src\devel\share\codeblocks\plugins\classwizard.dll
0x65e81000  0x65fa41f0  Yes         c:\projects\cb\src\devel\share\codeblocks\plugins\codecompletion.dll
0x64b01000  0x64c6ca48  Yes         c:\projects\cb\src\devel\share\codeblocks\plugins\compiler.dll
0x6d881000  0x6d8fcf18  Yes         c:\projects\cb\src\devel\share\codeblocks\plugins\debugger.dll
0x649c1000  0x649dc2a8  Yes         c:\projects\cb\src\devel\share\codeblocks\plugins\defaultmimehandler.dll
0x69041000  0x690515e4  Yes         c:\projects\cb\src\devel\share\codeblocks\plugins\openfileslist.dll
0x70501000  0x705335f8  Yes         c:\projects\cb\src\devel\share\codeblocks\plugins\projectsimporter.dll
0x63c01000  0x63c4a6e0  Yes         c:\projects\cb\src\devel\share\codeblocks\plugins\scriptedwizard.dll
0x6bac1000  0x6baecacc  Yes         c:\projects\cb\src\devel\share\codeblocks\plugins\todo.dll
0x62301000  0x6230d474  Yes         c:\projects\cb\src\devel\share\codeblocks\plugins\xpmanifest.dll
0x739f1000  0x73a3b464  Yes (*)     C:\Windows\SysWOW64\apphelp.dll
0x72b21000  0x72b2404c  Yes (*)     C:\Windows\SysWOW64\msimg32.dll
(*): Shared library is missing debugging information.


Quote from: rickg22 on October 17, 2012, 07:36:28 AM
(I haven't been able to profile them yet, how do you profile that part?)

wxStopWatch with a DebugLog output, maybe?

On linux it takes less than a second for a project with more then 7000 files (one of my wxWidgets test-projects).

But I noticed one thing, that can increase the loading time a lot:
At first run (when opening a project or workspace), we always call BuildTree() and RebuildTree() for the project.
RebuildTree() deletes the content of the tree and rebuilds it from scratch.
I think this should be avoided if possible.

I have no time to look into it at the moment, maybe later this week, if I am back home.

Using wxStopWatch I can confirm that almost all of the opening time is spent on wxFileName::MakeRelativeTo. Rebuilding the tree control isn't all that expensive.

Quote from: dmoore on October 17, 2012, 03:29:59 PM
Using wxStopWatch I can confirm that almost all of the opening time is spent on wxFileName::MakeRelativeTo. Rebuilding the tree control isn't all that expensive.

Mind showing us the cumulative stats?

Anyway, I got an idea that might just work. Here's the pseudocode.


1. wxString lastparentpath = "", curparentpath = "", lastrelativepath = "", currelativepath = "", currelativefilename = "", DS = "/"; // DS is the directory separator, adapt to OS needs.
2. for each of the filenames to be processed as curfilename: {
   2.1. curparentpath = obtain_path_only(curfilename); // get the full path without the filename
   2.2. basefilename = obtain_filename_only(curfilename); // filename only, plus extension
   2.3. if(curparentpath == lastparentpath && lastparentpath !== "") {
         currelativepath = lastrelativepath;
         currelativefilename = currelativepath + DS + basefilename;
         }
   2.4. else {
            currelativefilename = MakeRelative(filename,commontopprojectdirectory);
            lastparentpath = curparentpath;
            lastrelativepath = obtain_path_only(currelativefilename);
         }
   2.5. Do the rest of the tree node adding here.
}


This way, MakeRelative is only called for the first file of a given directory. The rest of the relative filenames are just calculated using a concatenation.

As for the obtain_path_only and obtain_filename_only functions, they are easily implemented using a reverse searching for the "/" and "\" characters on each filename, and splitting the string in two at that position. In fact, we could use a single function that splits the filename into path and basename using only one search.

@rick: The MakeRelativeTo calls were taking up >90% of the ProjectManager::RebuildTree (i.e. at least 1800 of about 2000 ms)

EDIT: Here's an example:

Done loading project in 233ms
BuildTree
MakeRelativeTo time 1807 ms
Total time 1905 ms
BuildTree
MakeRelativeTo time 1793 ms
Total time 1890 ms



Not sure the more complicated patch is needed because ProjectFile already has a member relativeToCommonTopLevelPath. This is initialized when the project is first opened, but it does not use MakeRelativeTo. Instead it does:


           pf->relativeToCommonTopLevelPath = fullFilename.Right(fullFilename.Length() - m_CommonTopLevelPath.Length());


Which I'm sure breaks in the corner case where files are split across volumes.

But anyway, if we use that member we have a simple patch:


Index: C:/Users/damienm/Documents/damien/Source/codeblockssrc/trunk/src/sdk/cbproject.cpp
===================================================================
--- C:/Users/damienm/Documents/damien/Source/codeblockssrc/trunk/src/sdk/cbproject.cpp (revision 8456)
+++ C:/Users/damienm/Documents/damien/Source/codeblockssrc/trunk/src/sdk/cbproject.cpp (working copy)
@@ -907,9 +907,7 @@
        ftd->SetProjectFile(f);
        ftd->SetFolder(f->file.GetFullPath());

-        wxFileName nodefile = f->file;
-        nodefile.MakeRelativeTo(m_CommonTopLevelPath);
-        wxString nodetext = nodefile.GetFullPath();
+        wxString nodetext = f->relativeToCommonTopLevelPath;
        FileTreeData::FileTreeDataKind folders_kind = FileTreeData::ftdkFolder;

        // by default, the parent node is the project node (in case of no grouping, no virtual folders)


If we want a patch that is robust to files split across volumes it should be a fix for the way relativeToCommonTopLevelPath is initialized.

There is still a small delay from the project loader (200ms), but that may just be parsing the xml and checking that files exist and, thus, not much can be done about.

The commonTopLevelPath might hav echanged since initializing relativeToCommonTopLevelPath.
This can happen every time a new file is added.
One way to handle this would be to reset this variable every time the commonTopLevelPath has changed.
Another would be to use a function instead, which does exactly what happens when it is initialized.
This should not be too expensive (just a wxString.Right() ).
A different volume on windows could be handled here also, most likely.
Should be doable without the use of wxFileName in case fullFileName also keeps the volume of a file, if not we might introduce another member, that keeps it.

we've been here before  :P

http://forums.next.codeblocks.org/index.php/topic,6288.0.html

svn (http://svn.berlios.de/wsvn/codeblocks?manualorder=1&op=comp&compare%5B0%5D=%2Ftrunk&compare_rev%5B0%5D=4217&compare%5B1%5D=%2Ftrunk&compare_rev%5B1%5D=4218&comparesubmit=Compare+Paths)

But yes, I agree with you Jens that probably the best way to go is to replace that member with a function that does the calculation on the fly.

@dmoore:
I attach a patch, that combines your patch and a slightly enhanced patch of MortenMacFly (see http://forums.next.codeblocks.org/index.php/topic,16947.msg115392.html#msg115392).

Soe measurements on linux with wxWidgets trunk (> 10.000 files):

without the patch:

Quote/home/jens/codeblocks-build/codeblocks.git/src/sdk/cbproject.cpp::void cbProject::BuildTree(cbTreeCtrl*, const wxTreeItemId&, int, FilesGroupsAndMasks*):1008  took : 1406 ms
/home/jens/codeblocks-build/codeblocks.git/src/sdk/cbproject.cpp::void cbProject::BuildTree(cbTreeCtrl*, const wxTreeItemId&, int, FilesGroupsAndMasks*):1008  took : 1432 ms

CalculateCommonTopLevelPath() took 275 ms
iterating through all 10677 files took 1030 ms
iterating through all 10677 files took 1066 ms

with the patch:

Quote/home/jens/codeblocks-build/codeblocks.git/src/sdk/cbproject.cpp::void cbProject::BuildTree(cbTreeCtrl*, const wxTreeItemId&, int, FilesGroupsAndMasks*):1002  took : 1123 ms
/home/jens/codeblocks-build/codeblocks.git/src/sdk/cbproject.cpp::void cbProject::BuildTree(cbTreeCtrl*, const wxTreeItemId&, int, FilesGroupsAndMasks*):1002  took : 1132 ms

CalculateCommonTopLevelPath() took 275 ms
iterating through all 10677 files took 753 ms
iterating through all 10677 files took 751 ms

Not much in absolute time, but about 25 to 30 %.

I will test it on windows soon, also with mixed volume projects.

Does this do anything to ensure relativeToCommonTopLevelPath is kept up to date? (Or is that already taken care of?)

Quote from: dmoore on October 18, 2012, 12:09:13 AM
Does this do anything to ensure relativeToCommonTopLevelPath is kept up to date? (Or is that already taken care of?)

I did not add any call to it, but if it is not kept up to date, it would be another bug.
Currently it's called in

• cbProject::Open()
• cbProject::AddFile()
• ProjectFile::Rename()
• ProjectManager::OnRemoveFileFromProject()
  

Quote from: jens on October 17, 2012, 11:54:55 PM
Not much in absolute time, but about 25 to 30 %.

I will test it on windows soon, also with mixed volume projects.

On windows without the volume stuff, I got an improvement from about 4000 to about 400 ms to open the C::B project. (Loading and the two BuildTree calls) Will test your patch tomorrow if I have time.

OK, I applied the patch (manually), and the project opens INSTANTLY! :D

Unfortunately, none of the files can be opened :P maybe I applied it wrong?  ???

EDIT: Oops, the extensions handler plugin was disabled :P

OK, confirmed for me. Congratulations, Jens! With your patch, loading time goes down from 22 seconds to ZERROW.

Apply the patch, Scotty! Er, I mean, Jens.

(Oh, and after this is patched, remember to do something about the double loading)

To make it clear, the real solution comes from dmoore, MortenMacFly fixes a problem when calculating the common TopLevelPath with projectfiles on different volumes, I just added a small part to skip files on different volumes when calculating the common TopLevelPath.

The real speedup comes from dmoores patch.

Now let's test it (especially on win with files spread over two and more volumes) and if all goes well it can be committed.

Quote from: jens on October 18, 2012, 10:26:09 AM
Now let's test it (especially on win with files spread over two and more volumes)

I can do it later on. What needs to be tested:
- project / project layout file saving / loading with files on same and different volumes
- project / project layout file saving / loading with files on UNC path's
- especially saving project files below and above a certain source folder
- compilation process and UI behaviour where this path's are used

...anything else?

Quote from: MortenMacFly on October 18, 2012, 11:20:56 AM

Quote from: jens on October 18, 2012, 10:26:09 AM
Now let's test it (especially on win with files spread over two and more volumes)

I can do it later on. What needs to be tested:
- project / project layout file saving / loading with files on same and different volumes
- project / project layout file saving / loading with files on UNC path's
- especially saving project files below and above a certain source folder
- compilation process and UI behaviour where this path's are used

...anything else?

I haven't tested everything, but I made up a one file ConsoleProject, added another file on another volume to a new target then compiled and ran that target successfully.


-------------- Build: ExternalFileProject in ConsoleProject (compiler: GNU GCC Compiler)---------------

mingw32-g++.exe -Wall -fexceptions  -O2     -c t:\CProjects\ConsoleProject\main.cpp -o obj\Release\t\CProjects\ConsoleProject\main.o
mingw32-g++.exe  -o bin\Release\ConsoleProject.exe obj\Release\t\CProjects\ConsoleProject\main.o   -s  
Output size is 478.50 KB
Process terminated with status 0 (0 minutes, 2 seconds)
0 errors, 0 warnings (0 minutes, 2 seconds)


Also did the same for adding a file just one directory above the project and no problems there either.

No problems loading/saving the project. The project tree shows the file on a separate volume nested appropriately under Sources/t/CProjects/ConsoleProject/main.cpp, and the different project tree options work as expected. For the file one directory above, the basedir of the tree adjusts appropriately.

So far so good.

Finally: Some time for testing - but unfortunately this patch breaks handling of file names on UNC drives.
If you add a file on an UNC drive this is immediately broken and cannot be opened.

While the relative file name in the file's properties look correct (it remains the UNC path w/o simplification), the absolute path is wrong: It is compiles ad project base path plus the UNC path, like:
C:\Folder\Base\SERVER\folder\file.cpp

This is obviously wrong. :-(

What also doesn't work:

Have this layout:
[Root][src] - file.cpp
[Root]proj.cbp
- Open project
- Save project under:
[Root][prj] - proj.cpp

-> All files are broken, project file is broken.

...what also doesn't work:
Have this layout:
[C:\Folder][src] - file.cpp
[C:\Folder]proj.cbp
- Open project
- Save project under:
[D:\Folder] - proj.cpp

-> All files are broken, project file is broken.

I'm afraid all this is clearly a "No-go"... :( :-\

Quote from: MortenMacFly on October 23, 2012, 03:59:22 PM
What also doesn't work:

Have this layout:
[Root][src] - file.cpp
[Root]proj.cbp
- Open project
- Save project under:
[Root][prj] - proj.cpp

-> All files are broken, project file is broken.

Did this work previously?

Quote from: MortenMacFly on October 23, 2012, 03:34:45 PM
While the relative file name in the file's properties look correct (it remains the UNC path w/o simplification), the absolute path is wrong: It is compiles ad project base path plus the UNC path, like:
C:\Folder\Base\SERVER\folder\file.cpp

So we need to write our own implementation of GetVolume that treats \\Server (or //Server) as a volume?

Quote from: dmoore on October 24, 2012, 03:17:45 AM
So we need to write our own implementation of GetVolume that treats \\Server (or //Server) as a volume?

I'd simply say do never change files with UNC path's. So before checking for the volume we should check for UNC path's and then leave it as it is. This would not require and modification of GetVolume.

Quote from: MortenMacFly on October 24, 2012, 07:12:13 AM

Quote from: dmoore on October 24, 2012, 03:17:45 AM
So we need to write our own implementation of GetVolume that treats \\Server (or //Server) as a volume?

I'd simply say do never change files with UNC path's. So before checking for the volume we should check for UNC path's and then leave it as it is. This would not require and modification of GetVolume.

But what if the projectfile (*.cbp) has an UNC path ?

Quote from: jens on October 24, 2012, 09:25:06 AM
But what if the projectfile (*.cbp) has an UNC path ?

You mean if the project file itself is on an UNC path? In that case, there is no common top-level path and all files should use absolute file names / path's for all purposes. However, this is a very special case.

If you mean it contains files that are on an UNC path, these files must always be handled without simplifications, so there is no base path for them and also no relative path's. Those file should always only use the full path.

In the project tree they are shown as:
[ROOT]-[SERVER]-[FOLDER]-[...]-File.cpp
...while "normal" files are shown as:
[ROOT=BASE_PATH]-[FOLDER]-[...]-File.cpp

...btw: having re-applied my original patch, most things work, except UNC handling. (Did I mention that UNC sucks, btw?! ;D)

Quote from: MortenMacFly on October 24, 2012, 09:34:03 AM
...btw: having re-applied my original patch, most things work, except UNC handling. (Did I mention that UNC sucks, btw?! ;D)

But this will lead to the slowdown again.
Checking for an UNC pat should not be too hard.

And for the other issue (if the projectfile itself is aved on a different volume, after calculating the commonTopLevelPath):
does saving the project on a different path trigger the recalculating of the commontopLevelPath (and rebuilding the project tree) ?
If not, the error is there.
I hope I find the time to look into it today.

Can you test the attached patch ?
It's not tested in any case by me (just a quick hack), so be careful with real projects.

Quote from: jens on October 24, 2012, 10:26:34 AM
But this will lead to the slowdown again.

True. But my trial was to see if the changes of dmoore actually caused the mis-behaviour.

Quote from: jens on October 24, 2012, 10:26:34 AM
does saving the project on a different path trigger the recalculating of the commontopLevelPath (and rebuilding the project tree) ?

Well it works for me now (I'll double-check in a minute) - so my guess is that the error was in the patch...

Quote from: jens on October 24, 2012, 11:23:52 AM
Can you test the attached patch ?

...will do. But it may take some time again.

Quote from: MortenMacFly on October 24, 2012, 09:31:58 AM

Quote from: jens on October 24, 2012, 09:25:06 AM
But what if the projectfile (*.cbp) has an UNC path ?

You mean if the project file itself is on an UNC path? In that case, there is no common top-level path and all files should use absolute file names / path's for all purposes. However, this is a very special case.

what about if everything is on the same UNC path (project and project files). Are you suggesting that the files in this project should always be treated as absolute? (I hope not.)

Quote from: dmoore on October 24, 2012, 03:15:55 AM

Quote from: MortenMacFly on October 23, 2012, 03:59:22 PM
What also doesn't work:

Have this layout:
[Root][src] - file.cpp
[Root]proj.cbp
- Open project
- Save project under:
[Root][prj] - proj.cpp

-> All files are broken, project file is broken.

Did this work previously?

To clarify, try the following:

1. open project
2. save project as -> move one directory up and click save
3. close project
4. reopen project -> all broken links

I tried applying Jens' latest patch, but that didn't appear to help with this issue.

EDIT: I'm guessing we need to do something more like ProjectFile::Rename for each file in the project if the project file is saved to a different place. i.e. we need to update relativeFilename and not just relativeToCommonTopLevelPath of each ProjectFile and any relative target/project paths.

Quote from: dmoore on October 24, 2012, 10:14:03 PM
EDIT: I'm guessing we need to do something more like ProjectFile::Rename for each file in the project if the project file is saved to a different place. i.e. we need to update relativeFilename and not just relativeToCommonTopLevelPath of each ProjectFile and any relative target/project paths.

Please test this patch, it calculates the relativeFilename also.
It uses the relative expensive MakeRelativeTo-function again, but only in case the project and the file are on the same volume or below the same UNC path, so it might avoid the slowdown.

UNC-handling is not (yet) tested.

Quote from: dmoore on October 24, 2012, 10:11:26 PM
what about if everything is on the same UNC path (project and project files). Are you suggesting that the files in this project should always be treated as absolute? (I hope not.)

Nope. In that case using relative path might be OK again. What I meant is if just the project file itself (and maybe 1..n , while n<max. project files) are on an UNC path. Whereas in that case all files that are relative to the project file on the same drive or same UNC base path can be relative as usual.

Quote from: jens on October 25, 2012, 01:25:58 AM
Please test this patch, it calculates the relativeFilename also.

Which one? Did you forget to attach a new, or do you mean the latter?

Quote from: MortenMacFly on October 25, 2012, 06:52:13 AM

Quote from: jens on October 25, 2012, 01:25:58 AM
Please test this patch, it calculates the relativeFilename also.

Which one? Did you forget to attach a new, or do you mean the latter?

Forgotten, sorry.
It was a little late last night, I think.
Here it is .

Quote from: jens on October 25, 2012, 09:09:55 AM
Here it is .

Ok, that resolves the broken links after saving a project to a new folder. Projects with relative build paths get built in different locations now, of course... Guessing this will also break builds  when there a link libs specified on relative paths. (e.g. the ..\..\devel linker path that we use in CB), but it might be enough to warn the user if they change the project path that they will need to manually update these things?

Quote from: dmoore on October 25, 2012, 05:26:22 PM

Quote from: jens on October 25, 2012, 09:09:55 AM
Here it is .

Ok, that resolves the broken links after saving a project to a new folder. Projects with relative build paths get built in different locations now, of course... Guessing this will also break builds  when there a link libs specified on relative paths. (e.g. the ..\..\devel linker path that we use in CB), but it might be enough to warn the user if they change the project path that they will need to manually update these things?

This surely breaks it, but trying to update each and any possible path will surely lead to other errors.

EDIT:
And as posted before, it is not related to the patch.

Quote from: dmoore on October 25, 2012, 05:26:22 PM
Projects with relative build paths get built in different locations now, of course...

BTW: It was always like that. In the past you were able to save the project file to different locations and the project file was updated accordingly. So yes, this breaks such things, but that would be the same for every IDE out there. And I think it should be pretty cleasr.

However, as also many Newbies use C::B - an AnnoyingDiaolg maybe worth a try...

What probably could be done in the Project SaveAs routine, is updating relative paths like workdir etc, but this would be ahidden magic, which can lead to broken projects as well as not doing this.
So an annoying dialog, before the project's new base-path is changed is the best thing.

Quote from: jens on October 24, 2012, 09:25:06 AM
But what if the projectfile (*.cbp) has an UNC path ?

You mean if its on an UNC path? In that case, all files need to have absolute information and there is no common top-level path. But that doesn't matter, if all files have absolute path's... isn't it?

BTW: Testing the patch:
Unfortunately already the first trial fails miserably:
- create a project
- add a path on an UNC path
- file is immediately broken in project tree (although the "tree path" would be OK)
-> you cannot open/edit that file from the project tree.
(This I had just tried to fix in trunk recently...:-()

Quote from: MortenMacFly on October 27, 2012, 08:53:37 PM
- file is immediately broken in project tree (although the "tree path" would be OK)

BTW: It seems there are two errors in that case:
- assume project on C:\Folder\project.cbp
- assume a file on \\server\folder\file.cpp
--> the file's absolute path is constructed from the common top level path plus the file's ("single slashed") UNC path like that:
C:\Folder\server\folder\file.cpp

--> the relative path is correct though:
\\server\folder\file.cpp

I'll commit a patch that will help to inspect what's going on in a minute...

Quote from: MortenMacFly on October 27, 2012, 08:53:37 PM
Unfortunately already the first trial fails miserably:

3rd post on that topic:

The rest seems to work well:
- saving project above/below source tree
- saving project on different drive
- having project files scattered across several drives

What does not work (and has very strange side-effects) is the following layout:

- project on/in C:\folder\project
- source files in C:\folder, C:\folder\src
- source files in D:\other_folder

--> now try to save the project under D:\other_folder\prj

--> weird project tree layout. including files with one letter, not existing.
--> seems the file name get truncated according the length of the folder

--> maybe its related to that check:
if ( tmpbaseF.GetDirCount() < base.GetDirCount() && ???

Quote from: MortenMacFly on October 27, 2012, 09:17:18 PM
--> maybe its related to that check:
if ( tmpbaseF.GetDirCount() < base.GetDirCount() && ???

This should only be reached if the projects base-path is on the same volume as the file being processed.
If this worls correctly, this can not be the cause.
Maybe the base-path is not correctly set after saving the project on different drive ?

Suddenly I'm starting to feel guilty for opening this can of worms  :-[

I'm so sorry!!! (But still, I don't want to go back to that 20-second delay on project opening)

Quote from: rickg22 on October 29, 2012, 05:04:10 AM
Suddenly I'm starting to feel guilty for opening this can of worms  :-[

Its easy to re-recover: Come up with a patch that works! :-)

Quote from: MortenMacFly on October 29, 2012, 09:03:10 AM
Come up with a patch that works! :-)

Well here I got one: If combines Jens approach with my fix for UNC path's in the project tree.

This fixes all issues with presenting except this one:
http://forums.next.codeblocks.org/index.php/topic,16967.msg116003.html#msg116003

...but we are one step closer!

...Jens and others: before you start digging into it - I guess I've solved the last issue meanwhile. Just need some more testing, then I'll come up with a new patch.

...here it comes, the final patch that works for all test cases (including different project tree viewer setups, btw):

The bottleneck: This fast method to obtain the path of a project file relative to the common top level path:
//            f->relativeToCommonTopLevelPath = fileName.Right(fileName.Length() - m_CommonTopLevelPath.Length());

...does not work. I've commented why it does not work an use now also a wxFileName call. This is more expensive, but safe. I would love to see time measuring again, if it doesn't cost too much.

But I believe with this patch, we finally solved the actual issue with UNC, relative and path's on different drives all together once and for all.

Ps.: If you use a good diff-viewer, you can disable white space changes, as there are some not related to the actual fix. Sorry for that, I "a-style"d some portions for better understanding of what's going on.


+        if (   (!prjHasUNCName &&  fileHasUNCName)
+            || ( prjHasUNCName && !fileHasUNCName) )


Maybe I'm a little pedantic here, but couldn't that be written as...


if(prjHasUNCName != fileHasUNCName)


?

Anyway, here's the core of your patch:


@@ -358,12 +380,37 @@
    for (FilesList::iterator it = m_Files.begin(); it != m_Files.end(); ++it)
    {
        ProjectFile* f = (*it);
-
        if (!f)
            continue;

        wxString fileName = f->file.GetFullPath();
-        f->relativeToCommonTopLevelPath = fileName.Right(fileName.Length() - m_CommonTopLevelPath.Length());
+        fileHasUNCName = fileName.StartsWith(_T("\\\\"));
+
+        if (   (prjHasUNCName && fileHasUNCName)
+            || (   !prjHasUNCName
+                && !fileHasUNCName
+                && vol.IsSameAs(f->file.GetVolume()) ) )
+        {
+            wxFileName relFileCTLP(f->file);
+            relFileCTLP.MakeRelativeTo( m_CommonTopLevelPath );
+            wxFileName relFileBase(f->file);
+            relFileBase.MakeRelativeTo( GetBasePath() );
+
+            // The commented (old) method to obtain the relativeToCommonTopLevelPath is fast, but does *not* work, if you save
+            // the project on a different drive in a sub-folder of an existing source file on that (different) drive:
+            // I.e.: Project on C:\Folder\Project.cbp has file C:\Folder\SubFolder\foo.cpp and D:\Folder\bar.cpp
+            // Saved the project under D:\Folder\SubFolder\ProjectNew.cbp would cause a wrong computation of bar.cpp otherwise!!!
+//            f->relativeToCommonTopLevelPath = fileName.Right(fileName.Length() - m_CommonTopLevelPath.Length());
+            // Using wxFileName instead, although its costly:
+            f->relativeToCommonTopLevelPath = relFileCTLP.GetFullPath();
+            f->relativeFilename             = relFileBase.GetFullPath();
+        }
+        else
+        {
+            f->relativeToCommonTopLevelPath = fileName;
+            f->relativeFilename             = fileName;
+        }
+


It looks pretty good to me; It uses wxFilename only when necessary. Anyway, I'm thinking that these two lines


+            f->relativeToCommonTopLevelPath = relFileCTLP.GetFullPath();
+            f->relativeFilename             = relFileBase.GetFullPath();


could be optimized further by adding a path "cache" and comparing the file's relative pathname (the path without the filename) with the last file's pathname, and using the last calculated relativeToCommonTopLevelPath and relativeFilename instead of just calling wxFilename right away. This was my original proposal, btw, but all the other optimizations submitted here seem pretty rad :)

EDIT: OH, WAIT A MINUTE!!! I re-read that patch again...


+        if (   (prjHasUNCName && fileHasUNCName)
+            || (   !prjHasUNCName
+                && !fileHasUNCName
+                && vol.IsSameAs(f->file.GetVolume()) ) )
+        {

This is the precondition for the fast calculation, right? And then you're just undoing all the optimizations here! If the only failing case happens when the old project's drive is different than the current one, why not add a condition that specifically says so?


if(prjHasChangedDrives) { // prjHasChangedDrives is the real issue here, isn't it?
//   If it has either changed windows drives or UNC network drives, we should use the full method;
//   Otherwise, the fast one.
//   Calculating prjHasChangedDrives should be trivial, anyway.

    wxFileName relFileCTLP(f->file);
    relFileCTLP.MakeRelativeTo( m_CommonTopLevelPath );
    wxFileName relFileBase(f->file);
    relFileBase.MakeRelativeTo( GetBasePath() );
    f->relativeToCommonTopLevelPath = relFileCTLP.GetFullPath();
    f->relativeFilename             = relFileBase.GetFullPath();
} else {
    f->relativeToCommonTopLevelPath = fileName.Right(fileName.Length() - m_CommonTopLevelPath.Length());
}


Another possibility:


int fn_length = fileName.Length();
int prj_length = m_CommonTopLevelPath.Length();
bool is_file_above_project = fn_length <= m_CommonTopLevelPath.Length;
// This is an approximation; The correct approach would be not to compare the filenames, but the pathnames without filenames.
if(is_file_above_project) {  // use wxFilename here because the file is on a directory above the project
 wxFileName relFileCTLP(f->file);
 relFileCTLP.MakeRelativeTo( m_CommonTopLevelPath );
 wxFileName relFileBase(f->file);
 relFileBase.MakeRelativeTo( GetBasePath() );
 f->relativeToCommonTopLevelPath = relFileCTLP.GetFullPath();
 f->relativeFilename             = relFileBase.GetFullPath();
} else {
 // Also, shouldn't we compare the part of the path we're removing?
 f->relativeToCommonTopLevelPath = fileName.Right(fileName.Length() - m_CommonTopLevelPath.Length());
}


Approach 2:


 int len_diff = fileName.Length() - m_CommonTopLevelPath.Length();
 bool path_mismatch = (len_diff < 0);
 if(!path_mismatch) {
    // If there's a wxString function that compares from right to left, we should use it here.
    path_mismatch = fileName.Left(m_CommonTopLevelPath.Length()) != m_CommonTopLevelPath;
     // NOTE: I'm assuming that m_CommonTopLevelPath ends in a directory separator; otherwise,
     // we could have problems if m_CommonTopLevelPath is something like "MyDirectory"
     // and the file is something like "MyDirectoryFilename".
 }
 if(path_mismatch) {
     wxFileName relFileCTLP(f->file);
     relFileCTLP.MakeRelativeTo( m_CommonTopLevelPath );
     wxFileName relFileBase(f->file);
     relFileBase.MakeRelativeTo( GetBasePath() );
     f->relativeToCommonTopLevelPath = relFileCTLP.GetFullPath();
     f->relativeFilename             = relFileBase.GetFullPath();
 } else {
     f->relativeToCommonTopLevelPath = fileName.Right(fileName.Length() - m_CommonTopLevelPath.Length());
 }


Quote from: rickg22 on October 31, 2012, 02:34:05 PM
Another possibility:

Rick, the problem is not switching the drive, but saving the project file below a folder of source files. This can be even on the same drive. So your patches re-introduce this bug.

Edit: Out posts crossed. I am inspecting your second post.

OK - after inspecting this, both of your methods are not safe.

The first one might fail, if the file name is longer than the path, for example (which can happen often) and the second one might fail as you have described in the comment already.

So - do we want something that works, or something that might work but is speedy? I prefer the first.

Also, remember that the speed-up was actually in another part of the code, so maybe we don't have issues at all and we do nut-picking. With this (my) patch applied, did you do time measuring with your project in question? IS it really that slow??? How much time does it take, compared to the initial version?

Quote from: MortenMacFly on October 31, 2012, 05:46:47 PM
OK - after inspecting this, both of your methods are not safe.

The first one might fail, if the file name is longer than the path, for example (which can happen often) and the second one might fail as you have described in the comment already.

Yeah. I'd go for the second approach, the only thing I need is a function analogue to php's dirname() to solve any inconsistencies.

QuoteAlso, remember that the speed-up was actually in another part of the code, so maybe we don't have issues at all and we do nut-picking. With this (my) patch applied, did you do time measuring with your project in question? IS it really that slow??? How much time does it take, compared to the initial version?

No, I haven't had time to test it, I may do it at home tonight. But if I'm correct, your fix will introduce exactly the same slowdown that was fixed in the other part of the code.

Anyway, against which version of SVN was the patch created?

Quote from: rickg22 on October 31, 2012, 10:24:42 PM
But if I'm correct, your fix will introduce exactly the same slowdown that was fixed in the other part of the code.

I believe the opposite... but lets see... ;-)

Quote from: rickg22 on October 31, 2012, 10:24:42 PM
Anyway, against which version of SVN was the patch created?

Trunk, as usual.

By the way, this bug may be related to the wxFilename slowdown:

http://trac.wxwidgets.org/ticket/13915

Quote from: MortenMacFly on November 01, 2012, 07:03:16 AM

Quote from: rickg22 on October 31, 2012, 10:24:42 PM
But if I'm correct, your fix will introduce exactly the same slowdown that was fixed in the other part of the code.

I believe the opposite... but lets see... ;-)

14 seconds. :-/

Quote from: rickg22 on November 01, 2012, 07:56:32 AM
14 seconds. :-/

Dammed. But I don't get it: According Jens and dmoore this patch:
http://forums.next.codeblocks.org/index.php/topic,16967.msg115619.html#msg115619
Did the trick. And this is not in the method we are touching now. :-\

Can I make a proposal: Shall we first commit what we have because it fixes the major bugs in file handling in project files. Then we think about what to do to speed-up things. The thing is I would like to freeze a working state as a reference before we fiddle with it again...

OK, I agree. Let's freeze this for now, and then work on it.

Quote from: rickg22 on November 01, 2012, 08:10:11 AM
OK, I agree. Let's freeze this for now, and then work on it.

OK - anybody else disagree (Jens, dmoore?!). Did somebody do some more testing? Results?

Quote from: MortenMacFly on November 01, 2012, 05:24:57 PM

Quote from: rickg22 on November 01, 2012, 08:10:11 AM
OK, I agree. Let's freeze this for now, and then work on it.

OK - anybody else disagree (Jens, dmoore?!). Did somebody do some more testing? Results?

No objections from my side.
I did not do any further tests, but I try to do it soon.

Fine with me. I won't have time to test anything new for at least several days.

GOOD NEWS EVERYONE!

I'm very close to fixing the bug!

I made a relative path cache: If a file is in an already-tested directory, its relative path is calculated based on its directory's relative path. When a file is not found in the cache, its paths are calculated using MortenMacFly's slow-but-sure path calculation via wxFileName.

Here are the preliminary results:

WITHOUT THE PATH CACHE:

C:\projects\cb\src\sdk\cbproject.cpp::void cbProject::CalculateCommonTopLevelPath():394  took : 50 ms
Project's common toplevel path: C:\htdocs\SIG\SGPG\
C:\projects\cb\src\sdk\cbproject.cpp::void cbProject::CalculateCommonTopLevelPath():460  took : 13496 ms
C:\projects\cb\src\sdk\cbproject.cpp::void cbProject::CalculateCommonTopLevelPath():461: Num Files: 1927; Num Cache Misses: 1927


WITH THE PATH CACHE:

C:\projects\cb\src\sdk\cbproject.cpp::void cbProject::CalculateCommonTopLevelPath():404  took : 55 ms
Project's common toplevel path: C:\htdocs\SIG\SGPG\
C:\projects\cb\src\sdk\cbproject.cpp::void cbProject::CalculateCommonTopLevelPath():464  took : 2437 ms
C:\projects\cb\src\sdk\cbproject.cpp::void cbProject::CalculateCommonTopLevelPath():465: Num Files: 1927; Num Cache Misses: 318


Now, since the files in the project are not in order (because we're using a HashMap and not an ordered map), many directories still trigger cache misses. Even so, execution time has disminished by 80%.

I was able to diminish the load time by a further 2% by aditionally checking the parent directory in the cache; Then again, the randomized file order doesn't help. So I'm rewriting the algorithm by using a cleaned-up recursive version of the cache so that we'll have to use wxFileName's functions only twice: Once for the working directory, and once for the project's common top level path; the rest will be mere string comparisons and concatenations. I'll keep you updated when I get the algorithm working.

UPDATE:

I finished polishing the Path Cache. There are some edge cases I haven't tested, mainly at root directories or the top directories of other drives, both for filenames and for the project file.

For standard projects with all files under the project file's directory, things work Okay.

So... ladies and gentlemen... drum rolls, please...


Loading project files...
1927 files loaded
Done loading project in 2042ms
Project's base path: C:\htdocs\SIG\SGPG\
C:\projects\cb\src\sdk\cbproject.cpp::void cbProject::CalculateCommonTopLevelPath():530  took : 50 ms
Project's common toplevel path: C:\htdocs\SIG\SGPG\
C:\projects\cb\src\sdk\cbproject.cpp::void cbProject::CalculateCommonTopLevelPath():570  took : 120 ms
C:\projects\cb\src\sdk\cbproject.cpp::void cbProject::CalculateCommonTopLevelPath():571: Num Files: 1927; Num Cache Misses: 0


Ta-da!  8)

I'm attaching the patch. It needs some styling (you know, braces, tabs, etc), but I hope you like it.

Attached is an untested refinement to Morten's patch that uses the old fast method in the more normal case where files are below the project dir, and uses MakeRelativeTo otherwise. This could be combined with Rick's patch to reduce the amount of cache usage.

Here's the changed code:


        if (   (prjHasUNCName && fileHasUNCName)
            || (   !prjHasUNCName
                && !fileHasUNCName
                && vol.IsSameAs(f->file.GetVolume()) ) )
        {
            if (!fileName.StartsWith(m_CommonTopLevelPath))
            {
                wxFileName relFileCTLP(f->file);
                relFileCTLP.MakeRelativeTo( m_CommonTopLevelPath );
                f->relativeToCommonTopLevelPath = relFileCTLP.GetFullPath();
            }
            else
                f->relativeToCommonTopLevelPath = fileName.Right(fileName.Length() - m_CommonTopLevelPath.Length());

            if (!fileName.StartsWith(GetBasePath()))
            {
                wxFileName relFileBase(f->file);
                relFileBase.MakeRelativeTo( GetBasePath() );
                f->relativeFilename             = relFileBase.GetFullPath();
            }
            else
                f->relativeFilename = fileName.Right(fileName.Length() - GetBasePath().Length());
        }


I am a little bit confused by this:


+            // The commented (old) method to obtain the relativeToCommonTopLevelPath is fast, but does *not* work, if you save
+            // the project on a different drive in a sub-folder of an existing source file on that (different) drive:
+            // I.e.: Project on C:\Folder\Project.cbp has file C:\Folder\SubFolder\foo.cpp and D:\Folder\bar.cpp
+            // Saved the project under D:\Folder\SubFolder\ProjectNew.cbp would cause a wrong computation of bar.cpp otherwise!!


In this case should D:\Folder be the new common top level path? I'm not sure why the old method doesn't work unless the common top level path isn't being calculated correctly. Maybe the problem is really that f->relativeFilename isn't being calculated correctly?

Quote from: dmoore on November 03, 2012, 02:29:35 PM
Attached is an untested refinement to Morten's patch that uses the old fast method in the more normal case where files are below the project dir, and uses MakeRelativeTo otherwise. This could be combined with Rick's patch to reduce the amount of cache usage.

Here's the changed code:

Code Select Expand


...


Hmm... maybe that code will make my patch redundant, but I like having a separate cache for relative paths. It could become useful later, and it does separate all the path checking.

Quote
In this case should D:\Folder be the new common top level path? I'm not sure why the old method doesn't work unless the common top level path isn't being calculated correctly. Maybe the problem is really that f->relativeFilename isn't being calculated correctly?

Perhaps. Anyway I got another idea to speed up project loading, I'll post it in the next reply.

OK guys, I found another bottleneck in project loading!


ProjectFile* cbProject::AddFile(int targetIndex, const wxString& filename, bool compile, bool link, unsigned short int /*weight*/)
{
...
if (!wxFileExists(fullFilename))
pf->SetFileState(fvsMissing);
else if (!wxFile::Access(fullFilename.c_str(), wxFile::write)) // readonly
pf->SetFileState(fvsReadOnly);


In other words, C::B checks for the existence of ALL the project files on load! Just to set the little "broken" or readonly flags on the project tree.  Imagine doing this for a ten-thousand-files java project where you only got to modify the configuration -_-.

I changed it to this:


namespace cbProjectAuxVariables {
bool check_for_existence_of_files = true;
};

...

ProjectFile* cbProject::AddFile(int targetIndex, const wxString& filename, bool compile, bool link, unsigned short int /*weight*/)
{
...

// Check for the file's status if the User chose to in the Environment settings dialog;
// or if the file is being added manually and not as part of the project loading.
   if(!m_CurrentlyLoading || cbProjectAuxVariables::check_for_existence_of_files) {
if (!wxFileExists(fullFilename))
pf->SetFileState(fvsMissing);
else if (!wxFile::Access(fullFilename.c_str(), wxFile::write)) // readonly
pf->SetFileState(fvsReadOnly);
   }



cbProjectAuxVariables::check_for_existence_of_files is modified on project load by checking a new Environment setting:


<object class="sizeritem">
<object class="wxCheckBox" name="chkMissingFilesOnLoad">
<label>Check for missing files on project load</label>
<tooltip>When checked, Code::Blocks will check that all the project files exist,&#x0A;and update their status on the Project Tree.&#x0A;Useful at times, but slower for very large projects.</tooltip>
<checked>1</checked>
</object>
<flag>wxTOP|wxLEFT|wxEXPAND|wxALIGN_LEFT|wxALIGN_TOP</flag>
<border>8</border>
</object>


And here's the result, with my previous patch and the file checking on load disabled:


1927 files loaded
Done loading project in 365ms
Project's base path: C:\htdocs\SIG\SGPG\
C:\projects\cb\src\sdk\cbproject.cpp::void cbProject::CalculateCommonTopLevelPath():539  took : 50 ms
Project's common toplevel path: C:\htdocs\SIG\SGPG\
C:\projects\cb\src\sdk\cbproject.cpp::void cbProject::CalculateCommonTopLevelPath():579  took : 130 ms
C:\projects\cb\src\sdk\cbproject.cpp::void cbProject::CalculateCommonTopLevelPath():580: Num Files: 1927; Num Cache Misses: 0


(Total project loading time: 2.50s)  8) ;D 8) ;D 8)

I'm attaching the full patch, including my previous changes. Enjoy.

[attachment deleted by admin]

Quote from: rickg22 on November 03, 2012, 03:33:25 PM
Hmm... maybe that code will make my patch redundant, but I like having a separate cache for relative paths. It could become useful later, and it does separate all the path checking.

Even if we do use the cache, it would make sense to use the logic to create the path (and add it to the cache) when there are cache missses to reduce the number of MakeRelativeTo calls.

Quote from: rickg22 on November 03, 2012, 03:40:04 PM
OK guys, I found another bottleneck in project loading!

Another good catch. But instead of having an environment setting, why not make this a project setting and/or offer an annoying dialog for projects with a file count above a certain size to disable the check for the project? In general, I would think that if you have one big project open and some smaller ones open, you would still want the checks on the smaller projects.

Actually, I was thinking on moving the whole file checking stuff to an event triggered by opening a node in the project tree. In the meantime, I think that having a user-modifiable setting will suffice. Users accustomed to having small projects won't bother changing the setting (which defaults to 'always check'), and users accustomed to large projects are free to trade between load speed and file checking.

We could also add a treshold number to define small and large projects (i.e. 100, 200 files?). In any case, at least now we have a choice. We can refine it later in further commits.

Quote from: dmoore on November 03, 2012, 06:31:22 PM

Even if we do use the cache, it would make sense to use the logic to create the path (and add it to the cache) when there are cache missses to reduce the number of MakeRelativeTo calls.

Yes, that's exactly what the cache does. Everytime it doesn't find a path, it creates it in memory and adds it to the list; (with the exception of files outside the base path i.e. the Common Top Level Path, because otherwise we could run in the risk of adding a parent path to the whole tree; actually, I think that extra check is unnecessary; we could cache external paths without problem, but we'd need to do some tests after disabling that check.)

Quote from: rickg22 on November 03, 2012, 06:51:38 PM

Quote from: dmoore on November 03, 2012, 06:31:22 PM

Even if we do use the cache, it would make sense to use the logic to create the path (and add it to the cache) when there are cache missses to reduce the number of MakeRelativeTo calls.

Yes, that's exactly what the cache does.

Sorry, wasn't clear. What I meant was whenever you have a cache miss, instead of using MakeRelativeTo in all cases, use the faster method when possible.

Quote from: dmoore on November 03, 2012, 07:12:49 PM

Sorry, wasn't clear. What I meant was whenever you have a cache miss, instead of using MakeRelativeTo in all cases, use the faster method when possible.

Ah, my apologies. I forgot to explain how my Cache works, and what I consider a "cache miss".

It's not really a full path cache; it's a PARENT PATH Cache. And it's recursive :D

Let's say that your filename has a full path + name like this:

C:\myProjects\myProject1\src\plugins\myplugin\includes\myheader.h

What my cache does is searching first for the parent path, and if it doesn't find it, go to its own parent path, and so on, like this:


C:\myProjects\myProject1\src\plugins\myplugin\includes
C:\myProjects\myProject1\src\plugins\myplugin
C:\myProjects\myProject1\src\plugins
C:\myProjects\myProject1\src
C:\myProjects\myProject1


At that point, C:\myProjects\myProject1 is actually our project common top level path. So it's found (which is not considered a cache miss, because we can construct the relative path without resorting to wxFilename's functions). Then, as the cache returns from its call stack, adds the items in the reverse order in which they were searched:


C:\myProjects\myProject1\src => src
C:\myProjects\myProject1\src\plugins => src\plugins
C:\myProjects\myProject1\src\plugins\myplugin => src\plugins\myplugin
C:\myProjects\myProject1\src\plugins\myplugin\includes => src\plugins\myplugin\includes
C:\myProjects\myProject1\src\plugins\myplugin\includes\myheader.h => src\plugins\myplugin\includes\myheader.h (not added to the cache, just calculated)


The actual filename, C:\myProjects\myProject1\src\plugins\myplugin\includes\myheader.h is never added to the cache, because it will only be requested ONCE. However,


C:\myProjects\myProject1\src\plugins\myplugin\includes\myheader2.h
C:\myProjects\myProject1\src\plugins\myplugin\includes\myheader3.h
C:\myProjects\myProject1\src\plugins\myplugin\includes\myheader4.h


WILL be requested. Their common denominator is their path, which is already cached. And so are their parent directories.

This means that at most, ONE cache miss will be present: The project's common top level path itself, but that's already added by default!  8)

The real cache misses will be files outside the project's common top level path, which require using wxFilename anyway, so they're not added to the cache. BUT I might add them, I just wasn't sure if adding them would trigger some misbehavior in the cache...

I just realized something.

With minor modifications to my cache's constructor, we can add all the parent paths from the project's CTLP to the root directory, and we will cover ALL cases!


C:\projects\myProject1 => ''
C:\projects => '..'
C:\ => '..\..'


which means that


C:\somepathoutside\someotherpath\whatever\myexternalfile.cpp => ..\..\somepathoutside\someotherpath\whatever\myexternalfile.cpp

At this point, using wxFilename won't be necessary because it's already certain that the file we're searching for and our project reside in the same unit (that's the big "if" that was added in the latest SVN, for cases with different units, the absolute paths are added verbatim). And since they're in the same unit, we have no cache misses AT ALL! For ANY FILE, either inside or outside our CTLP.

(The only caveat is if we have a symlink circular reference around... I'd need to think about how to solve that)

OK, I'm adding a *disabled* version of the path cache to trunk (to enable it, you have to #define use_path_cache_for_ctlp). I moved the path cache code to a separate file, so the code changes can be more obvious.

I'm also adding the "check for missing files" option.

Should I commit, or just post another patch? The code's ready.

Quote from: rickg22 on November 04, 2012, 02:32:49 AM
Should I commit, or just post another patch? The code's ready.

It is tested against UNC path's again and also against different project tree layout options?

BTW: I had a few days off and I am trying to catch up - what's with the other patches in the other posts of this thread? Does this one cover all?

Oh - and btw: I don't know if I personally would sacrifice the read-only flag for speed. When working with SVN controlled projects its often vital to see if a file is write-protected (i.e. locked). So I would definitely like to see a global option to be able to always have this check on in one place.

Quote from: MortenMacFly on November 04, 2012, 05:22:17 PM
It is tested against UNC path's again and also against different project tree layout options?

My code is magically sealed inside an #ifdef; unless you summon it with the define above, your code will be the one running.

Anyway, you've been the most thorough tester so far. Can you provide us with a test suit (zipped .cbp's and installation instructions) so that we can easily test these modifications?

Quote
BTW: I had a few days off and I am trying to catch up - what's with the other patches in the other posts o[f this thread? Does this one cover all?

All my patches? Yes. It's built against rev 8502.

Quote
Oh - and btw: I don't know if I personally would sacrifice the read-only flag for speed. When working with SVN controlled projects its often vital to see if a file is write-protected (i.e. locked). So I would definitely like to see a global option to be able to always have this check on in one place.

Yup, that's how it's coded. One global check. Perhaps we could add an item under the Project submenu with the title "Check for missing or write-protected files", that basically does the same thing. And we could modify the entry in the configuration with a dropdown menu: "Check for missing or readonly files" with the options "Always", "Never", "Only for small projects" (this option would let the user specify a project size). Or who knows, explicitly add it under the Project submenu?

Anyway, my patch is working and using your code, so it's safe to put in trunk.

Quote from: rickg22 on November 04, 2012, 05:48:24 PM
Anyway, you've been the most thorough tester so far. Can you provide us with a test suit (zipped .cbp's and installation instructions) so that we can easily test these modifications?

I've explained and enumerated it already in a previous message in this thread. There is no test suite - sorry. But I can do, if I find the time. For now I just have applied the patch to see...

Concerning that, I've spotted two issues:
- The config option is mis-spelled "mising" instead of "missing" (in all three places).
- It does not compile under wx2.9 because you are using the obsoleted (and wrong) c_str() method of wxString. You'll need to change it to wx_str().

Quote from: rickg22 on November 04, 2012, 05:48:24 PM
Yup, that's how it's coded. One global check.

That's good to hear. :-) As I said - I've applied it in my local copy... lets see if it nukes all my HDD. ;D

So if Rick's patch is on ice for a bit, why don't you also take my patch from a few posts back, which should take care of the slowdown for projects with normal layouts. (Not as elegantly as Rick's)

Quote from: dmoore on November 07, 2012, 01:16:13 PM
So if Rick's patch is on ice for a bit,

I wouldn't say its on ice. I think Rick can commit as the changes in question are indeed hidden with a #define for power users and further testing.
I can take care of the compilation stuff in wx29/64 bit (which I have done in my local copy already).

The only thing I am kind of "waiting" for is that somebody tells me it still works with UNC path's and so on... But I'm afraid I have to see myself as I am surrounded by Linux devs... huh! :'(

Quote from: MortenMacFly on November 07, 2012, 01:34:26 PM

Quote from: dmoore on November 07, 2012, 01:16:13 PM
So if Rick's patch is on ice for a bit,

I wouldn't say its on ice. I think Rick can commit as the changes in question are indeed hidden with a #define for power users and further testing.
I can take care of the compilation stuff in wx29/64 bit (which I have done in my local copy already).

The only thing I am kind of "waiting" for is that somebody tells me it still works with UNC path's and so on... But I'm afraid I have to see myself as I am surrounded by Linux devs... huh! :'(

I regularly use windows... but at work where I have very limited time... and the servers don't have accessible UNC paths.

Quote from: dmoore on November 07, 2012, 04:01:39 PM
I regularly use windows... but at work where I have very limited time... and the servers don't have accessible UNC paths.

Usually even \\localhost\foo or \\127.0.0.1\foo should work if you are able to create a share on your local PC (just for testing). Another option is the administrative share \\localhost\c$, but this is not easily accessible with Windows after Windows XP.

Quote from: MortenMacFly on November 07, 2012, 04:04:35 PM
Another option is the administrative share \\localhost\c$, but this is not easily accessible with Windows after Windows XP.

That appears to work fine. Thanks for the tip! Will test Rick's stuff when I can.

Should add that I think Rick's patch probably does need to be tested on Linux as well, esp. because he replaces a bunch of wxFileName calls. Also has anyone tested how any of these recent changes work with symlinks? (Not that I'm sure how it worked before)

I've been learning about test-driven development recently.... I think we can effectively build a test suite together:

a) Get a dev with *nix to open a C::B project.

b) Add a debugging to print out the result of the conversion, using exclusively the wxFileName:

i.e.


Relative Common Top Level Path: _______
Base Path: ___________

List of paths and filenames:

/somepath/someotherpath/myfilename.ext
/somepath/someotherpath/anotherfilename.ext
...

'/somepath/someotherpath/myfilename.ext (CTLP)' => 'someotherpath/myfilename.ext'
'/somepath/someotherpath (CTLP)' => 'someotherpath'

'/somepath/someotherpath/myfilename.ext (Base Path)' => 'myfilename.ext'
'/athirdpath/myfilename.ext (CTLP)' => '../athirdpath/myfilename.ext'
'/athirdpath' => '../athirdpath'




And so on. Then, we can replicate the wxFilename's conversion for said path by simply storing the results in a string map.

c) Refactor our code so that it will be able to be covered by unit tests.



// This will be our interface for invoking wxFilename::MakeRelativeTo();
// also our interface for our wxFakeFilename so that we can pass it to our Cached Make Relative function.

class cbFilenameConverterInterface {
   abstract void setBasePath(const wxString& basePath);
   abstract wxString MakeRelative(const wxString& path1, const wxString& path2);
   virtual void setFallBack(cbFilenameConverterInterface* fallback) {} // For cache misses
   virtual void setPathSeparator(wxChar sep) {}
}

// Instead of directly calling wxFilename::MakeRelative, we'll use this.

class cbSlowButSureMakeRelative : public cbFilenameConverterInterface {
   void setBasePath(const wxString& basePath) {} // wxFilename doesn't work this way
   void wxString MakeRelative(const wxString& path1, const wxString& path2) {
       wxFilename filename(path1);
       return filename.makeRelativeTo(path2); // Or whatever
   }
}

class MyPathCache: public cbFilenameConverterInterface {
   ...
   // TODO: I need to modify my Path Cache code to adapt to this interface.
}


d) Add our mock converter for test cases.


class cbMockFilename: public cbFilenameConverterInterface {
   public:    
       void setBasePath(const wxString& basePath);
       wxString MakeRelative(const wxString& path1, const wxString& path2);

       // Use this to insert all the results obtained via wxFilename.
       // I need to modify my path cache to output (in debug) all the times wxFilename is invoked so that we can later insert them manually.
       void insertTestCase(const wxString& basePath, const wxString& path1, const wxString& path2, const wxString& result);
       void clear();
   protected:
       wxString getTestCase(const wxString& basePath, const wxString& path1, const wxString& path2);
       wxString m_BasePath;
       bool m_CaseNotFound; // set or Reset everytime getTestCase is invoked.
       // TODO: Insert here storage objects for our test cases
}

wxString cbMockFilename::MakeRelative(const wxString& path1, const wxString& path2) {
   wxString result = this->getTestCase(this->m_BasePath, path1,path2);
   if(this->m_CaseNotFound { // We should have a list of various basepaths in our test suit
       // ALL test cases should have their correct results covered! (We can't test a function when we don't know what it's supposed to return)
       debug("Error! '%s' -> '%s'  are not covered by our test case!", path1, path2);
       return wxString(_T('*ERROR*'));
   } else {
       return result;
   }
}




e) We have everything ready to test!



(pseudocode)

void MakeRelativeUnitTest(const wxString basePath, vector<wxString>ourFilenames,map<wxString, wxString>ourExpectedResults, const wxChar sep, wxFilenameConverterInterface* func) {
   bool testPassed = true;
   cbFilenameConverterInterface* ourExperimentalObject = new MyPathCache();
   cbFilenameConverterInterface* ourMockObject = new cbMockFilename();
   ourExperimentalObject->setBasePath(basePath);
   ourExperimentalObject->setPathSeparator(sep);
   ourMockObject->setBasePath(basePath);
   ourMockObject->setPathSeparator(sep);

   ourExperimentalObject->setFallBack(ourMockObject); // Needed for files outside the CTLP.

   wxString actualResult, expectedResult;
   for(it = ourFilenames.begin(); it != ourFilenames.end(); ++it) {
       expectedResult = cbMockFilename(*it);
       actualResult = ourExperimentalClass->makeRelative(*it);
       if(actualResult != expectedResult) {
        debug("Test failed for: '%s'! Expected result: '%s'; Actual result: '%s',*itexpectedResult,actualResult);
        testPassed = false;
        break;
    }
   }
   if(testPassed) {
        debug("Test passed! Our Experimental Object is safe to use.");
   }
}


IMHO, we should have done this years ago. So that the next time we get a rare bug, all we have to do is add it to our test cases; this way we can catch regression bugs much sooner.