5
\$\begingroup\$

I am writing C++ code to enumerate the whole HDD and drive listing. However, it takes more than 15 minutes to complete the disk enumeration of all drives (HDD capacity of 500GB) and compile the response in a binary file.

However, I have a 3rd party executable which gives me the listing of the whole disk in just less than two minutes. Can you please look into my code and suggest some performance improvement techniques?

EnumFiles(CString FolderPath, CString SearchParameter,WIN32_FIND_DATAW *FileInfoData)
{
         CString SearchFile = FolderPath + SearchParameter;
         CString FileName;

        hFile = FindFirstFileW(SearchFile, FileInfoData); //   \\?\C:\*

            if (hFile == INVALID_HANDLE_VALUE)
            {
                // Error
            }
          else
         {
                do
                {
                    FileName = FileInfoData->cFileName; 
                    if (FileInfoData->dwFileAttributes & FILE_ATTRIBUTE_DIRECTORY)
                    {
                        if (! (FileName == L"." || FileName == L".."))
                        {
                                        // Save the Folder Information
                          EnumFiles(FolderPath + FileName +(L"\\"), SearchParameter,FileInfoData);
                        }
                    }
                    else
                   {
                     // Save the File Parameters 
                    }     

              } while (FindNextFileW(hFile, FileInfoData));
}
           FindClose(hFile);
       }
\$\endgroup\$
  • 3
    \$\begingroup\$ The difference between your code and the 3rd party app is they are not recursively scanning the directory structure. They will basically be running threw the directory tables on the disk and dumping the content. Yours is very random access into the disk and will incor lets of seek time. Theres is probably just a linear scan of a few hundred drive sectors (no extra seek time). \$\endgroup\$ – Martin York Jan 31 '14 at 17:32
  • 2
    \$\begingroup\$ Does the code extracted into and quoted in the question take a long time? Or does the code which takes a long time also include code to write output to the binary file? \$\endgroup\$ – ChrisW Jan 31 '14 at 18:12
6
\$\begingroup\$

Can you please look into my code and suggest me some performance improvement techniques.

No -- it doesn't work like that. While there are some obvious things you may catch visually (like observing string comparisons at each iteration) every time you optimize you should start by profiling your execution, then isolating the parts that take the most time, then optimizing, then profiling again.

For example, it's possible that optimizing in this case is not removing or changing something you "do wrong" but splitting your hard drive into chunks and parallelizing the effort.

Either way, first, profile the execution and identify the worst offenders.

\$\endgroup\$
  • 2
    \$\begingroup\$ Hi, please note whether this [type of] answer is acceptable or not as a CR answer is being debated on meta. \$\endgroup\$ – Mathieu Guindon Feb 1 '14 at 2:21
  • 2
    \$\begingroup\$ I think this answer may be off-topic, for the reasons I gave in this answer on meta: although it may be good advice and help the OP, it is not a code review. \$\endgroup\$ – ChrisW Feb 1 '14 at 2:41
6
\$\begingroup\$

You don't show the code to "compile the response in a binary file" so we can't review that. Does your testing show that only the code shown in the OP, without the output, is slow? Reviewing the code you showed ...


I don't know a faster Windows API than FindFirstFileW/FindFirstNextW: my Googling found no asynchronous version of these APIs. However:


This is unnecessary (you're copying the filename into heap-allocated memory) ...

FileName = FileInfoData->cFileName;

... why not use the C-style cFileName string value directly? Reusing memory which already exists is probably faster than anything which allocates new memory buffers.


This could be slightly faster ...

if (! (FileName == L"." || FileName == L".."))

... because it's currently doing two whole-string comparisons; you could use something like this instead ...

if ((FileInfoData->cFileName[0] != L`.`)
    ? true // doesn't start with '.'
    : (FileInfoData->cFileName[1] == 0)
    ? false // is "."
    : (FileInfoData->cFileName[1] != L`.`)
    ? true // doesn't start with ".."
    : (FileInfoData->cFileName[2] != 0) // is not ".."
    )

I usually prefer pass-by-reference instead of pass-by-copying-the-value, for any type of class; so instead of ...

EnumFiles(CString FolderPath, CString SearchParameter,WIN32_FIND_DATAW *FileInfoData) {...}

... this instead ...

EnumFiles(const CString& FolderPath, const CString& SearchParameter,
    WIN32_FIND_DATAW *FileInfoData) {...}

Pass-by-value is likely to be copying string values from one object to another, or at least running code to share the value between two objects.

Alternatively, all (recursive) instances of the EnumFiles function could share a WCHAR filenameBuffer[MAX_FILENAME_LEN] used for FolderPath and for FolderPath + FileName +(L"\\"), and passed between the recursive EnumFiles function instances as a non-const WCHAR* pointer or as a static private variable. That would avoid doing a heap operation (e.g. malloc) for each new filename.

\$\endgroup\$
  • \$\begingroup\$ Regarding "why no instead FileName[0] != L'.'": You will now all of a sudden ignore directories starting with a dot. \$\endgroup\$ – ChrisWue Feb 1 '14 at 10:36
  • \$\begingroup\$ @ChrisWue Thank you, I had forgotten about those. I've edited the answer. \$\endgroup\$ – ChrisW Feb 1 '14 at 11:42

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.