diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 1b930d51d75..d75d3689d4e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -24,7 +24,7 @@ repos: hooks: - id: check-added-large-files # Retain the exact lossless cross-platform performance evidence archives. - exclude: ^docs/benchmarks/((executor-copy-audit|bounded-read-ahead)-20261101|(directory-task-slots|packed-visit-snapshots|bounded-match-results|useful-match-workers|direct-evaluator-workers|derived-content-handoff|immutable-default-catalogs|prepared-diff-configuration|prepared-hash-operands|aggregate-comparison-totals|byte-shingle-representations)-20261102)\.json\.gz$ + exclude: ^docs/benchmarks/((executor-copy-audit|bounded-read-ahead)-20261101|(directory-task-slots|packed-visit-snapshots|bounded-match-results|useful-match-workers|direct-evaluator-workers|derived-content-handoff|immutable-default-catalogs|prepared-diff-configuration|prepared-hash-operands|aggregate-comparison-totals|byte-shingle-representations|archive-input-buffers)-20261102)\.json\.gz$ - id: check-merge-conflict - id: check-yaml # BINARY_TYPES below: archive containers are binary but can START with text (a phar begins with diff --git a/CHANGELOG.md b/CHANGELOG.md index bcc00d1f266..cd7880f0a4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ # 0.11.0 +- Reuse a session-owned archive input buffer when the source supports caller-owned reads, preserving decoder fallbacks and read budgets. + - Avoid repeated word-window string construction in text similarity checks while preserving exact byte-word scores. - Aggregate ordinary comparison side totals without retaining duplicate per-path summary tables; category and effect-sensitive summaries keep their existing behavior. diff --git a/TODO.md b/TODO.md index d7595417859..08419744062 100644 --- a/TODO.md +++ b/TODO.md @@ -446,7 +446,10 @@ EP/JC/P work; candidates are not speedup claims until measured on both platforms caller-inclusive/admission policies. Preserve leaf jobs, explicit CPU/role accounting and JC gates. - [ ] O18: Qualify byte-bounded overlap with backpressure, ordered publication and cancellation; avoid per-batch throwaway threads and nested waits on the same saturated pool. -- [ ] O19: Qualify archive seeking/index/replay and decoder admission independently. Investigate +- [x] O19a: Qualify reusable session-owned archive input buffers on Linux/macOS, retaining short + reads, errors, decoder fallbacks and shared reservations. Preserve all representation trials + and controls in [the qualification report](docs/benchmarks/archive-input-buffers-20261102.md). +- [ ] O19b: Qualify archive seeking/index/replay and decoder admission independently. Investigate native/memory ZIP seeking before concurrency; preserve sequential-source and shared-budget fallbacks. - [ ] O20: Qualify optional bulk metadata by backend capability with precise fallback, while keeping metadata-free traversal free of extra probes and extended-stat work. diff --git a/bazelmod/clang_tidy_targets.bzl b/bazelmod/clang_tidy_targets.bzl index e25667a9326..855fcf18747 100644 --- a/bazelmod/clang_tidy_targets.bzl +++ b/bazelmod/clang_tidy_targets.bzl @@ -9,6 +9,7 @@ keeps the two sets identical. """ CLANG_TIDY_MANUAL_TARGETS = [ + "//tools:archive_source_benchmark", "//xff/cli:globals_benchmark", "//xff/engine:read_accounting_test", "//xff/engine:compare_benchmark", diff --git a/docs/benchmarks/archive-input-buffers-20261102.json.gz b/docs/benchmarks/archive-input-buffers-20261102.json.gz new file mode 100644 index 00000000000..bf7b261b152 Binary files /dev/null and b/docs/benchmarks/archive-input-buffers-20261102.json.gz differ diff --git a/docs/benchmarks/archive-input-buffers-20261102.md b/docs/benchmarks/archive-input-buffers-20261102.md new file mode 100644 index 00000000000..aadd153c57c --- /dev/null +++ b/docs/benchmarks/archive-input-buffers-20261102.md @@ -0,0 +1,90 @@ +# Reusable archive input buffers (2026-11-02) + +## Decision + +Retain a lazily allocated, session-owned `std::array` for sources advertising caller-owned reads. Its allocation is uninitialized; only the returned byte count is exposed to libarchive. A borrowed view reaches the C callback and expires at the next refill. The existing owned-string path remains for decoders without `ReadInto`. Capability changes release the inactive storage so both buffer forms are not retained under one reservation. The session and its existing 64 KiB reservation retain the buffer until the reader is destroyed. + +Reject counts beyond the supplied span before exposing bytes; preserve errors, short reads, EOF, format probes, seeking, member identity and shared budgets. There is no new cache, thread, lock, exception path, dependency, format parser, flag or file-I/O bypass. ZIP member indexing/seeking and decoder concurrency remain separate O19 work. + +A separately measured reusable-string prototype avoids allocations too. The selected owned array removes its nested `resize_and_overwrite` callback and error mutation, with comparable or slightly better pilot timings. The original matrix, duplicate-baseline controls and three-way pilot remain available in the raw artifact. + +## Measurement contracts + +Parent `1a210e61da249e27314dc5fc9a779cba50eb8d72` includes #1059 and the isolated CI repair already merged in #1058. The selected runtime changes only `archive_reader.cc`; tests and the manual benchmark are common to every variant. Both use extension-enabled `//xff/cli:xff_full`, `clang_release`/ThinLTO and recorded compiler/link/source/executable identities. These are paired changes on a fixed dependency set; later dependency updates require fresh qualification. + +Per host: 76 native cases x three sessions over TAR, gzip TAR and ZIP with 10/1,000/10,000 members and 128/4,096-byte payloads, listing versus a literal-content test on the final member, one/three requested workers, and ordinary-file controls. Independent Python-generated fixtures and full stdout/zero exits are checked. Workers are requests, not a claim of newly parallel archive decoding. Linux uses tmpfs and CPU affinity; macOS uses warm APFS. + +Separate in-memory timing has 24 cases x three sessions x two variants, with legacy and caller-owned sources. Each case has nine rounds/repetitions, fastest seven retained and discarded warmup. Confirmation uses eight native cases x five sessions and seven memory cases x three sessions, adding an identical-baseline participant. Timings never overlap builds, lint, sanitizers or allocation probes on their host. A filename-aware formatting gate checks every variant before building. Earlier include ordering produced different binaries, so the complete matrix and confirmation were rerun on the canonical source rather than reusing those timings. + +## Native commands + +Median paired session change; negative means less elapsed time. + +| Fixture / action | CPUs | Linux | macOS | +| :-------------------- | ---: | -----: | -----: | +| tar-10-128 / list | 1 | -1.85% | +0.37% | +| tar-10000-128 / list | 1 | -1.69% | -1.76% | +| tar-10000-4096 / list | 1 | -2.50% | -4.99% | +| tar-10000-4096 / last | 3 | -2.61% | -6.94% | +| zip-10000-4096 / last | 1 | +0.41% | -0.19% | +| tgz-10000-4096 / last | 3 | +0.04% | +0.13% | +| ordinary / list | 1 | -0.07% | -1.09% | +| ordinary / last | 3 | -1.03% | -0.48% | + +## Confirmation and controls + +Candidate and byte-identical repeat each compare directly with baseline; repeat movement is not subtracted. Preserve slower small/ordinary controls as well as heavier TAR gains. ZIP/gzip work may remain decoder/header dominated. + +| Fixture / action | CPUs | Confirm Linux | Repeat Linux | Confirm macOS | Repeat macOS | +| :-------------------- | ---: | ------------: | -----------: | ------------: | -----------: | +| ordinary / last | 3 | +2.43% | +0.56% | +1.18% | +1.12% | +| tar-10000-4096 / list | 1 | -2.26% | +0.05% | -3.55% | -0.12% | +| zip-10000-4096 / last | 1 | +0.89% | +0.18% | +0.18% | +0.10% | +| ordinary / list | 1 | +0.27% | +0.11% | +1.87% | -0.93% | +| tgz-10000-4096 / last | 3 | -0.04% | -0.09% | -0.03% | -0.22% | +| tar-10-128 / list | 1 | -0.08% | +0.70% | +0.02% | +0.38% | +| tar-10000-128 / list | 1 | -0.74% | +0.49% | -0.40% | +0.26% | +| tar-10000-4096 / last | 3 | -1.96% | -0.71% | -5.51% | -0.05% | + +## In-memory callbacks + +Values are microseconds: median session fastest-seven repetition means. Fixture creation is outside the timed iteration. Legacy sources retain their existing owned-read path; a different executable can still move their timings. + +| Source / action / members / bytes | Linux baseline | Linux selected | Linux delta | macOS baseline | macOS selected | macOS delta | +| :-------------------------------- | -------------: | -------------: | ----------: | -------------: | -------------: | ----------: | +| caller-owned/list/10000/4096 | 4,286.44 | 3,997.75 | -6.73% | 3,957.20 | 3,957.09 | -0.00% | +| caller-owned/last/10000/4096 | 3,843.83 | 3,629.33 | -5.58% | 3,768.58 | 3,770.62 | +0.05% | +| caller-owned/list/10/128 | 7.89 | 8.11 | +2.75% | 6.62 | 6.69 | +0.99% | +| legacy/list/10000/4096 | 4,263.61 | 3,956.39 | -7.21% | 3,965.36 | 3,991.04 | +0.65% | +| legacy/last/10000/128 | 2,395.93 | 2,134.93 | -10.89% | 2,980.26 | 2,980.54 | +0.01% | + +## Allocation diagnostics + +Separate warmed calling-thread ASan hooks count requests/bytes only. These are neither release timings nor peak/RSS measurements; fixture setup and reporting are excluded. Session input-buffer allocations no longer scale with chunk count on the caller-owned path; member records still allocate. + +| Case | Host | Baseline requests | Selected requests | Baseline requested bytes | Selected requested bytes | +| :--------------------------- | :---- | ----------------: | ----------------: | -----------------------: | -----------------------: | +| caller-owned/list/10000/4096 | linux | 60,742 | 60,039 | 50,601,064 | 4,574,320 | +| caller-owned/list/10000/4096 | macos | 60,742 | 60,039 | 50,595,400 | 4,574,288 | +| caller-owned/last/10000/4096 | linux | 60,730 | 60,027 | 48,377,040 | 2,350,296 | +| caller-owned/last/10000/4096 | macos | 60,730 | 60,027 | 48,371,352 | 2,350,240 | +| legacy/list/10000/4096 | linux | 60,742 | 60,742 | 50,601,064 | 50,601,072 | +| legacy/list/10000/4096 | macos | 60,742 | 60,742 | 50,595,400 | 50,595,408 | + +## Correctness and footprint + +Nine new behavior cases verify stable storage, short reads, late failures, impossible counts, EOF, unchanged legacy fallback, compression, alternating read capabilities and budget sharing/release. All 324 extension-enabled release suites pass per host, with required Linux FUSE. Four focused ASan suites pass per host; four Linux TSan suites and five Linux MSan suites pass, including the similarity reference test with its corrected timeout. Exact selected-source checks precede final validation. Scoped lint and enforcing hooks remain required; current pushed-head CI determines readiness. + +Observed maximum process RSS for the 10,000-member, 4,096-byte TAR listing across all native samples; this is separate from cumulative allocation requests. + +| Host | Baseline RSS MiB | Selected RSS MiB | +| :---- | ---------------: | ---------------: | +| linux | 13.04 | 13.06 | +| macos | 13.56 | 14.28 | + +- linux: unstripped full CLI 18,118,000 -> 18,121,664 bytes (+3,664). +- macos: unstripped full CLI 8,140,216 -> 8,140,216 bytes (+0). + +## Lossless evidence + +[archive-input-buffers-20261102.json.gz](archive-input-buffers-20261102.json.gz): 5,196,448 bytes; SHA-256 `8ee9b6ccdf8656162ff129fa1440d5ec4826be240a9e800d46e6f5089fcedf93`. diff --git a/docs/performance-analysis.md b/docs/performance-analysis.md index 9caa414031e..e38b7fab15e 100644 --- a/docs/performance-analysis.md +++ b/docs/performance-analysis.md @@ -3455,6 +3455,33 @@ own saturated pool. O08 task storage and O11 warm participation do not establish executor is faster. Global sorting/effects/deferred/archive cases need explicit fallback contracts until independently qualified. +### O19a: reuse archive input storage + +`SourceSession::Read` previously requested a new owned string on each raw 64 KiB callback even +when its cursor supported caller-owned reads. The selected path lazily owns one uninitialized +`std::array` and passes only initialized bytes to libarchive. Its existing shared memory reservation +is unchanged. Sequential decoders without that capability retain their owned-string fallback; +short reads remain distinct from EOF, and impossible counts fail before exposing the buffer. +If a cursor changes its advertised read capability, release the inactive buffer before switching +storage forms so the single reservation does not retain both allocations. + +[The qualification report](benchmarks/archive-input-buffers-20261102.md) keeps native TAR/gzip/ZIP +workloads, in-memory caller-owned and legacy controls, identical-baseline confirmation, allocation +diagnostics, exact source identities and binary sizes separate. The reusable-string prototype and +the simpler owned-array pilot are retained. Removing allocation churn does not remove header, +index or decoder costs, and ordinary/small controls can remain slower. Requested workers do not +mean archive decoding became parallel. + +Format each frozen variant with its actual filename before building. Formatting standard input +without that filename can change main-header ordering; in this experiment it changed the binary +hashes. The canonical matrix and confirmation are rerun rather than transferring old timing claims. +The builder now verifies filename-aware formatting before any variant build, and final correctness +validation checks the exact restored source. Counts from instrumented allocators remain diagnostics, +never release timing or peak-memory evidence. + +O19b random-access capability, ZIP seeking/indexing, nested replay and decoder admission remain +open. This change adds none of them and does not claim constant-time member lookup. + ### O21a: byte-word shingle representations The exact verifier previously built a vector of owning words and another owning string for every diff --git a/extra_modules/archive/BUILD.bazel b/extra_modules/archive/BUILD.bazel index 100b8028b58..028812632b7 100644 --- a/extra_modules/archive/BUILD.bazel +++ b/extra_modules/archive/BUILD.bazel @@ -344,6 +344,24 @@ cc_test( # libarchive's write API, then reads it back through our reader, so it proves the dependency links, # the format/filter detection works, and the header fields map correctly - without committing binary # fixtures. Also pins the two distinct error states (not an archive vs. corrupt archive). +cc_test( + name = "archive_source_buffer_test", + size = "small", + srcs = ["archive_source_buffer_test.cc"], + deps = [ + ":archive_reader_cc", + "@abseil-cpp//absl/status", + "@abseil-cpp//absl/status:statusor", + "@googletest//:gtest", + "@googletest//:gtest_main", + "@libarchive//libarchive", + "@mboworks_mbo//mbo/testing:matchers_cc", + "@mboworks_mbo//mbo/testing:status_cc", + "@xff_extras_api//:read_budget_cc", + "@xff_extras_api//:read_source_cc", + ], +) + cc_test( name = "archive_reader_test", size = "small", diff --git a/extra_modules/archive/archive_reader.cc b/extra_modules/archive/archive_reader.cc index c7d54e9b777..d6fc4b084c9 100644 --- a/extra_modules/archive/archive_reader.cc +++ b/extra_modules/archive/archive_reader.cc @@ -25,6 +25,7 @@ #include #include #include +#include #include #include #include @@ -204,12 +205,31 @@ struct SourceSession { vfs::ReadBudget::Reservation reservation; std::unique_ptr stream; std::string buffer; + std::unique_ptr> read_buffer; std::string prefix; absl::Status error; std::optional size; // Declared last so the reader is released before its callback state. ArchivePtr handle; + // Views remain valid until the next refill or session destruction. + absl::StatusOr Refill() { + if (!stream->SupportsReadInto()) { + read_buffer.reset(); + MBO_ASSIGN_OR_RETURN(buffer, stream->Read(kBlockSize)); + return std::string_view(buffer); + } + if (!read_buffer) { + std::string{}.swap(buffer); + read_buffer = std::make_unique_for_overwrite>(); + } + MBO_ASSIGN_OR_RETURN(const auto count, stream->ReadInto(std::span(*read_buffer))); + if (count > kBlockSize) { + return absl::DataLossError("archive source returned more bytes than its read buffer"); + } + return std::string_view(read_buffer->data(), count); + } + // XFF_ABI_POINTER: XAR requests absolute heap positions through libarchive's C callback. static la_int64_t Seek(struct ::archive*, void* data, la_int64_t offset, int whence) { auto& self = *static_cast(data); @@ -244,17 +264,16 @@ struct SourceSession { // XFF_ABI_POINTER: libarchive's input callback uses opaque state and a borrowed byte buffer. static la_ssize_t Read(struct ::archive*, void* data, const void** buffer) { auto& self = *static_cast(data); - auto result = self.stream->Read(kBlockSize); + auto result = self.Refill(); if (!result.ok()) { self.error = result.status(); return -1; } - self.buffer = std::move(*result); if (self.prefix.size() < 4) { - self.prefix.append(self.buffer, 0, 4 - self.prefix.size()); + self.prefix.append(*result, 0, 4 - self.prefix.size()); } - *buffer = self.buffer.data(); - return static_cast(self.buffer.size()); + *buffer = result->data(); + return static_cast(result->size()); } }; diff --git a/extra_modules/archive/archive_source_buffer_test.cc b/extra_modules/archive/archive_source_buffer_test.cc new file mode 100644 index 00000000000..dea81024cc4 --- /dev/null +++ b/extra_modules/archive/archive_source_buffer_test.cc @@ -0,0 +1,250 @@ +// SPDX-FileCopyrightText: Copyright (c) M. Boerger, the MBO Works authors +// SPDX-License-Identifier: Apache-2.0 + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include + +#include "absl/status/status.h" +#include "absl/status/statusor.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" +#include "mbo/testing/matchers.h" +#include "mbo/testing/status.h" +#include "xff/archive/archive_reader.h" +#include "xff/vfs/read_budget.h" +#include "xff/vfs/read_source.h" + +namespace xff::archive { +namespace { + +using ::mbo::testing::EqualsText; +using ::mbo::testing::IsOkAndHolds; +using ::mbo::testing::StatusIs; +using ::testing::ElementsAre; +using ::testing::Eq; +using ::testing::Field; +using ::testing::Gt; +using ::testing::HasSubstr; +using ::testing::IsEmpty; +using ::testing::IsTrue; + +constexpr std::size_t kReadBlock = 64UZ * 1'024; + +struct ReadOptions { + bool caller_owned = true; + std::size_t chunk = kReadBlock; + std::size_t fail_at = std::numeric_limits::max(); + bool oversized_count = false; + bool alternate_capability = false; +}; + +struct ReadCounts { + std::size_t owned = 0; + std::size_t caller_owned = 0; + std::size_t eof = 0; + bool stable_storage = true; +}; + +class CountingStream final : public vfs::ReadStream { + public: + CountingStream(std::shared_ptr bytes, ReadOptions options, std::shared_ptr counts) + : bytes_(std::move(bytes)), options_(options), counts_(std::move(counts)) {} + + absl::StatusOr Read(std::size_t max_bytes) override { + ++counts_->owned; + ++callbacks_; + if (max_bytes > 4 && offset_ >= options_.fail_at) { + return absl::DataLossError("injected archive source failure"); + } + const auto count = std::min({max_bytes, options_.chunk, bytes_->size() - offset_}); + auto result = bytes_->substr(offset_, count); + offset_ += count; + return result; + } + + bool SupportsReadInto() const noexcept override { + return options_.caller_owned && (!options_.alternate_capability || callbacks_ % 2 == 0); + } + + absl::StatusOr ReadInto(std::span buffer) override { + ++counts_->caller_owned; + ++callbacks_; + // Alternating paths intentionally release the inactive buffer; addresses may change. + if (!options_.alternate_capability) { + if (first_buffer_.empty()) { + first_buffer_ = buffer; + } else { + counts_->stable_storage &= first_buffer_.data() == buffer.data(); + } + } + if (offset_ >= options_.fail_at) { + return absl::DataLossError("injected archive source failure"); + } + if (options_.oversized_count) { + return buffer.size() + 1; + } + const auto count = std::min({buffer.size(), options_.chunk, bytes_->size() - offset_}); + std::ranges::copy(std::string_view(*bytes_).substr(offset_, count), buffer.begin()); + offset_ += count; + counts_->eof += count == 0 ? 1 : 0; + return count; + } + + private: + std::shared_ptr bytes_; + ReadOptions options_; + std::shared_ptr counts_; + std::size_t offset_ = 0; + std::size_t callbacks_ = 0; + std::span first_buffer_; +}; + +class CountingSource final : public vfs::ReadSource { + public: + CountingSource( + std::string bytes, + ReadOptions options, + std::shared_ptr counts, + std::shared_ptr budget = std::make_shared()) + : ReadSource(std::move(budget)), + bytes_(std::make_shared(std::move(bytes))), + options_(options), + counts_(std::move(counts)) {} + + absl::StatusOr> Open() const override { + return std::make_unique(bytes_, options_, counts_); + } + + private: + std::shared_ptr bytes_; + ReadOptions options_; + std::shared_ptr counts_; +}; + +struct ArchiveSourceBufferTest : ::testing::Test { + static std::string MakeTar(std::string_view content, bool compressed = false) { + // XFF_ABI_POINTER: libarchive owns its C writer handle until archive_write_free. + struct ::archive* writer = ::archive_write_new(); + EXPECT_THAT(::archive_write_set_format_pax_restricted(writer), Eq(ARCHIVE_OK)); + if (compressed) { + EXPECT_THAT(::archive_write_add_filter_gzip(writer), Eq(ARCHIVE_OK)); + } + std::string bytes(content.size() + kReadBlock, '\0'); + std::size_t used = 0; + EXPECT_THAT(::archive_write_open_memory(writer, bytes.data(), bytes.size(), &used), Eq(ARCHIVE_OK)); + // XFF_ABI_POINTER: libarchive owns the entry until archive_entry_free. + struct ::archive_entry* entry = ::archive_entry_new(); + ::archive_entry_set_pathname(entry, "data.txt"); + ::archive_entry_set_size(entry, static_cast(content.size())); + ::archive_entry_set_filetype(entry, AE_IFREG); + ::archive_entry_set_perm(entry, 0644); + EXPECT_THAT(::archive_write_header(writer, entry), Eq(ARCHIVE_OK)); + EXPECT_THAT(::archive_write_data(writer, content.data(), content.size()), Eq(content.size())); + ::archive_entry_free(entry); + EXPECT_THAT(::archive_write_close(writer), Eq(ARCHIVE_OK)); + EXPECT_THAT(::archive_write_free(writer), Eq(ARCHIVE_OK)); + bytes.resize(used); + return bytes; + } +}; + +TEST_F(ArchiveSourceBufferTest, ReusesTheCallerOwnedBufferForFullAndShortBlocks) { + const auto counts = std::make_shared(); + const auto source = + std::make_shared(MakeTar(std::string(3 * kReadBlock, 'x')), ReadOptions{}, counts); + EXPECT_THAT(ListMembersOfSource(source), IsOkAndHolds(ElementsAre(Field(&Member::path, EqualsText("data.txt"))))); + EXPECT_THAT(counts->owned, Eq(1)); // Only the independent four-byte format probe owns its result. + EXPECT_THAT(counts->caller_owned, Gt(3)); + EXPECT_THAT(counts->stable_storage, IsTrue()); +} + +TEST_F(ArchiveSourceBufferTest, ShortReadsDoNotMeanEndOfArchiveOrLeakPreviousBytes) { + const std::string content = std::string(20'000, 'x') + "tail"; + const auto counts = std::make_shared(); + const auto source = std::make_shared(MakeTar(content), ReadOptions{.chunk = 37}, counts); + const auto member = MemberReadSource(source, "data.txt"); + EXPECT_THAT(vfs::ReadSourceBytes(*member, content.size()), IsOkAndHolds(EqualsText(content))); + EXPECT_THAT(counts->caller_owned, Gt(500)); + EXPECT_THAT(counts->stable_storage, IsTrue()); +} + +TEST_F(ArchiveSourceBufferTest, UnsupportedCursorsKeepTheOwnedReadFallback) { + const std::string content(100'000, 'x'); + const auto counts = std::make_shared(); + const auto source = + std::make_shared(MakeTar(content), ReadOptions{.caller_owned = false, .chunk = 2'051}, counts); + EXPECT_THAT( + vfs::ReadSourceBytes(*MemberReadSource(source, "data.txt"), content.size()), IsOkAndHolds(EqualsText(content))); + EXPECT_THAT(counts->owned, Gt(40)); + EXPECT_THAT(counts->caller_owned, Eq(0)); +} + +TEST_F(ArchiveSourceBufferTest, AlternatingCapabilitiesRetainContentAndOneSessionReservation) { + const std::string content(3 * kReadBlock, 'x'); + const auto budget = std::make_shared(kReadBlock + 4); + const auto counts = std::make_shared(); + const auto source = + std::make_shared(MakeTar(content), ReadOptions{.alternate_capability = true}, counts, budget); + const auto member = MemberReadSource(source, "data.txt"); + EXPECT_THAT(vfs::ReadSourceBytes(*member, content.size()), IsOkAndHolds(EqualsText(content))); + EXPECT_THAT(counts->owned, Gt(1)); + EXPECT_THAT(counts->caller_owned, Gt(1)); + EXPECT_THAT(budget->MemoryUsed(), Eq(0)); +} + +TEST_F(ArchiveSourceBufferTest, CallerOwnedFailureIsPreservedDuringOpen) { + const auto source = + std::make_shared(MakeTar("content"), ReadOptions{.fail_at = 0}, std::make_shared()); + EXPECT_THAT(ListMembersOfSource(source), StatusIs(absl::StatusCode::kDataLoss, HasSubstr("injected"))); +} + +TEST_F(ArchiveSourceBufferTest, CallerOwnedFailureAfterOpenIsNotReportedAsEndOfFile) { + const auto source = std::make_shared( + MakeTar(std::string(3 * kReadBlock, 'x')), ReadOptions{.fail_at = kReadBlock}, std::make_shared()); + MBO_ASSERT_OK_AND_ASSIGN(auto stream, MemberReadSource(source, "data.txt")->Open()); + EXPECT_THAT(stream->Read(kReadBlock), StatusIs(absl::StatusCode::kDataLoss, HasSubstr("injected"))); +} + +TEST_F(ArchiveSourceBufferTest, RejectsAnImpossibleReadCountBeforeCommittingTheStringSize) { + const auto source = std::make_shared( + MakeTar("content"), ReadOptions{.oversized_count = true}, std::make_shared()); + EXPECT_THAT(ListMembersOfSource(source), StatusIs(absl::StatusCode::kDataLoss, HasSubstr("more bytes"))); + EXPECT_THAT(source->Budget()->MemoryUsed(), Eq(0)); +} + +TEST_F(ArchiveSourceBufferTest, ZeroCountRemainsEofAndEmptySourcesRemainInvalid) { + const auto counts = std::make_shared(); + const auto source = std::make_shared("", ReadOptions{}, counts); + EXPECT_THAT(ListMembersOfSource(source), StatusIs(absl::StatusCode::kInvalidArgument)); + EXPECT_THAT(counts->eof, Gt(0)); + EXPECT_THAT(source->Budget()->MemoryUsed(), Eq(0)); +} + +TEST_F(ArchiveSourceBufferTest, RetainsExactlyOneSessionReservationAndReleasesItWithTheCursor) { + const auto budget = std::make_shared(kReadBlock + 4); + const auto source = std::make_shared( + MakeTar("content", true), ReadOptions{.chunk = 23}, std::make_shared(), budget); + MBO_ASSERT_OK_AND_ASSIGN(auto first, MemberReadSource(source, "data.txt")->Open()); + EXPECT_THAT(first->Read(7), IsOkAndHolds(EqualsText("content"))); + EXPECT_THAT(budget->MemoryUsed(), Eq(kReadBlock)); + EXPECT_THAT(MemberReadSource(source, "data.txt")->Open(), StatusIs(absl::StatusCode::kResourceExhausted)); + first.reset(); + EXPECT_THAT(budget->MemoryUsed(), Eq(0)); + MBO_ASSERT_OK_AND_ASSIGN(auto second, MemberReadSource(source, "data.txt")->Open()); + EXPECT_THAT(second->Read(7), IsOkAndHolds(EqualsText("content"))); + EXPECT_THAT(second->Read(7), IsOkAndHolds(IsEmpty())); + second.reset(); + EXPECT_THAT(budget->MemoryUsed(), Eq(0)); +} + +} // namespace +} // namespace xff::archive diff --git a/release-site.json b/release-site.json index 5cf72cb48de..c1856983779 100644 --- a/release-site.json +++ b/release-site.json @@ -76,7 +76,8 @@ "docs/benchmarks/prepared-diff-configuration-20261102.md": "docs/benchmarks/prepared-diff-configuration-20261102.html", "docs/benchmarks/prepared-hash-operands-20261102.md": "docs/benchmarks/prepared-hash-operands-20261102.html", "docs/benchmarks/aggregate-comparison-totals-20261102.md": "docs/benchmarks/aggregate-comparison-totals-20261102.html", - "docs/benchmarks/byte-shingle-representations-20261102.md": "docs/benchmarks/byte-shingle-representations-20261102.html" + "docs/benchmarks/byte-shingle-representations-20261102.md": "docs/benchmarks/byte-shingle-representations-20261102.html", + "docs/benchmarks/archive-input-buffers-20261102.md": "docs/benchmarks/archive-input-buffers-20261102.html" }, "generated_html": { "XFF.md": { diff --git a/tools/BUILD.bazel b/tools/BUILD.bazel index 864e6608134..8750926ecee 100644 --- a/tools/BUILD.bazel +++ b/tools/BUILD.bazel @@ -23,6 +23,23 @@ load(":compile_sources_test.bzl", "compile_sources_tests") package(default_visibility = ["//visibility:private"]) +cc_binary( + name = "archive_source_benchmark", + srcs = ["archive_source_benchmark.cc"], + tags = [ + "clang-tidy", + "manual", + ], + deps = [ + "@abseil-cpp//absl/status", + "@abseil-cpp//absl/status:statusor", + "@abseil-cpp//absl/strings", + "@com_github_google_benchmark//:benchmark", + "@xff_archive//:archive_reader_cc", + "@xff_extras_api//:read_source_cc", + ], +) + py_library( name = "release_backfill_lib", srcs = ["release_backfill.py"], diff --git a/tools/archive_source_benchmark.cc b/tools/archive_source_benchmark.cc new file mode 100644 index 00000000000..86d90fc7722 --- /dev/null +++ b/tools/archive_source_benchmark.cc @@ -0,0 +1,159 @@ +// SPDX-FileCopyrightText: Copyright (c) M. Boerger, the MBO Works authors +// SPDX-License-Identifier: Apache-2.0 + +// Compare archive input callbacks with caller-owned versus legacy owned storage. +// Fixture creation and source ownership are outside the timed iteration loop. + +#include +#include + +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#include "absl/status/status.h" +#include "absl/status/statusor.h" +#include "absl/strings/str_cat.h" +#include "benchmark/benchmark.h" +#include "xff/archive/archive_reader.h" +#include "xff/vfs/read_source.h" + +namespace xff::archive { +namespace { + +class MemoryCursor final : public vfs::ReadStream { + public: + explicit MemoryCursor(std::shared_ptr bytes) : bytes_(std::move(bytes)) {} + + absl::StatusOr Read(std::size_t max_bytes) override { + auto result = bytes_->substr(offset_, std::min(max_bytes, bytes_->size() - offset_)); + offset_ += result.size(); + return result; + } + + bool SupportsReadInto() const noexcept override { return true; } + + absl::StatusOr ReadInto(std::span buffer) override { + const std::size_t count = std::min(buffer.size(), bytes_->size() - offset_); + std::ranges::copy(std::string_view(*bytes_).substr(offset_, count), buffer.begin()); + offset_ += count; + return count; + } + + private: + std::shared_ptr bytes_; + std::size_t offset_ = 0; +}; + +class CallerOwnedSource final : public vfs::ReadSource { + public: + explicit CallerOwnedSource(std::string bytes) : bytes_(std::make_shared(std::move(bytes))) {} + + absl::StatusOr> Open() const override { + return std::make_unique(bytes_); + } + + private: + std::shared_ptr bytes_; +}; + +absl::StatusOr MakeTar(std::size_t members, std::size_t payload) { + // XFF_ABI_POINTER: libarchive's C writer handle is released before this function returns. + struct ::archive* writer = ::archive_write_new(); + ::archive_write_set_format_pax_restricted(writer); + const std::string content(payload, 'x'); + std::string bytes((members * (payload + 1'024)) + 16'384, '\0'); + std::size_t used = 0; + ::archive_write_open_memory(writer, bytes.data(), bytes.size(), &used); + bool valid = true; + for (std::size_t index = 0; index < members; ++index) { + // XFF_ABI_POINTER: libarchive's C entry is released after each fixture record. + struct ::archive_entry* entry = ::archive_entry_new(); + const std::string name = absl::StrCat("file-", index); + ::archive_entry_set_pathname(entry, name.c_str()); + ::archive_entry_set_size(entry, static_cast(payload)); + ::archive_entry_set_filetype(entry, AE_IFREG); + ::archive_entry_set_perm(entry, 0644); + valid &= ::archive_write_header(writer, entry) == ARCHIVE_OK; + valid &= ::archive_write_data(writer, content.data(), content.size()) == static_cast(payload); + ::archive_entry_free(entry); + } + valid &= ::archive_write_close(writer) == ARCHIVE_OK; + ::archive_write_free(writer); + if (!valid) { + return absl::InternalError("cannot create archive benchmark fixture"); + } + bytes.resize(used); + return bytes; +} + +void Measure(benchmark::State& state, bool caller_owned, bool read_last) { + const auto members = static_cast(state.range(0)); + const auto payload = static_cast(state.range(1)); + auto bytes = MakeTar(members, payload); + if (!bytes.ok()) { + state.SkipWithError(bytes.status().ToString()); + return; + } + const vfs::SharedReadSource source = + caller_owned ? std::make_shared(std::move(*bytes)) : vfs::MemoryReadSource(std::move(*bytes)); + const auto member = MemberReadSource(source, absl::StrCat("file-", members - 1)); + const std::string expected(payload, 'x'); + // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores): benchmark loop controls the measured iterations. + for (const auto unused : state) { + if (read_last) { + auto result = vfs::ReadSourceBytes(*member, payload); + if (!result.ok() || *result != expected) { + state.SkipWithError("archive member content differs from fixture"); + return; + } + benchmark::DoNotOptimize(result); + } else { + auto result = ListMembersOfSource(source); + if (!result.ok() || result->size() != members || std::cmp_not_equal(result->back().size, payload)) { + state.SkipWithError("archive listing differs from fixture"); + return; + } + benchmark::DoNotOptimize(result); + } + } + state.SetItemsProcessed(state.iterations() * static_cast(members)); +} + +void RegisterAll() { + constexpr auto kCounts = std::to_array({10, 1'000, 10'000}); + constexpr auto kPayloads = std::to_array({128, 4'096}); + for (const bool caller_owned : {false, true}) { + for (const bool read_last : {false, true}) { + const std::string name = + absl::StrCat("archive/", caller_owned ? "caller-owned/" : "legacy/", read_last ? "last" : "list"); + // XFF_ABI_POINTER: google-benchmark returns a borrowed mutable registration object. + auto* registration = benchmark::RegisterBenchmark(name, Measure, caller_owned, read_last); + for (const auto count : kCounts) { + for (const auto payload : kPayloads) { + registration->Args({count, payload}); + } + } + registration->Unit(benchmark::kMicrosecond)->UseRealTime(); + } + } +} + +} // namespace +} // namespace xff::archive + +int main(int argc, char** argv) { + benchmark::Initialize(&argc, argv); + xff::archive::RegisterAll(); + if (benchmark::ReportUnrecognizedArguments(argc, argv)) { + return 1; + } + benchmark::RunSpecifiedBenchmarks(); + benchmark::Shutdown(); +}