User forums > Using Code::Blocks

Bug Report: [#18755] C::B hangs for 20 seconds while opening large project...

<< < (13/20) > >>

MortenMacFly:

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

--- End quote ---
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.codeblocks.org/index.php/topic,16967.msg116003.html#msg116003

...but we are one step closer!

MortenMacFly:
...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.

MortenMacFly:
...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:

--- Code: ---//            f->relativeToCommonTopLevelPath = fileName.Right(fileName.Length() - m_CommonTopLevelPath.Length());

--- End code ---
...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.

rickg22:

--- Code: ---+        if (   (!prjHasUNCName &&  fileHasUNCName)
+            || ( prjHasUNCName && !fileHasUNCName) )

--- End code ---

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


--- Code: ---if(prjHasUNCName != fileHasUNCName)

--- End code ---

?

Anyway, here's the core of your patch:


--- Code: ---@@ -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;
+        }
+

--- End code ---

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


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

--- End code ---

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...


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

--- End code ---
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?


--- Code: ---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());
}

--- End code ---

rickg22:
Another possibility:


--- Code: ---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());
}

--- End code ---

Approach 2:


--- Code: ---  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());
  }

--- End code ---

Navigation

[0] Message Index

[#] Next page

[*] Previous page

Go to full version