Jump to content
dummzeuch

Delphi’s TZipFile working on a stream

Recommended Posts

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

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

Share this post


Link to post
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 by uligerhardt

Share this post


Link to post

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 by Lajos Juhász
  • Thanks 1

Share this post


Link to post

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;

 

  • Thanks 1

Share this post


Link to post

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 by David Heffernan
  • Thanks 1

Share this post


Link to post
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 by vhanla
Brackets dissapeared after post.

Share this post


Link to post

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×