diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 6079492fa9f..d53ad45b713 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|archive-input-buffers)-20261102|digest-and-record-storage-20261103)\.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|(digest-and-record-storage|caller-owned-streams)-20261103)\.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 6a0a76566c7..d8d52105f05 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ # 0.11.0 +- Reuse caller-owned input storage for memory-backed and nested archive-member streams while preserving read errors, sparse content and shared budgets. + - Reduce cached-digest copies and plain/escaped output allocation churn while preserving snapshot lifetimes and exact output bytes. - Reuse a session-owned archive input buffer when the source supports caller-owned reads, preserving decoder fallbacks and read budgets. diff --git a/TODO.md b/TODO.md index eff6ba6e5cf..eda939bc12e 100644 --- a/TODO.md +++ b/TODO.md @@ -451,6 +451,13 @@ EP/JC/P work; candidates are not speedup claims until measured on both platforms 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. +- [x] O19c: Qualify caller-owned reads for immutable memory and archive-member cursors, including + nested containers, short reads, errors, cursor interleaving and budget/owner lifetimes. Keep an + explicitly incapable legacy benchmark control and separately exercise actual memory/member sources. + [Qualification, costs and rejected rewrite](docs/benchmarks/caller-owned-streams-20261103.md) retain all four variants and duplicate-baseline controls. +- [ ] O19d: Qualify direct filling of materialized ranges/results through caller-owned reads, with + exact overflow/limit probes, EOF, changing capabilities and owned fallback. Keep range memory + reservations and replay accounting; do not silently treat a zero-length read as proof of EOF. - [ ] O20: Qualify optional bulk metadata by backend capability with precise fallback, while keeping metadata-free traversal free of extra probes and extended-stat work. - [x] O21a: Qualify staged one-word keys, borrowed normalized shingle windows and an isolated byte diff --git a/docs/benchmarks/caller-owned-streams-20261103.json.gz b/docs/benchmarks/caller-owned-streams-20261103.json.gz new file mode 100644 index 00000000000..a73ee556c34 Binary files /dev/null and b/docs/benchmarks/caller-owned-streams-20261103.json.gz differ diff --git a/docs/benchmarks/caller-owned-streams-20261103.md b/docs/benchmarks/caller-owned-streams-20261103.md new file mode 100644 index 00000000000..510d5b90dfb --- /dev/null +++ b/docs/benchmarks/caller-owned-streams-20261103.md @@ -0,0 +1,105 @@ +# Caller-owned memory and archive-member streams (2026-11-03) + +## Decision + +Expose the existing optional `ReadInto` capability from memory-backed streams and archive-member streams. Caller-owned storage reuses the archive session buffer introduced by O19a; legacy cursors keep their owned-string fallback. Memory cursors copy only initialized source bytes and advance once. Archive members retain the 64 KiB read cap, errors, EOF, sparse holes, cursor position, parent ownership and shared budget. Owned `Read` remains available. + +This removes cumulative allocation churn rather than archive indexing or decoder work. Native gains are modest; nested in-memory workloads benefit more on macOS. Slower ordinary/small controls remain in the tables and raw data. The change does not promise a general archive speedup. Tiny memory sources can request a larger buffer even when the allocation count is unchanged. + +No new flags, external dependencies, threads, locks, exceptions, format parsers, seeking/indexing or host-I/O routes. O19b index/seek/replay/admission and O19d direct final-buffer materialization remain separate. + +## Protocol + +Fixed parent `492c1ef635837571d26c5c45b78f7c6ef1776798` includes the #1065 dependency updates, #1059, O19a and O22a/c1. Four variants isolate baseline, memory-only, member-only and combined runtime changes. Common tests and benchmark support are identical. Every variant is filename-formatted before building. Captured release configuration, compiler hash, link actions, binary hash and sizes accompany the results. + +Per host: 100 native cases x three sessions, each with discarded participant warmup and nine interleaved rounds, fastest seven retained. Independent Python fixtures cover TAR-in-TAR, TAR-in-gzip-TAR and ZIP-in-TAR, flat TAR/gzip/ZIP controls and ordinary files. Both 1/3 requested workers, 10/1,000/10,000 members and 128/4,096-byte payloads are represented. Complete stdout and zero exits are checked. Nested traversal explicitly uses `--archive=all --archive-depth=2`; a missing-depth setup attempt was rejected before accepting any row. + +The 48 in-memory cases x three sessions x four variants separately exercise explicitly incapable legacy cursors, custom caller-owned cursors, actual MemoryReadSource and actual nested MemberReadSource. Fixture setup is outside each timed iteration. Batching amortizes untimed CPU metadata startup; each case retains its own warmup and nine repetitions, fastest seven retained. Linux uses tmpfs/native fixtures and CPU affinity; macOS uses warm APFS without CPU pinning. Requested workers do not imply parallel archive decoding. + +Independent confirmation uses ten native cases x five sessions and nine memory cases x three sessions, with a byte-identical baseline repeat. A supplemental two-small-case check fixes each repetition at 100,000 iterations on the same binaries; it is separate from the time-calibrated populations. Reference-repeat movement is shown directly, never subtracted. No timing overlaps compilation, lint or instrumentation on its host. + +## Native commands + +Median paired session elapsed-time change; negative means faster. + +| Fixture / action | CPUs | Linux initial | Linux confirm | Linux repeat | macOS initial | macOS confirm | macOS repeat | +| :--------------------------- | ---: | ------------: | ------------: | -----------: | ------------: | ------------: | -----------: | +| ordinary / list | 1 | -0.64% | +2.19% | +0.67% | +0.54% | +3.35% | +0.14% | +| tar-in-tar-10000-4096 / list | 1 | -1.18% | -0.60% | +0.70% | -1.07% | -2.03% | -0.05% | +| tar-10000-4096 / list | 1 | +0.19% | +1.81% | +0.56% | -0.65% | +1.06% | -0.11% | +| tar-in-tgz-10000-4096 / last | 3 | -0.30% | -0.33% | -0.00% | -0.77% | -0.57% | +0.13% | +| tar-in-tar-10000-128 / list | 1 | -0.66% | +1.02% | +0.73% | +0.22% | +0.23% | +0.18% | +| zip-in-tar-10000-4096 / last | 1 | -0.68% | +0.52% | +0.31% | +0.00% | +0.07% | -0.08% | +| zip-10-128 / last | 3 | +1.46% | +1.51% | +0.22% | +1.93% | +2.54% | +1.21% | +| tar-in-tar-10-128 / list | 1 | -2.61% | +0.90% | +0.69% | -1.89% | +4.01% | +2.83% | +| ordinary / last | 3 | -0.24% | +3.85% | -0.59% | +0.51% | -0.07% | +2.79% | +| tar-in-tar-10000-4096 / last | 3 | -1.93% | -1.22% | -0.63% | -2.27% | -2.51% | -0.67% | + +## In-memory streams + +Median of paired session fastest-seven changes. Preserve all four variants, including regressions. + +| Source / action / members / bytes | Linux memory | Linux member | Linux combined | macOS memory | macOS member | macOS combined | +| :-------------------------------- | -----------: | -----------: | -------------: | -----------: | -----------: | -------------: | +| legacy/list/10/128 | +1.38% | +8.45% | +3.08% | -0.53% | -1.25% | +0.40% | +| legacy/list/10000/4096 | -1.37% | +6.49% | +1.19% | -0.31% | -0.17% | +0.01% | +| legacy/last/10000/4096 | -2.68% | +9.59% | +0.44% | -2.56% | -0.32% | +0.57% | +| caller-owned/list/10/128 | +2.81% | +8.86% | +3.68% | +0.90% | +1.68% | -2.20% | +| caller-owned/list/10000/4096 | +0.89% | +6.95% | +2.40% | -0.52% | -0.06% | +0.41% | +| caller-owned/last/10000/4096 | -1.08% | +6.28% | +0.91% | +0.29% | +1.53% | +2.01% | +| memory/list/10/128 | +1.11% | +8.23% | +3.12% | +2.58% | -0.08% | +1.56% | +| memory/list/10000/4096 | -1.57% | +8.41% | +1.11% | -0.55% | -0.41% | -0.04% | +| memory/last/10000/4096 | -0.39% | +7.68% | +0.85% | -1.41% | -0.44% | -0.86% | +| nested-memory/list/10/128 | +1.23% | -1.43% | -5.18% | +17.64% | -0.72% | -6.06% | +| nested-memory/list/10000/4096 | +2.69% | +3.74% | -3.33% | -8.25% | -14.58% | -14.81% | +| nested-memory/last/10000/4096 | +0.99% | +1.70% | -3.60% | -39.76% | -38.92% | -42.75% | + +## Confirmation and small-case stability + +Original time-calibrated confirmation and the fixed-iteration check remain separate. Large reference-repeat movement indicates measurement instability and limits attribution of the candidate movement. + +| Case | Phase | Linux combined | Linux repeat | macOS combined | macOS repeat | +| :---------------------------- | :------------------ | -------------: | -----------: | -------------: | -----------: | +| nested-memory/last/10/128 | confirmation-memory | -5.70% | +1.97% | -11.77% | +3.69% | +| caller-owned/list/10000/4096 | confirmation-memory | +2.09% | -0.92% | -1.93% | +0.07% | +| nested-memory/list/10000/128 | confirmation-memory | +1.65% | -1.31% | -7.11% | -2.17% | +| nested-memory/last/10000/4096 | confirmation-memory | -3.06% | +0.36% | -28.74% | +10.30% | +| memory/list/10/128 | confirmation-memory | +4.12% | +0.72% | +30.41% | +19.10% | +| nested-memory/list/10000/4096 | confirmation-memory | -1.78% | +0.18% | -15.28% | +1.09% | +| memory/last/10000/4096 | confirmation-memory | +4.02% | +1.52% | -0.30% | -0.45% | +| memory/list/10000/4096 | confirmation-memory | +1.29% | +0.31% | +0.12% | +0.02% | +| legacy/list/10000/4096 | confirmation-memory | +3.60% | -0.81% | -1.27% | +0.26% | +| memory/list/10/128 | fixed-small | +3.70% | +0.19% | +0.11% | -0.23% | +| legacy/list/10/128 | fixed-small | +3.62% | +0.57% | +0.26% | +0.13% | + +## Rejected owned-read rewrite + +A separate three-variant pilot preserved the original owned member-read implementation and added the caller-owned method independently. It did not consistently improve the controls: Linux large kernel controls became slower than the selected draft, while small macOS controls also moved. Keep one checked decoder path instead of duplicating it. Both hosts retain all 24 native pilot rows and 72 memory pilot runs. The baseline and selected binaries were byte-identical to their original builds; changed control timings therefore also limit causal interpretation of small percentage changes. + +## Allocation diagnostics + +Warmed calling-thread ASan hooks count allocation requests and cumulative requested bytes only. They exclude fixture construction and reporting; they are not release timings, peak memory or RSS savings. Small sources reserve the same existing read budget but may allocate a full 64 KiB buffer instead of a short owned string. + +| Case | Host | Baseline requests | Combined requests | Baseline requested bytes | Combined requested bytes | +| :---------------------------- | :---- | ----------------: | ----------------: | -----------------------: | -----------------------: | +| memory/list/10/128 | linux | 90 | 90 | 218,544 | 272,800 | +| memory/list/10/128 | macos | 90 | 90 | 218,504 | 272,768 | +| memory/list/10000/4096 | linux | 60,742 | 60,039 | 50,601,072 | 4,574,320 | +| memory/list/10000/4096 | macos | 60,742 | 60,039 | 50,595,408 | 4,574,288 | +| nested-memory/list/10000/4096 | linux | 61,503 | 60,097 | 97,124,976 | 5,013,600 | +| nested-memory/list/10000/4096 | macos | 61,503 | 60,097 | 97,113,608 | 5,013,504 | +| nested-memory/last/10000/4096 | linux | 61,491 | 60,085 | 94,900,952 | 2,789,576 | +| nested-memory/last/10000/4096 | macos | 61,491 | 60,085 | 94,889,560 | 2,789,456 | + +## Correctness and footprint + +Memory tests cover default unsupported cursors, empty/binary sources, zero spans, independent offsets, EOF, untouched tails, mixed owned/borrowed position and lifetime. Member tests cover 64 KiB caps, positive short reads, late failures, retained parents, four nested compression combinations, shared-budget release and sparse PAX/gzip holes. Sparse readers may legally return a positive partial block: the test consumes each returned count and checks untouched tails. + +Both hosts pass the 313 selected root-and-extension release suites and five focused ASan suites. Linux additionally passes five MSan and five TSan suites, with FUSE required in the full release selection. Five changed translation units pass real scoped clang-tidy on each host. Source guards verify restoration before/after validation and diagnostics. Current pushed-head CI, including coverage, remains required for readiness. + +- linux: unstripped full CLI 18,146,176 -> 18,148,600 bytes (+2,424). +- macos: unstripped full CLI 8,157,560 -> 8,158,200 bytes (+640). + +## Lossless evidence + +[caller-owned-streams-20261103.json.gz](caller-owned-streams-20261103.json.gz): 3,186,597 bytes; SHA-256 `b23aadac072637ca3882b6fb25b1df2d5ee1960d4eae95d295743d38fb6b4d5c`. diff --git a/docs/performance-analysis.md b/docs/performance-analysis.md index 3d3bb59b014..27ae1e1e55d 100644 --- a/docs/performance-analysis.md +++ b/docs/performance-analysis.md @@ -3553,3 +3553,34 @@ APIs intact; a private stateful record writer could reuse capacity on the coordi view only during the synchronous sink call. Audit aliasing, reentrant callbacks, cancellation and worker/result lifetimes. Worker-produced output must stay owned or have a separately proven batch lifetime. This is separate from O18 byte backpressure and ordered pipeline publication. + +### O19c: caller-owned memory and member cursors + +Immutable memory and archive-member cursors now expose the existing `ReadInto` capability. O19a +can reuse its reserved buffer rather than asking these sources for a new owned string per callback. +Only initialized bytes become visible. Memory reads preserve independent offsets and source +ownership; member reads retain their 64 KiB cap, sparse holes, positive short reads, errors and +parent/budget lifetime. The owned API remains available and uses the same checked decoder path. + +[The cross-platform report](benchmarks/caller-owned-streams-20261103.md) isolates memory-only, +member-only and combined changes on the dependency set including #1065. Actual memory and nested +member families are separate from explicitly incapable legacy and custom caller-owned controls. +Native nested TAR gains are small; macOS nested in-memory gains are larger. Preserve slower cells: +confirmation includes roughly 2-4% ordinary/small command movements and Linux small kernel costs. +A 10,000-member nested listing reduces cumulative requested allocation bytes from about 97 MB to +5 MB; this is not a claim of peak-memory or RSS savings. A tiny memory source can instead request +about 54 KiB more while retaining the same allocation count and existing read reservation. + +Short macOS kernel runs showed large reference-repeat movement. A separate fixed-100,000-iteration +check on the same binaries brought the tiny memory and legacy cases within 0.3% of baseline there; +Linux retained about 3.6-3.7% movement. Keep these populations separate, with their controls, rather +than replacing or pooling inconvenient samples. A later pilot of byte-identical baseline/selected +binaries also moved the controls; small percentage changes have limited causal attribution. + +A rewrite that duplicated the original owned member-read path did not consistently improve the +cross-platform controls and was rejected. Its 24 native rows and 72 memory runs per host remain +in the raw artifact. The frozen formatter, source guards, independent full-output fixture oracle, +all selected root/extension release tests and focused sanitizers remain part of qualification. +The initial nested fixture command missed `--archive-depth=2`; its warmup oracle failed before +accepting any native row. The corrected matrix uses explicit depth two. O19b seeking/index/replay +and admission, and O19d direct final-buffer materialization, remain open. diff --git a/extra_modules/archive/archive_reader.cc b/extra_modules/archive/archive_reader.cc index d6fc4b084c9..8c1713a187e 100644 --- a/extra_modules/archive/archive_reader.cc +++ b/extra_modules/archive/archive_reader.cc @@ -310,13 +310,20 @@ class MemberStream final : public vfs::ReadStream { absl::StatusOr Read(std::size_t max_bytes) override { std::string result(std::min(max_bytes, kBlockSize), '\0'); - const auto count = ::archive_read_data(session_->handle.get(), result.data(), result.size()); + MBO_ASSIGN_OR_RETURN(const auto count, ReadInto(std::span(result))); + result.resize(count); + return result; + } + + bool SupportsReadInto() const noexcept override { return true; } + + absl::StatusOr ReadInto(std::span buffer) override { + const auto count = ::archive_read_data(session_->handle.get(), buffer.data(), std::min(buffer.size(), kBlockSize)); if (count < 0) { MBO_RETURN_IF_ERROR(session_->error); return absl::DataLossError(LastError(session_->handle.get())); } - result.resize(static_cast(count)); - return result; + return static_cast(count); } private: diff --git a/extra_modules/archive/archive_source_buffer_test.cc b/extra_modules/archive/archive_source_buffer_test.cc index dea81024cc4..074325fec43 100644 --- a/extra_modules/archive/archive_source_buffer_test.cc +++ b/extra_modules/archive/archive_source_buffer_test.cc @@ -36,6 +36,8 @@ using ::testing::Gt; using ::testing::HasSubstr; using ::testing::IsEmpty; using ::testing::IsTrue; +using ::testing::Le; +using ::testing::Lt; constexpr std::size_t kReadBlock = 64UZ * 1'024; @@ -131,10 +133,12 @@ class CountingSource final : public vfs::ReadSource { }; struct ArchiveSourceBufferTest : ::testing::Test { - static std::string MakeTar(std::string_view content, bool compressed = false) { + static std::string MakeTar(std::string_view content, bool compressed = false, bool sparse = 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)); + EXPECT_THAT( + sparse ? ::archive_write_set_format_pax(writer) : ::archive_write_set_format_pax_restricted(writer), + Eq(ARCHIVE_OK)); if (compressed) { EXPECT_THAT(::archive_write_add_filter_gzip(writer), Eq(ARCHIVE_OK)); } @@ -147,6 +151,9 @@ struct ArchiveSourceBufferTest : ::testing::Test { ::archive_entry_set_size(entry, static_cast(content.size())); ::archive_entry_set_filetype(entry, AE_IFREG); ::archive_entry_set_perm(entry, 0644); + if (sparse) { + ::archive_entry_sparse_add_entry(entry, static_cast(kReadBlock), 4); + } 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); @@ -246,5 +253,97 @@ TEST_F(ArchiveSourceBufferTest, RetainsExactlyOneSessionReservationAndReleasesIt EXPECT_THAT(budget->MemoryUsed(), Eq(0)); } +TEST_F(ArchiveSourceBufferTest, MemberReadIntoCapsChunksPreservesTailAndSharesOwnedReadPosition) { + std::string content((2 * kReadBlock) + 17, 'x'); + for (std::size_t at = 0; at < content.size(); ++at) { + content.at(at) = static_cast('a' + (at % 26)); + } + const auto budget = std::make_shared(kReadBlock + 4); + auto source = vfs::MemoryReadSource(MakeTar(content), budget); + auto member = MemberReadSource(source, "data.txt"); + MBO_ASSERT_OK_AND_ASSIGN(auto cursor, member->Open()); + source.reset(); + member.reset(); + EXPECT_THAT(cursor->SupportsReadInto(), IsTrue()); + std::string buffer(70'000, '!'); + const std::span span(buffer); + EXPECT_THAT(cursor->ReadInto(std::span()), IsOkAndHolds(Eq(0))); + EXPECT_THAT(cursor->ReadInto(span.first(0)), IsOkAndHolds(Eq(0))); + EXPECT_THAT(cursor->ReadInto(span), IsOkAndHolds(Eq(kReadBlock))); + EXPECT_THAT(std::string_view(buffer).substr(0, kReadBlock), EqualsText(content.substr(0, kReadBlock))); + EXPECT_THAT(std::string_view(buffer).substr(kReadBlock), EqualsText(std::string(buffer.size() - kReadBlock, '!'))); + EXPECT_THAT(cursor->Read(3), IsOkAndHolds(EqualsText(content.substr(kReadBlock, 3)))); + EXPECT_THAT(cursor->ReadInto(span), IsOkAndHolds(Eq(kReadBlock))); + EXPECT_THAT(std::string_view(buffer).substr(0, kReadBlock), EqualsText(content.substr(kReadBlock + 3, kReadBlock))); + EXPECT_THAT(cursor->ReadInto(span), IsOkAndHolds(Eq(14))); + EXPECT_THAT(std::string_view(buffer).substr(0, 14), EqualsText(content.substr(content.size() - 14))); + EXPECT_THAT(cursor->ReadInto(span), IsOkAndHolds(Eq(0))); + EXPECT_THAT(budget->MemoryUsed(), Eq(kReadBlock)); + cursor.reset(); + EXPECT_THAT(budget->MemoryUsed(), Eq(0)); +} + +TEST_F(ArchiveSourceBufferTest, MemberReadIntoPreservesLateSourceFailures) { + 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 cursor, MemberReadSource(source, "data.txt")->Open()); + std::string buffer(kReadBlock, '!'); + EXPECT_THAT(cursor->ReadInto(std::span(buffer)), StatusIs(absl::StatusCode::kDataLoss, HasSubstr("injected"))); +} + +TEST_F(ArchiveSourceBufferTest, SparseMemberReadIntoInitializesHolesInPreviouslyUsedStorage) { + std::string content(2 * kReadBlock, '\0'); + content.replace(kReadBlock, 4, "data"); + for (const bool compressed : {false, true}) { + SCOPED_TRACE(compressed); + const std::string bytes = MakeTar(content, compressed, true); + // The uncompressed archive is smaller than the logical member, proving it stores holes. + EXPECT_THAT(bytes.size(), Lt(content.size())); + MBO_ASSERT_OK_AND_ASSIGN(auto cursor, MemberReadSource(vfs::MemoryReadSource(bytes), "data.txt")->Open()); + std::string buffer(kReadBlock, '!'); + const std::span span(buffer); + // Sparse extents can yield short reads; verify only each returned prefix, then continue. + for (std::size_t position = 0; position < content.size();) { + std::ranges::fill(buffer, '!'); + MBO_ASSERT_OK_AND_ASSIGN(const auto count, cursor->ReadInto(span)); + ASSERT_THAT(count, Gt(0)); + ASSERT_THAT(count, Le(buffer.size())); + EXPECT_THAT(std::string_view(buffer).substr(0, count), EqualsText(content.substr(position, count))); + EXPECT_THAT(std::string_view(buffer).substr(count), EqualsText(std::string(buffer.size() - count, '!'))); + position += count; + } + EXPECT_THAT(cursor->ReadInto(span), IsOkAndHolds(Eq(0))); + } +} + +TEST_F(ArchiveSourceBufferTest, NestedMemberBuffersRetainParentsAndReleaseSharedReservations) { + for (const bool outer_compressed : {false, true}) { + for (const bool inner_compressed : {false, true}) { + SCOPED_TRACE(outer_compressed); + SCOPED_TRACE(inner_compressed); + const std::string content(20'000, 'x'); + // The temporary prefix reader and both four-byte format probes can overlap. + const auto budget = std::make_shared((3 * kReadBlock) + 8); + auto source = vfs::MemoryReadSource(MakeTar(MakeTar(content, inner_compressed), outer_compressed), budget); + auto nested = MemberReadSource(source, "data.txt"); + auto member = MemberReadSource(nested, "data.txt"); + MBO_ASSERT_OK_AND_ASSIGN(auto cursor, member->Open()); + source.reset(); + nested.reset(); + member.reset(); + EXPECT_THAT(cursor->SupportsReadInto(), IsTrue()); + std::string buffer(content.size() + 8, '!'); + const std::span span(buffer); + EXPECT_THAT(cursor->ReadInto(span), IsOkAndHolds(Eq(content.size()))); + EXPECT_THAT(std::string_view(buffer).substr(0, content.size()), EqualsText(content)); + EXPECT_THAT(std::string_view(buffer).substr(content.size()), EqualsText("!!!!!!!!")); + EXPECT_THAT(cursor->ReadInto(span), IsOkAndHolds(Eq(0))); + EXPECT_THAT(budget->MemoryUsed(), Eq(2 * kReadBlock)); + cursor.reset(); + EXPECT_THAT(budget->MemoryUsed(), Eq(0)); + } + } +} + } // namespace } // namespace xff::archive diff --git a/release-site.json b/release-site.json index 42af27f9334..2eb9d52ff97 100644 --- a/release-site.json +++ b/release-site.json @@ -78,7 +78,8 @@ "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/archive-input-buffers-20261102.md": "docs/benchmarks/archive-input-buffers-20261102.html", - "docs/benchmarks/digest-and-record-storage-20261103.md": "docs/benchmarks/digest-and-record-storage-20261103.html" + "docs/benchmarks/digest-and-record-storage-20261103.md": "docs/benchmarks/digest-and-record-storage-20261103.html", + "docs/benchmarks/caller-owned-streams-20261103.md": "docs/benchmarks/caller-owned-streams-20261103.html" }, "generated_html": { "XFF.md": { diff --git a/tools/BUILD.bazel b/tools/BUILD.bazel index 8750926ecee..aafdf434aa4 100644 --- a/tools/BUILD.bazel +++ b/tools/BUILD.bazel @@ -35,6 +35,7 @@ cc_binary( "@abseil-cpp//absl/status:statusor", "@abseil-cpp//absl/strings", "@com_github_google_benchmark//:benchmark", + "@mboworks_mbo//mbo/status:status_macros_cc", "@xff_archive//:archive_reader_cc", "@xff_extras_api//:read_source_cc", ], diff --git a/tools/archive_source_benchmark.cc b/tools/archive_source_benchmark.cc index 86d90fc7722..32fbec440e7 100644 --- a/tools/archive_source_benchmark.cc +++ b/tools/archive_source_benchmark.cc @@ -21,6 +21,7 @@ #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" #include "benchmark/benchmark.h" +#include "mbo/status/status_macros.h" #include "xff/archive/archive_reader.h" #include "xff/vfs/read_source.h" @@ -29,7 +30,8 @@ namespace { class MemoryCursor final : public vfs::ReadStream { public: - explicit MemoryCursor(std::shared_ptr bytes) : bytes_(std::move(bytes)) {} + MemoryCursor(std::shared_ptr bytes, bool caller_owned) + : bytes_(std::move(bytes)), caller_owned_(caller_owned) {} absl::StatusOr Read(std::size_t max_bytes) override { auto result = bytes_->substr(offset_, std::min(max_bytes, bytes_->size() - offset_)); @@ -37,7 +39,7 @@ class MemoryCursor final : public vfs::ReadStream { return result; } - bool SupportsReadInto() const noexcept override { return true; } + bool SupportsReadInto() const noexcept override { return caller_owned_; } absl::StatusOr ReadInto(std::span buffer) override { const std::size_t count = std::min(buffer.size(), bytes_->size() - offset_); @@ -49,26 +51,30 @@ class MemoryCursor final : public vfs::ReadStream { private: std::shared_ptr bytes_; std::size_t offset_ = 0; + bool caller_owned_; }; -class CallerOwnedSource final : public vfs::ReadSource { +class BenchmarkSource final : public vfs::ReadSource { public: - explicit CallerOwnedSource(std::string bytes) : bytes_(std::make_shared(std::move(bytes))) {} + BenchmarkSource(std::string bytes, bool caller_owned) + : bytes_(std::make_shared(std::move(bytes))), caller_owned_(caller_owned) {} absl::StatusOr> Open() const override { - return std::make_unique(bytes_); + return std::make_unique(bytes_, caller_owned_); } private: std::shared_ptr bytes_; + bool caller_owned_; }; -absl::StatusOr MakeTar(std::size_t members, std::size_t payload) { +absl::StatusOr MakeTar(std::size_t members, std::size_t payload, std::string_view entry_content = {}) { // 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'); + const std::string generated = entry_content.empty() ? std::string(payload, 'x') : std::string(); + const std::string_view content = entry_content.empty() ? std::string_view(generated) : entry_content; + std::string bytes((members * (content.size() + 1'024)) + 16'384, '\0'); std::size_t used = 0; ::archive_write_open_memory(writer, bytes.data(), bytes.size(), &used); bool valid = true; @@ -77,11 +83,11 @@ absl::StatusOr MakeTar(std::size_t members, std::size_t payload) { 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_size(entry, static_cast(content.size())); ::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); + valid &= ::archive_write_data(writer, content.data(), content.size()) == static_cast(content.size()); ::archive_entry_free(entry); } valid &= ::archive_write_close(writer) == ARCHIVE_OK; @@ -93,7 +99,22 @@ absl::StatusOr MakeTar(std::size_t members, std::size_t payload) { return bytes; } -void Measure(benchmark::State& state, bool caller_owned, bool read_last) { +enum class SourceKind { kLegacy, kCallerOwned, kMemory, kNestedMemory }; + +absl::StatusOr MakeSource(SourceKind kind, std::string bytes) { + switch (kind) { + case SourceKind::kLegacy: return std::make_shared(std::move(bytes), false); + case SourceKind::kCallerOwned: return std::make_shared(std::move(bytes), true); + case SourceKind::kMemory: return vfs::MemoryReadSource(std::move(bytes)); + case SourceKind::kNestedMemory: { + MBO_ASSIGN_OR_RETURN(auto outer, MakeTar(1, bytes.size(), bytes)); + return MemberReadSource(vfs::MemoryReadSource(std::move(outer)), "file-0"); + } + } + return absl::InvalidArgumentError("unknown archive benchmark source"); +} + +void Measure(benchmark::State& state, SourceKind kind, bool read_last) { const auto members = static_cast(state.range(0)); const auto payload = static_cast(state.range(1)); auto bytes = MakeTar(members, payload); @@ -101,8 +122,13 @@ void Measure(benchmark::State& state, bool caller_owned, bool read_last) { state.SkipWithError(bytes.status().ToString()); return; } - const vfs::SharedReadSource source = - caller_owned ? std::make_shared(std::move(*bytes)) : vfs::MemoryReadSource(std::move(*bytes)); + // Keep legacy controls explicitly incapable; exercise actual memory/member sources separately. + auto built_source = MakeSource(kind, std::move(*bytes)); + if (!built_source.ok()) { + state.SkipWithError(built_source.status().ToString()); + return; + } + const auto source = std::move(*built_source); 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. @@ -127,20 +153,30 @@ void Measure(benchmark::State& state, bool caller_owned, bool read_last) { } void RegisterAll() { + struct SourceCase { + std::string_view name; + SourceKind kind; + }; + + constexpr auto kSources = std::to_array({ + {.name = "legacy", .kind = SourceKind::kLegacy}, + {.name = "caller-owned", .kind = SourceKind::kCallerOwned}, + {.name = "memory", .kind = SourceKind::kMemory}, + {.name = "nested-memory", .kind = SourceKind::kNestedMemory}, + }); 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 auto& source : kSources) { for (const bool read_last : {false, true}) { - const std::string name = - absl::StrCat("archive/", caller_owned ? "caller-owned/" : "legacy/", read_last ? "last" : "list"); + const std::string name = absl::StrCat("archive/", source.name, "/", 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); + auto* registration = benchmark::RegisterBenchmark(name, Measure, source.kind, read_last); for (const auto count : kCounts) { for (const auto payload : kPayloads) { registration->Args({count, payload}); } } - registration->Unit(benchmark::kMicrosecond)->UseRealTime(); + registration->Unit(benchmark::kMicrosecond)->UseRealTime()->MinTime(0.02)->MinWarmUpTime(0.01)->Repetitions(9); } } } @@ -148,6 +184,7 @@ void RegisterAll() { } // namespace } // namespace xff::archive +// XFF_ABI_POINTER: Google Benchmark's process argument interface. int main(int argc, char** argv) { benchmark::Initialize(&argc, argv); xff::archive::RegisterAll(); diff --git a/xff/cli/BUILD.bazel b/xff/cli/BUILD.bazel index 9244338f292..62f7cdf72aa 100644 --- a/xff/cli/BUILD.bazel +++ b/xff/cli/BUILD.bazel @@ -1272,6 +1272,7 @@ bashtest( bashtest( name = "rg_full_test", size = "small", + timeout = "moderate", srcs = ["rg_full_test.sh"], data = ["//xff/cli/testing:xff_full"], target_compatible_with = XFF_FULL_ONLY, diff --git a/xff_extras_api/read_source.cc b/xff_extras_api/read_source.cc index 2adc21bff26..843c4cfa0a4 100644 --- a/xff_extras_api/read_source.cc +++ b/xff_extras_api/read_source.cc @@ -33,6 +33,15 @@ class MemoryStream final : public ReadStream { 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; diff --git a/xff_extras_api/read_source_test.cc b/xff_extras_api/read_source_test.cc index a4eeb3ca3fb..16928199896 100644 --- a/xff_extras_api/read_source_test.cc +++ b/xff_extras_api/read_source_test.cc @@ -32,13 +32,64 @@ using ::testing::Optional; struct ReadSourceTest : ::testing::Test {}; -TEST_F(ReadSourceTest, UnsupportedReadIntoNeitherConsumesNorCopiesMemorySource) { - MBO_ASSERT_OK_AND_ASSIGN(auto stream, MemoryReadSource("abcdef")->Open()); - EXPECT_THAT(stream->SupportsReadInto(), IsFalse()); +class OwnedOnlyStream final : public ReadStream { + public: + absl::StatusOr Read(std::size_t max_bytes) override { + auto result = bytes_.substr(offset_, max_bytes); + offset_ += result.size(); + return result; + } + + private: + const std::string bytes_ = "abcdef"; + std::size_t offset_ = 0; +}; + +TEST_F(ReadSourceTest, UnsupportedReadIntoNeitherConsumesNorCopiesOwnedOnlyStream) { + OwnedOnlyStream stream; + EXPECT_THAT(stream.SupportsReadInto(), IsFalse()); std::string buffer = "unchanged"; - EXPECT_THAT(stream->ReadInto(std::span(buffer)), StatusIs(absl::StatusCode::kUnimplemented)); + EXPECT_THAT(stream.ReadInto(std::span(buffer)), StatusIs(absl::StatusCode::kUnimplemented)); EXPECT_THAT(buffer, EqualsText("unchanged")); - EXPECT_THAT(stream->Read(6), IsOkAndHolds(EqualsText("abcdef"))); + EXPECT_THAT(stream.Read(6), IsOkAndHolds(EqualsText("abcdef"))); +} + +TEST_F(ReadSourceTest, MemoryReadIntoSharesOwnedReadPositionAndRetainsSourceBytes) { + auto source = MemoryReadSource("abcdef"); + MBO_ASSERT_OK_AND_ASSIGN(auto stream, source->Open()); + MBO_ASSERT_OK_AND_ASSIGN(auto suffix, source->OpenAt(2)); + MBO_ASSERT_OK_AND_ASSIGN(auto beyond, source->OpenAt(99)); + source.reset(); + EXPECT_THAT(stream->SupportsReadInto(), IsTrue()); + std::string buffer(8, '!'); + const std::span span(buffer); + EXPECT_THAT(stream->ReadInto(span.first(0)), IsOkAndHolds(Eq(0))); + EXPECT_THAT(stream->Read(2), IsOkAndHolds(EqualsText("ab"))); + EXPECT_THAT(stream->ReadInto(span.first(3)), IsOkAndHolds(Eq(3))); + EXPECT_THAT(buffer, EqualsText("cde!!!!!")); + EXPECT_THAT(stream->Read(99), IsOkAndHolds(EqualsText("f"))); + EXPECT_THAT(stream->ReadInto(span), IsOkAndHolds(Eq(0))); + EXPECT_THAT(buffer, EqualsText("cde!!!!!")); + EXPECT_THAT(suffix->ReadInto(span), IsOkAndHolds(Eq(4))); + EXPECT_THAT(buffer, EqualsText("cdef!!!!")); + EXPECT_THAT(beyond->ReadInto(span), IsOkAndHolds(Eq(0))); + EXPECT_THAT(buffer, EqualsText("cdef!!!!")); +} + +TEST_F(ReadSourceTest, MemoryReadIntoPreservesBinaryBytesAndHandlesEmptySpansAndSources) { + const std::string bytes("a\0b\xff", 4); + MBO_ASSERT_OK_AND_ASSIGN(auto cursor, MemoryReadSource(bytes)->Open()); + std::string buffer(8, '!'); + EXPECT_THAT(cursor->ReadInto(std::span()), IsOkAndHolds(Eq(0))); + EXPECT_THAT(buffer, EqualsText("!!!!!!!!")); + EXPECT_THAT(cursor->ReadInto(std::span(buffer)), IsOkAndHolds(Eq(bytes.size()))); + EXPECT_THAT(std::string_view(buffer).substr(0, bytes.size()), EqualsText(bytes)); + EXPECT_THAT(std::string_view(buffer).substr(bytes.size()), EqualsText("!!!!")); + const std::string previous = buffer; + EXPECT_THAT(cursor->ReadInto(std::span(buffer)), IsOkAndHolds(Eq(0))); + MBO_ASSERT_OK_AND_ASSIGN(auto empty, MemoryReadSource("")->Open()); + EXPECT_THAT(empty->ReadInto(std::span(buffer)), IsOkAndHolds(Eq(0))); + EXPECT_THAT(buffer, EqualsText(previous)); } TEST_F(ReadSourceTest, HostReadIntoCapsChunksPreservesTailAndSharesCursorPosition) {