Delphi’s TZipFile working on a stream

[Lars Fosdal 1792]

Which part is slow?
IsValid, Open, or the loop?

Does the slowness depend on the size of the zip file?

[uligerhardt 18]

45 minutes ago, vhanla said:

 

    var vZipContents := TStringList.Create;
    var zip := TZipFile.Create;
    try
      if TZipFile.IsValid(fZipFilename) then
      begin
        zip.Open(fZipFilename, zmRead);
        for I := Low(zip.FileNames) to High(zip.FileNames) do    
        begin
          vZipContents.Add(zip.FileNames[I]);
        end;
        Memo1.Lines := vZipContents;
      end;
    finally
      zip.Free;
	  vZipContents.Free;
    end;

 

Try storing zip.FileNames in a local variable. 

 

Edit: ... and maybe drop vZipContents and use that local variable instead. 

Edited November 18, 2021 by uligerhardt

[Lajos Juhász 293]

You could easily debug and optimize this code. Calling zip.FileNames in the loop is very unhealthy as it will construct the list for every iteration.

 

var
  lFileNames: TArray<string>;
begin
  var vZipContents := TStringList.Create;
  var Zip := TZipFile.Create;
  try
    if TZipFile.IsValid(fZipfileName) then
    begin
      Zip.Open(fZipfileName, zmRead);
      lFileNames := Zip.FileNames;
      for var I := Low(lFileNames) to High(lFileNames) do
      begin
        vZipContents.Add(lFileNames[I]);
      end;
      Memo1.Lines := vZipContents;
    end;
  finally
    Zip.Free;
    vZipContents.Free;
  end;
end;

*(Edit: of course in this code there is no benefit to use vZipContents it would be better to use Memo1.Lines.BeginUpdate and Memo1.Lines.EndUpdate and insert the file names directly to the memo.)

Edited November 18, 2021 by Lajos Juhász

[Uwe Raabe 2057]

TZipFile.Filenames creates the result array each time on the fly. Caching that in a local variable should speed it up a bit. As an alternative you can use an array iterator:

        for var filename in zip.FileNames do
        begin
          vZipContents.Add(fileName);
        end;

 

[David Heffernan 2345]

The ZIP file headers are parsed and stored when you call Open. So I don't think this should be especially slow. However, it is wasteful to call FileNames repeatedly because it is a property with a getter method that makes a new dynamic array every time you access it.

 

So I'd do it like this

var zip := TZipFile.Create;
try
  zip.Open(fZipFilename, zmRead);
  for var fileName in zip.FileNames do
    Memo1.Lines.Add(fileName);
finally
  zip.Free;
end;

(not sure if the inline var inside the for declaration works, but if not you know what to do!)

 

Although actually in this case you could simply write

var zip := TZipFile.Create;
try
  zip.Open(fZipFilename, zmRead);
  Memo1.Lines.AddStrings(zip.FileNames);
finally
  zip.Free;
end;

 

Edited November 18, 2021 by David Heffernan

[vhanla 2]

14 hours ago, David Heffernan said:

The ZIP file headers are parsed and stored when you call Open. So I don't think this should be especially slow. However, it is wasteful to call FileNames repeatedly because it is a property with a getter method that makes a new dynamic array every time you access it.

 

So I'd do it like this

var zip := TZipFile.Create;
try
  zip.Open(fZipFilename, zmRead);
  for var fileName in zip.FileNames do
    Memo1.Lines.Add(fileName);
finally
  zip.Free;
end;

(not sure if the inline var inside the for declaration works, but if not you know what to do!)

 

Although actually in this case you could simply write

var zip := TZipFile.Create;
try
  zip.Open(fZipFilename, zmRead);
  Memo1.Lines.AddStrings(zip.FileNames);
finally
  zip.Free;
end;

 

Thank you! 😀 AddStrings is way faster.

 

The issue occurred by calling:

zip.Filenames[i]

many times inside that loop because the reason you mentioned, specillay on big zip files which has many files compressed.

 

 

Edited November 18, 2021 by vhanla
Brackets dissapeared after post.