ali ahmadi
ali ahmadi

Reputation: 189

IdFTP performance issue when Parsing all the files in a directory and subdirectories

I need to parse every single file in a directory including the files in sub directories and sub sub directories and ...
I have already successfully done this using the code below :

class function TATDFTPUtility.findAllDirectoryFiles(var ftpClient: TIdFTP; directory: String; deepness: Integer = 0): TidFTPListItems;
var
  I: Integer;
  localDirectoryListing: TIdFTPListItems;
  baseDirectory: string;
begin
  Result := TIdFTPListItems.Create;
  *// this function uses ftpClient.ChangeDirUp until it reaches the '' directory*
  changeUpToDirectory(ftpClient, '');
  try
    ftpClient.ChangeDir(directory);
    ftpClient.List;
    Result.Assign(ftpClient.DirectoryListing);
    localDirectoryListing := Result;
    baseDirectory := ftpClient.RetrieveCurrentDir;
    for I := 0 to localDirectoryListing.Count - 1 do
    begin
      if (localDirectoryListing.Items[i].ItemType = ditDirectory) then
      begin
        result := addTwoFTPListItems(result, findAllDirectoryFiles(ftpClient, baseDirectory + '/' + localDirectoryListing.Items[i].FileName));
      end;
    end;
  except
  end;
end;

class function TATDFTPUtility.addTwoFTPListItems(listA: TIdFTPListItems; listB: TIdFTPListItems): TidFTPListItems;
var
  i: integer;
begin
  Result := listA;
  for I := 0 to listB.Count - 1 do
  begin
    with Result.Add do
    begin
      Data := listB.Items[i].data;
      Size := listB.Items[i].Size;
      ModifiedDate := listB.Items[i].ModifiedDate;
      LocalFileName := listB.Items[i].LocalFileName;
      FileName := listB.Items[i].FileName;
      ItemType := listB.Items[i].ItemType;
      SizeAvail := listB.Items[i].SizeAvail;
      ModifiedAvail := listB.Items[i].ModifiedAvail;
      PermissionDisplay := listB.Items[i].PermissionDisplay;
    end;
  end;
end;

Now the problem is that this takes about 15-20 minutes !!! Is there a more efficient way ?
Here is a few fact about this particular case :
1- After i ran the program it found about 12000 files with almost 100-200 directories but the highest deepness was about 7
2- I only need to parse and i do not need to download or upload anything
3- The reason i have used an exception is because inside the FTP there are a few folder which i do not have access and this causes an access violation Error in IdFTP and i used try...except to ignore any directory which can not be accessed.

Upvotes: 2

Views: 1224

Answers (1)

Remy Lebeau
Remy Lebeau

Reputation: 597051

You are calling ChangeDirUp() (potentially many times?) and then calling ChangeDir() afterwards. If directory is an absolute path, you can just call ChangeDir() one time to jump directly to the target folder and avoid ChangeDirUp() altogether. The recursive loop inside of findAllDirectoryFiles() is using absolute paths from RetrieveCurrentDir(), so the repeated calls to ChangeDirUp() and ChangeDir() are wasted overhead. You can greatly reduce overhead by not navigating up and down the folder tree unnecessarily.

findAllDirectoryFiles() is returning a newly allocated TIdFTPListItems that the caller must free. That in itself is generally a bad design choice, but especially in this case because the recursive loop is not freeing those secondary TIdFTPListItems objects at all, so they are being leaked.

When adding files to the output TIdFTPListItems, you are only adding their filenames and not their paths. What good is recursively searching for files if the caller does not know where each file was found? Or do you only care about the filenames and not the paths?

You are ignoring the deepness parameter completely.

With that said, try something more like this instead:

class procedure TATDFTPUtility.findAllDirectoryFiles(ftpClient: TIdFTP; const directory: String;var files: TIdFTPListItems; deepness: Integer = -1);
var
  I: Integer;
  baseDirectory: string;
  subDirectories: TStringList;
  item: TIdFTPListItem;
  localDirectoryListing: TIdFTPListItems;
begin
  try
    if directory <> '' then
      ftpClient.ChangeDir(directory);
    ftpClient.List;
  except
    Exit;
  end;
  baseDirectory := ftpClient.RetrieveCurrentDir;
  localDirectoryListing := ftpClient.DirectoryListing;
  subDirectories := nil;
  try
    for I := 0 to localDirectoryListing.Count - 1 do
    begin
      case localDirectoryListing[i].ItemType of
        ditFile: begin
          item := files.Add;
          item.Assign(localDirectoryListing[i]);
          // if you need the full path of each file...
          item.FileName := baseDirectory + '/' + item.FileName;
        end;
        ditDirectory: begin
          item := localDirectoryListing[i];
          if ((item.FileName <> '.') and (item.FileName <> '..')) and
             ((deepness = -1) or (deepness > 0)) then
          begin
            if subDirectories = nil then
              subDirectories := TStringList.Create;
            subDirectories.Add(baseDirectory + '/' + item.FileName);
          end;
        end;
      end;
    end;
    if subDirectories <> nil then
    begin
      if (deepness > 0) then Dec(deepness);
      for I := 0 to subDirectories.Count - 1 do begin
        findAllDirectoryFiles(ftpClient, subDirectories[I], files, deepness);
      end;
    end;
  finally
    subDirectories.Free;
  end;
end;

When calling findAllDirectoryFiles() for the first time, you can set directory to either:

  • a blank string to start searching in the current directory.

  • a subfolder that is relative to the current directory.

  • an absolute folder that is relative to the server's root.

And set deepness to either

  • -1 for endless recursion

  • >= 0 to specify how deep to recurse.

files := TIdFTPListItems.Create;
try
  TATDFTPUtility.findAllDirectoryFiles(ftpClient, 'desired directory', files, desired deepness);
  // use files as needed...
finally
  files.Free;
end;

Upvotes: 3

Related Questions