From a6633d7739c7761b06d1b347a0b4c08cdf618143 Mon Sep 17 00:00:00 2001 From: Yurii Zubrytskyi Date: Mon, 17 Jun 2019 15:43:16 -0700 Subject: [PATCH] [zip] Save 1 malloc and memset for each added file in ZipWriter + add a benchmark for the function. This change speeds up the function by about 3%: 910ns->880ns Change-Id: I33c8c31de18d10eb38f109917ecbcbdda45b4034 --- libziparchive/zip_archive_benchmark.cpp | 27 +++++++++++++++++++++++-- libziparchive/zip_writer.cc | 17 +++++++++++++--- 2 files changed, 39 insertions(+), 5 deletions(-) diff --git a/libziparchive/zip_archive_benchmark.cpp b/libziparchive/zip_archive_benchmark.cpp index 23ed40811..09d3b8ad8 100644 --- a/libziparchive/zip_archive_benchmark.cpp +++ b/libziparchive/zip_archive_benchmark.cpp @@ -58,7 +58,7 @@ static void FindEntry_no_match(benchmark::State& state) { std::string_view name("thisFileNameDoesNotExist"); // Start the benchmark. - while (state.KeepRunning()) { + for (auto _ : state) { OpenArchive(temp_file->path, &handle); FindEntry(handle, name, &data); CloseArchive(handle); @@ -73,7 +73,7 @@ static void Iterate_all_files(benchmark::State& state) { ZipEntry data; std::string name; - while (state.KeepRunning()) { + for (auto _ : state) { OpenArchive(temp_file->path, &handle); StartIteration(handle, &iteration_cookie); while (Next(iteration_cookie, &data, &name) == 0) { @@ -84,4 +84,27 @@ static void Iterate_all_files(benchmark::State& state) { } BENCHMARK(Iterate_all_files); +static void StartAlignedEntry(benchmark::State& state) { + TemporaryFile file; + FILE* fp = fdopen(file.fd, "w"); + + ZipWriter writer(fp); + + auto alignment = uint32_t(state.range(0)); + std::string name = "name"; + int counter = 0; + for (auto _ : state) { + writer.StartAlignedEntry(name + std::to_string(counter++), 0, alignment); + state.PauseTiming(); + writer.WriteBytes("hola", 4); + writer.FinishEntry(); + state.ResumeTiming(); + } + + writer.Finish(); + fclose(fp); +} +BENCHMARK(StartAlignedEntry)->Arg(2)->Arg(16)->Arg(1024)->Arg(4096); + + BENCHMARK_MAIN(); diff --git a/libziparchive/zip_writer.cc b/libziparchive/zip_writer.cc index 3c5320941..198154b7e 100644 --- a/libziparchive/zip_writer.cc +++ b/libziparchive/zip_writer.cc @@ -247,13 +247,24 @@ int32_t ZipWriter::StartAlignedEntryWithTime(std::string_view path, size_t flags ExtractTimeAndDate(time, &file_entry.last_mod_time, &file_entry.last_mod_date); off_t offset = current_offset_ + sizeof(LocalFileHeader) + file_entry.path.size(); - std::vector zero_padding; + // prepare a pre-zeroed memory page in case when we need to pad some aligned data. + static constexpr auto kPageSize = 4096; + static constexpr char kSmallZeroPadding[kPageSize] = {}; + // use this buffer if our preallocated one is too small + std::vector zero_padding_big; + const char* zero_padding = nullptr; + if (alignment != 0 && (offset & (alignment - 1))) { // Pad the extra field so the data will be aligned. uint16_t padding = static_cast(alignment - (offset % alignment)); file_entry.padding_length = padding; offset += padding; - zero_padding.resize(padding, 0); + if (padding <= std::size(kSmallZeroPadding)) { + zero_padding = kSmallZeroPadding; + } else { + zero_padding_big.resize(padding, 0); + zero_padding = zero_padding_big.data(); + } } LocalFileHeader header = {}; @@ -269,7 +280,7 @@ int32_t ZipWriter::StartAlignedEntryWithTime(std::string_view path, size_t flags return HandleError(kIoError); } - if (file_entry.padding_length != 0 && fwrite(zero_padding.data(), 1, file_entry.padding_length, + if (file_entry.padding_length != 0 && fwrite(zero_padding, 1, file_entry.padding_length, file_) != file_entry.padding_length) { return HandleError(kIoError); }