Author Topic: Bug Report: [#18755] C::B hangs for 20 seconds while opening large project...  (Read 63236 times)

Offline rickg22

  • Lives here!
  • ****
  • Posts: 2283
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.
« Last Edit: November 04, 2012, 06:30:50 pm by rickg22 »

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
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().

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

  • Developer
  • Lives here!
  • *****
  • Posts: 1576
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)

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
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! :'(
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 dmoore

  • Developer
  • Lives here!
  • *****
  • Posts: 1576
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.

Offline MortenMacFly

  • Administrator
  • Lives here!
  • *****
  • Posts: 9694
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.
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 dmoore

  • Developer
  • Lives here!
  • *****
  • Posts: 1576
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.

Offline dmoore

  • Developer
  • Lives here!
  • *****
  • Posts: 1576
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)

Offline rickg22

  • Lives here!
  • ****
  • Posts: 2283
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.
Code

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.


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

Code
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!

Code

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