Use my modified implementation of extractAllFiles, set default chunks…

…ize to 32768, restore benchmarking to a workable state

src/utilities/core/UnzipFile.cpp

@@ -32,7 +32,7 @@
 
 namespace openstudio {
 
-UnzipFile::UnzipFile(const openstudio::path& filename) : m_unzFile(unzOpen(openstudio::toString(filename).c_str())), m_chunksize(1024) {
+UnzipFile::UnzipFile(const openstudio::path& filename) : m_unzFile(unzOpen(openstudio::toString(filename).c_str())), m_chunksize(32768) {
   if (!m_unzFile) {
     if (!openstudio::filesystem::exists(filename)) {
       throw std::runtime_error("UnzipFile " + openstudio::toString(filename) + " does not exist, could not be opened");
@@ -54,24 +54,10 @@ void UnzipFile::setChunksize(unsigned long chunksize) {
 }
 
 std::vector<openstudio::path> UnzipFile::extractAllFiles(const openstudio::path& outputPath) const {
-  std::vector<openstudio::path> files = listFiles();
 
-  std::vector<openstudio::path> retfiles;
+  // We do not call listFiles() then for each extractFile, for performance reasons (some of the work would be done twice). cf #4456
 
-  for (std::vector<openstudio::path>::const_iterator itr = files.begin(); itr != files.end(); ++itr) {
-    if (toString(itr->filename()) == "." || toString(itr->filename()) == "/") {
-      // This is a directory - skip it
-    } else {
-      retfiles.push_back(extractFile(*itr, outputPath));
-    }
-  }
-
-  return retfiles;
-}
-
-std::vector<openstudio::path> UnzipFile::extractAllFilesMod(const openstudio::path& outputPath) const {
-
-  bool cont = unzGoToFirstFile(m_unzFile) == UNZ_OK;
+  bool cont_files = unzGoToFirstFile(m_unzFile) == UNZ_OK;
 
   std::vector<openstudio::path> filesOnDisk;
 
@@ -104,16 +90,16 @@ std::vector<openstudio::path> UnzipFile::extractAllFilesMod(const openstudio::pa
       }
 
       try {
-        bool cont = true;
+        bool cont_read = true;
 
         // Open the target file on disk
         openstudio::filesystem::ofstream file(createdFilePath, std::ios_base::trunc | std::ios_base::binary);
-        while (cont) {
+        while (cont_read) {
 
           int bytesread = unzReadCurrentFile(m_unzFile, &buffer.front(), buffer.size());
 
           if (bytesread == 0) {
-            cont = false;
+            cont_read = false;
           } else if (bytesread < 0) {
             throw std::runtime_error("Unable to read from file: " + openstudio::toString(zippedFileRelPath));
           } else {
@@ -132,8 +118,8 @@ std::vector<openstudio::path> UnzipFile::extractAllFilesMod(const openstudio::pa
       }
     }
 
-    cont = unzGoToNextFile(m_unzFile) == UNZ_OK;
-  } while (cont);
+    cont_files = unzGoToNextFile(m_unzFile) == UNZ_OK;
+  } while (cont_files);
 
   return filesOnDisk;
 }
@@ -147,6 +133,8 @@ openstudio::path UnzipFile::extractFile(const openstudio::path& filename, const
     throw std::runtime_error("Unable to open file in archive: " + openstudio::toString(filename));
   }
 
+  std::vector<char> buffer(m_chunksize);
+
   try {
     bool cont = true;
 
@@ -155,7 +143,6 @@ openstudio::path UnzipFile::extractFile(const openstudio::path& filename, const
 
     openstudio::filesystem::ofstream file(createdFile, std::ios_base::trunc | std::ios_base::binary);
     while (cont) {
-      std::vector<char> buffer(m_chunksize);
       int bytesread = unzReadCurrentFile(m_unzFile, &buffer.front(), buffer.size());
 
       if (bytesread == 0) {
@@ -178,16 +165,18 @@ openstudio::path UnzipFile::extractFile(const openstudio::path& filename, const
   }
 
   unzCloseCurrentFile(m_unzFile);
+  return openstudio::path{};
 }
 
 std::vector<openstudio::path> UnzipFile::listFiles() const {
   bool cont = unzGoToFirstFile(m_unzFile) == UNZ_OK;
 
   std::vector<openstudio::path> paths;
 
+  std::vector<char> filename(300);
+
   do {
     unz_file_info file_info;
-    std::vector<char> filename(300);
 
     unzGetCurrentFileInfo(m_unzFile, &file_info, &filename.front(), filename.size(), nullptr, 0, nullptr, 0);
 

src/utilities/core/UnzipFile.hpp

@@ -56,8 +56,6 @@ class UTILITIES_API UnzipFile
   /// Extracts all files in the archive to the given path, preserving relative paths.
   std::vector<openstudio::path> extractAllFiles(const openstudio::path& outputPath) const;
 
-  std::vector<openstudio::path> extractAllFilesMod(const openstudio::path& outputPath) const;
-
   unsigned long chunksize() const;
   void setChunksize(unsigned long chunksize);
 

src/utilities/core/test/Zip_Benchmark.cpp

@@ -14,7 +14,7 @@ openstudio::path prepareOutDir(const std::string& test_case) {
   return outpath;
 }
 
-static void BM_Current(benchmark::State& state) {
+static void BM_Unzip(benchmark::State& state) {
   openstudio::path p = resourcesPath() / openstudio::toPath("utilities/Zip/test1.zip");
   openstudio::path outpath = prepareOutDir("Current");
 
@@ -28,19 +28,4 @@ static void BM_Current(benchmark::State& state) {
   }
 }
 
-static void BM_Mod(benchmark::State& state) {
-  openstudio::path p = resourcesPath() / openstudio::toPath("utilities/Zip/test1.zip");
-  openstudio::path outpath = prepareOutDir("Mod");
-
-  openstudio::UnzipFile uf(p);
-  uf.setChunksize(state.range(0));
-
-  // Code inside this loop is measured repeatedly
-  for (auto _ : state) {
-    uf.extractAllFilesMod(outpath);
-    openstudio::filesystem::remove_all(outpath);
-  }
-}
-
-BENCHMARK(BM_Current)->Iterations(5)->Unit(benchmark::kMillisecond)->RangeMultiplier(2)->Range(1024, 8 << 13);
-BENCHMARK(BM_Mod)->Iterations(5)->Unit(benchmark::kMillisecond)->RangeMultiplier(2)->Range(1024, 8 << 13);
+BENCHMARK(BM_Unzip)->Unit(benchmark::kMillisecond)->RangeMultiplier(2)->Range(1024, 8 << 13);