[zip] Stop calculating crc if it's not checked
Crc calculation shows up in the profiler in 2-5% range, and is
never currently validated. Let's disable it for good.
For a well-compressible test data the difference is even nicer:
Benchmark Time CPU Iteration
------------------------------------------------------------------
ziparchive-benchmarks:
before:
#ExtractEntry/2 1943244 ns 1926758 ns 375
#ExtractEntry/16 1877295 ns 1867049 ns 375
#ExtractEntry/1024 1888772 ns 1879976 ns 373
after:
#ExtractEntry/2 817003 ns 812870 ns 874
#ExtractEntry/16 814029 ns 809813 ns 875
#ExtractEntry/1024 804904 ns 800972 ns 879
Bug: 153392568
Test: atest, manual
Change-Id: I917abecab01301f1d09a5bf3b542d24b3875e359
diff --git a/libziparchive/zip_archive.cc b/libziparchive/zip_archive.cc
index 7658d5c..19f95d4 100644
--- a/libziparchive/zip_archive.cc
+++ b/libziparchive/zip_archive.cc
@@ -59,7 +59,7 @@
// Used to turn on crc checks - verify that the content CRC matches the values
// specified in the local file header and the central directory.
-static const bool kCrcChecksEnabled = false;
+static constexpr bool kCrcChecksEnabled = false;
// The maximum number of bytes to scan backwards for the EOCD start.
static const uint32_t kMaxEOCDSearch = kMaxCommentLen + sizeof(EocdRecord);
@@ -1314,11 +1314,15 @@
if (!writer->Append(&buf[0], block_size)) {
return kIoError;
}
- crc = crc32(crc, &buf[0], block_size);
+ if (crc_out) {
+ crc = crc32(crc, &buf[0], block_size);
+ }
count += block_size;
}
- *crc_out = crc;
+ if (crc_out) {
+ *crc_out = crc;
+ }
return 0;
}
@@ -1331,9 +1335,11 @@
int32_t return_value = -1;
uint64_t crc = 0;
if (method == kCompressStored) {
- return_value = CopyEntryToWriter(handle->mapped_zip, entry, writer, &crc);
+ return_value =
+ CopyEntryToWriter(handle->mapped_zip, entry, writer, kCrcChecksEnabled ? &crc : nullptr);
} else if (method == kCompressDeflated) {
- return_value = InflateEntryToWriter(handle->mapped_zip, entry, writer, &crc);
+ return_value =
+ InflateEntryToWriter(handle->mapped_zip, entry, writer, kCrcChecksEnabled ? &crc : nullptr);
}
if (!return_value && entry->has_data_descriptor) {
diff --git a/libziparchive/zip_archive_benchmark.cpp b/libziparchive/zip_archive_benchmark.cpp
index 09d3b8a..cfa5912 100644
--- a/libziparchive/zip_archive_benchmark.cpp
+++ b/libziparchive/zip_archive_benchmark.cpp
@@ -17,7 +17,6 @@
#include <cstdio>
#include <cstdlib>
#include <cstring>
-#include <iostream>
#include <string>
#include <tuple>
#include <vector>
@@ -28,17 +27,20 @@
#include <ziparchive/zip_archive_stream_entry.h>
#include <ziparchive/zip_writer.h>
-static TemporaryFile* CreateZip() {
- TemporaryFile* result = new TemporaryFile;
+static std::unique_ptr<TemporaryFile> CreateZip(int size = 4, int count = 1000) {
+ auto result = std::make_unique<TemporaryFile>();
FILE* fp = fdopen(result->fd, "w");
ZipWriter writer(fp);
std::string lastName = "file";
- for (size_t i = 0; i < 1000; i++) {
+ for (size_t i = 0; i < count; i++) {
// Make file names longer and longer.
lastName = lastName + std::to_string(i);
writer.StartEntry(lastName.c_str(), ZipWriter::kCompress);
- writer.WriteBytes("helo", 4);
+ while (size > 0) {
+ writer.WriteBytes("helo", 4);
+ size -= 4;
+ }
writer.FinishEntry();
}
writer.Finish();
@@ -106,5 +108,28 @@
}
BENCHMARK(StartAlignedEntry)->Arg(2)->Arg(16)->Arg(1024)->Arg(4096);
+static void ExtractEntry(benchmark::State& state) {
+ std::unique_ptr<TemporaryFile> temp_file(CreateZip(1024 * 1024, 1));
+
+ ZipArchiveHandle handle;
+ ZipEntry data;
+ if (OpenArchive(temp_file->path, &handle)) {
+ state.SkipWithError("Failed to open archive");
+ }
+ if (FindEntry(handle, "file0", &data)) {
+ state.SkipWithError("Failed to find archive entry");
+ }
+
+ std::vector<uint8_t> buffer(1024 * 1024);
+ for (auto _ : state) {
+ if (ExtractToMemory(handle, &data, buffer.data(), uint32_t(buffer.size()))) {
+ state.SkipWithError("Failed to extract archive entry");
+ break;
+ }
+ }
+ CloseArchive(handle);
+}
+
+BENCHMARK(ExtractEntry)->Arg(2)->Arg(16)->Arg(1024);
BENCHMARK_MAIN();