stray and unused vectors<wxString> in projectloader.cpp

[frithjofh]

hi everybody,

in svn 12073 file projectloader.cpp I found a unused

Code

std::vector<wxString> files; 

on line 995. It is not used by anything nor assigned anything to it. Was it forgotten there?

Then, in the same file starting from line 1553 I found this code:

Code

    std::vector<wxString> filesThrougGlobs;
    const std::vector<cbProject::Glob>& unitGlobs = m_pProject->GetGlobs();
    for (std::size_t index = 0; index < unitGlobs.size(); ++index)
    {
        const cbProject::Glob& glob = unitGlobs[index];
        if (TiXmlElement* unitsGlobNode = AddElement(prjnode, "UnitsGlob", "directory", glob.m_Path))
        {
            unitsGlobNode->SetAttribute("recursive", glob.m_Recursive ? "1" : "0");
            unitsGlobNode->SetAttribute("wildcard", cbU2C(glob.m_WildCard));
        }
        std::vector<wxString> files = filesInDir(glob.m_Path, glob.m_WildCard, glob.m_Recursive, m_pProject->GetBasePath());
        std::copy(files.begin(), files.end(), std::back_inserter(filesThrougGlobs));

In the second line counting from the end a vector gets declared and initialized. In the direct following last line, this same vector is copied to a up to this moment empty vector which was declared on the first line of the snippet. Is this the remains of something not properly edited out?

Regards

frithjofh

[oBFusCATed]

If you're talking about this line

Code

std::copy(files.begin(), files.end(), std::back_inserter(filesThrougGlobs));

Then filesThrougGlobs might not be empty on the second iteration of the outer loop.
The code looks correct to me.

If we want to make it fast we need to modify filesInDir to directly fill the output array.

[frithjofh]

how about using at least the member function insert() instead? it should beat the copy() in speed by about factor 5 ...

Code

filesThrougGlobs.insert( filesThrougGlobs.end(), files.begin(), files.end() );

ps: and what about the other stray vector at line 995?

[oBFusCATed]

how about using at least the member function insert() instead? it should beat the copy() in speed by about factor 5 ...

Are you sure? Have you really measured it?

[frithjofh]

i wrote a small (maybe somewhat naive) test down these lines:

Code

#include <string>
#include <vector>
#include <algorithm>

int main()
{
    std::vector<std::string> target;
    
    for ( int i = 0; i < 100000; ++i )
    {
            std::vector<std::string> source(500, "catnip");
#if VECTOR_INSERT
            target.insert( target.end(), source.begin(), source.end() );
#else
            std::copy( source.begin(), source.end(), std::back_inserter(target) );
#endif
    }
    
    return 0;
}

compiled and timed the execution:

Code

fri@neptuno:~/Desktop> time ./copy_insert

real    0m3,740s
user    0m2,384s
sys     0m1,308s
fri@neptuno:~/Desktop> time ./insert

real    0m2,467s
user    0m1,031s
sys     0m1,377s

I think maybe my test is too naive...

[oBFusCATed]

Yes, are you sure you're building with optimizations?
If you're it is interesting that the compiler is unable to remove both constructs
In theory both these executables should take zero time.