diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 7c32cd68722..657358b4417 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|caller-owned-streams|direct-source-materialization|hash-field-binding|hash-field-binding-tag)-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|direct-source-materialization|hash-field-binding|hash-field-binding-tag|forward-cursor-skipping)-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 c9051ebdcdf..7927d9e2522 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ # 0.11.0 +- Skip discarded large archive payloads through eligible memory and read-only native cursors while preserving sequential fallbacks, errors and shared read budgets. + - Bind explicit hash-field qualifiers once while preserving dynamic defaults, copied templates and snapshot lifetimes. - Fill bounded source ranges directly into final storage and reuse owned decoder blocks during materialization, preserving read limits, errors and budgets. diff --git a/TODO.md b/TODO.md index 3c6070e02b3..3ba81e68e26 100644 --- a/TODO.md +++ b/TODO.md @@ -449,6 +449,13 @@ EP/JC/P work; candidates are not speedup claims until measured on both platforms - [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). +- [x] O19b1: Qualify bounded forward cursor skipping on native/memory sources with sequential + fallbacks, latched errors, unchanged buffer lifetimes, shared budgets and physical bounds. + Keep [all source/cursor trials and slower controls](docs/benchmarks/forward-cursor-skipping-20261103.md). +- [ ] Streamed ZIP sizes: fix member metadata for ZIP inputs whose local headers defer sizes to + data descriptors. A nonseekable libarchive memory-writer fixture reproduces zero size for + a 1 MiB member on the unchanged parent. Preserve the exact size oracle and parent proof in + the O19b1 artifact; do not silently admit an unbudgeted seekable central-directory index. - [ ] 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 diff --git a/docs/benchmarks/forward-cursor-skipping-20261103.json.gz b/docs/benchmarks/forward-cursor-skipping-20261103.json.gz new file mode 100644 index 00000000000..8cc9c942835 Binary files /dev/null and b/docs/benchmarks/forward-cursor-skipping-20261103.json.gz differ diff --git a/docs/benchmarks/forward-cursor-skipping-20261103.md b/docs/benchmarks/forward-cursor-skipping-20261103.md new file mode 100644 index 00000000000..246087f0f91 --- /dev/null +++ b/docs/benchmarks/forward-cursor-skipping-20261103.md @@ -0,0 +1,136 @@ +# Bounded forward skipping of archive payloads + +Discarded large payloads can advance an existing input cursor without reading every byte. +The archive reader requests optional skipping only for remaining spans of at least 64 KiB. +Memory cursors advance their existing offset. Read-only host cursors lazily inspect their owned +regular-file descriptor, bound advances by its observed size, and seek that same descriptor. +No path reopening or cursor allocation is added per skip. Sequential, decoded and member cursors +retain their ordinary read/discard fallback when they do not implement this capability. + +`ReadStream::Skip` returns an optional count or a failure: absence means unsupported, a present +count must not exceed the request, and zero alone does not prove EOF. Read, ReadInto and Skip +share the cursor position. Cursor objects have unique ownership; transfer their unique_ptr rather +than copying or moving a cursor object. Immutable sources retain ordinary copy/move operations. + +The libarchive callback preserves borrowed-buffer lifetimes. A failed skip or an oversized count +latches an error; its fallback read reports that error before touching the cursor again. This is +required by the pinned skip proxy, which does not treat a negative custom callback result as a +reliable termination mechanism. A common value-returning read-only syscall adapter preserves native +status conversion and is exercised by real metadata failures. No new writable handles, host +mutations, dependencies, threads, locks, decoder concurrency or CLI flags are introduced. + +## Selected mechanism and controls + +The first source-level experiment queried a source capability/size and reopened at each skipped +position. It demonstrates the potential benefit but adds cursor allocation/reopening and session +position state. The selected cursor-level design avoids those operations and leaves SourceSession +layout unchanged. Keep the first experiment's partial qualification separate: its memory pilots +completed on both hosts, but its native pilot completed only on Linux. + +Four frozen participants compare the exact parent, API-only cursor changes without callback +admission, eager skipping and the selected 64 KiB threshold. The callback threshold avoids work +for small remainders; it is an internal optimization, not a new user-controlled safety limit. +Legacy and caller-owned-only sources are explicitly incapable controls. Actual memory and nested +member sources exercise production implementations rather than being relabeled legacy wrappers. + +Final preparation is pinned to `38ac542f621abd9fb2610e0d01f8646f42428ab3`. Parent PR #1070 was +subsequently merged as `44e7e30102eb6c6ae122135189b6c62340c35bdb`; their complete Git trees are identical, +and all six candidate source files are unchanged after synchronization. Fresh current-head CI is +still required for readiness; tree equivalence does not substitute another PR's checks. + +## Cross-platform measurements + +The final four-participant matrix contains 37 in-memory cases and 40 native cases per host, each +in three sessions. Native cases cover small, boundary and large TARs, stored/deflated ZIP, gzip TAR, +nested TAR and ordinary files with one/three requested workers. Native Python fixtures independently +verify sizes and complete path/content output; stored/deflated ZIP fixtures have explicit local-header +sizes. Each participant discards a warmup; nine interleaved samples retain the fastest seven. + +A separate confirmation compares the original parent, a byte-identical parent copy, the preceding +selected binary and the final selected binary. It repeats 12 memory cases in five sessions and +20 native cases in three sessions. The table shows median within-session duration changes from +that final confirmation. Negative means less elapsed time. Requested workers do not imply parallel +archive decoding. Linux uses tmpfs plus CPU affinity; macOS uses warm APFS without affinity. +No same-host timing overlaps builds, tests, lint or instrumentation. + +| Native fixture | Operation | Linux / 1 worker | Linux / 3 workers | macOS / 1 worker | macOS / 3 workers | +| :---------------------- | :-------- | ---------------: | ----------------: | ---------------: | ----------------: | +| TAR, 100 x 1 MiB | list | -82.18% | -81.00% | -51.54% | -51.95% | +| TAR, 100 x 1 MiB | last | -86.93% | -86.74% | -63.80% | -64.56% | +| Stored ZIP, 100 x 1 MiB | list | -80.12% | -79.27% | -49.66% | -49.54% | +| Stored ZIP, 100 x 1 MiB | last | -84.79% | -84.21% | -61.42% | -61.30% | +| TAR, 10 x 64 KiB | list | -1.34% | -1.34% | +0.56% | +0.17% | +| TAR, 10 x 64 KiB | last | -0.92% | -0.22% | -0.14% | +0.48% | +| Ordinary 1,000 files | list | +0.79% | +0.29% | +0.28% | +0.36% | +| Ordinary 1,000 files | last | -0.49% | -1.93% | +0.38% | +0.05% | + +The final nested-TAR confirmation improves listing about 11% on Linux and 7% on macOS, and +last-member content about 11% and 6%, respectively. Gzip-TAR controls stay within roughly 0.3%. +Actual in-memory 100 x 1 MiB TAR listing improves about 97% Linux / 92% macOS; reading the final +member improves about 96% / 85%. Sequential/incapable inputs remain explicit controls. + +These are paired medians, not the quotient of independently computed median absolute times. +The lossless data retains every raw sample, session and participant. The byte-identical native +reference copy moves by -10.61% to +1.63% on Linux and -1.33% to +0.59% on macOS; the large gains +are much greater than that movement. Final ordinary/boundary native controls cost at most 0.79% +on Linux and 0.56% on macOS in this confirmation. + +Some tiny macOS memory kernels are unstable. An initial 4 KiB listing cell reports +59.72%, with +absolute values ranging roughly 7.5-15.9 us across participants/sessions; its final selected +confirmation is -3.80%, while an identical reference copy moves +4.75%. Preserve the initial +population rather than discarding it. Other final macOS controls retain costs up to 4.42%, +including caller-owned-only inputs that cannot skip. Do not claim a universal speedup or infer +that every small-kernel difference is caused by this mechanism. + +The explicit ownership-declaration refinement produces identical runtime code sections for the +CLI and both kernels on each host. The subsequent status-adapter refinement changes CLI code, +and its final binary is used in native confirmation; both memory kernel code sections remain +identical. These are code-section identity checks, not claims that complete debug-bearing files +have identical hashes. Final full unstripped CLI footprint increases 6,712 bytes on Linux and +17,040 bytes on macOS relative to the parent (about 0.04% and 0.21%). + +## Correctness, budgets and limitations + +Eleven new regressions exercise unsupported/default cursors, zero/partial/maximum requests, +shared read positions, owner retention, native descriptors after source-path deletion, large +payload skipping, owned buffers, selected content, small/compressed inputs, sequential fallbacks, +latched failures, oversized results and physical truncation. Compile-time checks prevent accidental +cursor copying/moving. Existing archive format, member, XAR, sparse-input, resource and fuzz tests +remain active. Only successfully returned/initialized bytes reach readers. + +Skip changes no shared memory/replay budget, decoder workspace or archive-owner lifetime. +The existing session buffer reservation is released on success and failure. Native cursor state +adds its access classification, position and observed size; it does not add a metadata syscall +to ordinary reads. Size inspection is lazy at the first eligible positive skip. Host input retains +the existing read-only source model; this is not an atomic snapshot facility for concurrent writers. + +All 313 selected root/extension release suites pass on both hosts. Five focused ASan suites pass +per host, and five MSan/five TSan suites pass on Linux. Explicit checks of five actual modified +translation units include both logical and physical read_source.h aliases; an inherited missing +special-member declaration was corrected after that check found it. The final implementation +passes a focused check of its changed runtime unit. Full Linux coverage runs all 313 suites and +passes every repository threshold: the candidate has 61/61 changed lines and 28/28 branches covered. +No executable-line exclusions, threshold reduction or test-only production hook is used. + +One existing streamed-ZIP limitation was independently reproduced on the unchanged parent: +a ZIP produced by libarchive's nonseekable memory writer can report a zero member size at the +header when the size is deferred to a data descriptor. The benchmark's exact size oracle caught +this; it was not weakened. Large in-memory payload kernels therefore use TAR. Retain the failed +setup, writer source and parent reproduction as evidence, and track the ZIP issue separately. + +This completes bounded forward skipping only. Seekable ZIP central-directory indexing, repeated +member/replay reuse, retained-index accounting and decoder admission remain separate unqualified +work. The existing XAR seek callback is unchanged. No unbudgeted ZIP index or parallel decoder is +silently enabled by this optimization. + +## Reproduction evidence + +[Lossless experiment data](forward-cursor-skipping-20261103.json.gz) retains frozen sources for the +source-level and cursor-level experiments, original/eager/API-only/bounded controls, final source +refinements, compiler/configuration/link/binary identities, complete timing populations, +independent oracles, validation/coverage logs, scripts and parent synchronization proof. Earlier +failures and noisy populations remain labeled separately. Normal enforcing commit/push hooks and +fresh CI are additional merge-readiness requirements. + +Archive: 5,017,324 bytes, containing 803 labeled evidence files. SHA-256: +`b66c5d4a2785e9d4659314293b1ffd5545459d6c379a9e70bc54691d2f309461`. diff --git a/docs/design.md b/docs/design.md index ad9c28f240c..382f00d91ea 100644 --- a/docs/design.md +++ b/docs/design.md @@ -124,6 +124,11 @@ Exit status is `0` for successful execution and `2` for errors. With `--quiet` o ## Specialized features and development +Read-only archive cursors may skip discarded large payloads without reading them when their backend +supports bounded forward advancement. Sequential fallbacks, cursor ownership, failures and shared +budgets remain explicit; [the cursor qualification](benchmarks/forward-cursor-skipping-20261103.md) +records the contract and limits. This does not enable parallel decoding or a new retained index. + Archives and compressed streams are composable extras; virtual entries retain their source identity and host extraction/writing requires the corresponding capabilities. `--shards` supports the documented `of`, `dotnum`, and `underscore` schemes, with custom `--shard-pattern` definitions and explicit display diff --git a/docs/performance-analysis.md b/docs/performance-analysis.md index 0b21e13b8c2..616bc73f483 100644 --- a/docs/performance-analysis.md +++ b/docs/performance-analysis.md @@ -3657,3 +3657,28 @@ and three-session cross-platform measurements. Sixteen warm explicit fields impr while small ordinary kernels and one Linux single-hash cell retain costs. Native differences remain small, and the full CLI decreases 4,896 bytes on Linux and 208 bytes on macOS relative to the prior binding implementation. A structural regression preserves literal/hash/extraction distinctions. + +### O19b1: bounded forward cursor skipping + +Archive readers now request optional cursor skipping only for unread spans of at least 64 KiB. +Memory cursors advance offsets; native cursors lazily bound a seek using their owned descriptor. +Sequential/member decoders keep their read fallback, and skip failures/oversized counts are latched +before another read. Borrowed buffers, selected content, truncation errors, shared reservations and +replay accounting remain explicit. Cursor objects cannot duplicate their owned state; sources retain +ordinary copy/move operations. No new output path, worker, lock or decoder admission is introduced. + +[Cross-platform qualification](benchmarks/forward-cursor-skipping-20261103.md) retains the earlier +source-reopen design, separate four-variant pilots, the final 37-memory/40-native matrix and an +independent identical-reference confirmation. Large native uncompressed TAR/stored-ZIP cases improve +about 79-87% on Linux and 50-65% on macOS. Ordinary/boundary native controls remain within roughly +0.8%; virtual-only controls retain some costs, and unstable tiny macOS cells stay visible. + +Final full CLI increases 6,712 bytes Linux / 17,040 bytes macOS. Complete release suites and focused +sanitizers pass both hosts; all five actual modified TUs receive explicit changed-header checks, +including physical and include-prefix aliases. Full Linux coverage passes every category and the +candidate's own changed-line/branch gates (61/61 and 28/28). An inherited missing special-member +warning and duplicate native syscall status handling are corrected without weakening policy. + +The exact member-size oracle also reproduces an existing streamed-ZIP data-descriptor limitation +on the unchanged parent. Keep that correctness task separate. O19b ZIP seeking/indexing, repeated +member/replay reuse and decoder admission remain open; forward skipping does not qualify them. diff --git a/extra_modules/archive/archive_reader.cc b/extra_modules/archive/archive_reader.cc index 8c1713a187e..75883b87399 100644 --- a/extra_modules/archive/archive_reader.cc +++ b/extra_modules/archive/archive_reader.cc @@ -45,6 +45,7 @@ namespace { // The streaming block size for both the header walk and the content read: one value, so the two // entry points cannot drift apart. constexpr std::size_t kBlockSize = std::size_t{64} * 1'024; +constexpr std::uint64_t kMinimumSkipSize = 64UZ * 1'024; // `archive_read_free` for a unique_ptr, so every early return releases the reader. struct ArchiveDeleter { @@ -212,6 +213,28 @@ struct SourceSession { // Declared last so the reader is released before its callback state. ArchivePtr handle; + // XFF_ABI_POINTER: libarchive requests forward byte skips through this C callback. + static la_int64_t Skip(struct ::archive*, void* data, la_int64_t requested) { + // libarchive's client_skip_proxy invokes this callback only for positive requests. + const auto limit = static_cast(requested); + if (limit < kMinimumSkipSize) { + return 0; + } + auto& self = *static_cast(data); + const auto skipped = self.stream->Skip(limit); + if (!skipped.ok()) { + self.error = skipped.status(); + // Return zero to leave the skip proxy; its fallback Read propagates the latched error. + return 0; + } + const std::uint64_t count = skipped->value_or(0); + if (count > limit) { + self.error = absl::DataLossError("archive cursor skipped more bytes than requested"); + return 0; + } + return static_cast(count); + } + // Views remain valid until the next refill or session destruction. absl::StatusOr Refill() { if (!stream->SupportsReadInto()) { @@ -264,6 +287,9 @@ 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); + if (!self.error.ok()) { + return -1; + } auto result = self.Refill(); if (!result.ok()) { self.error = result.status(); @@ -291,7 +317,9 @@ absl::StatusOr> OpenSourceSession(const vfs::Shar if (prefix.bytes == "xar!") { ::archive_read_set_seek_callback(session->handle.get(), &SourceSession::Seek); } - if (::archive_read_open(session->handle.get(), session.get(), nullptr, &SourceSession::Read, nullptr) != ARCHIVE_OK) { + if (::archive_read_open2( + session->handle.get(), session.get(), nullptr, &SourceSession::Read, &SourceSession::Skip, nullptr) + != ARCHIVE_OK) { MBO_RETURN_IF_ERROR(session->error); if (session->prefix == "xar!") { return absl::DataLossError(LastError(session->handle.get())); diff --git a/extra_modules/archive/archive_source_buffer_test.cc b/extra_modules/archive/archive_source_buffer_test.cc index d2e8e5e5224..66cfa0111c7 100644 --- a/extra_modules/archive/archive_source_buffer_test.cc +++ b/extra_modules/archive/archive_source_buffer_test.cc @@ -6,12 +6,15 @@ #include #include +#include #include #include +#include #include #include #include #include +#include #include "absl/status/status.h" #include "absl/status/statusor.h" @@ -29,6 +32,7 @@ namespace { using ::mbo::testing::EqualsText; using ::mbo::testing::IsOkAndHolds; using ::mbo::testing::StatusIs; +using ::testing::Each; using ::testing::ElementsAre; using ::testing::Eq; using ::testing::Field; @@ -38,6 +42,8 @@ using ::testing::IsEmpty; using ::testing::IsTrue; using ::testing::Le; using ::testing::Lt; +using ::testing::Not; +using ::testing::Optional; constexpr std::size_t kReadBlock = 64UZ * 1'024; @@ -47,6 +53,9 @@ struct ReadOptions { std::size_t fail_at = std::numeric_limits::max(); bool oversized_count = false; bool alternate_capability = false; + bool direct_skip = false; + bool fail_skip = false; + bool oversized_skip = false; }; struct ReadCounts { @@ -54,12 +63,20 @@ struct ReadCounts { std::size_t caller_owned = 0; std::size_t eof = 0; bool stable_storage = true; + std::size_t bytes_read = 0; + std::size_t skip_calls = 0; + std::vector positions; + std::optional reads_at_error; }; 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)) {} + CountingStream( + std::shared_ptr bytes, + ReadOptions options, + std::shared_ptr counts, + std::size_t offset = 0) + : bytes_(std::move(bytes)), options_(options), counts_(std::move(counts)), offset_(offset) {} absl::StatusOr Read(std::size_t max_bytes) override { ++counts_->owned; @@ -70,6 +87,7 @@ class CountingStream final : public vfs::ReadStream { const auto count = std::min({max_bytes, options_.chunk, bytes_->size() - offset_}); auto result = bytes_->substr(offset_, count); offset_ += count; + counts_->bytes_read += count; return result; } @@ -97,10 +115,30 @@ class CountingStream final : public vfs::ReadStream { 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_->bytes_read += count; counts_->eof += count == 0 ? 1 : 0; return count; } + absl::StatusOr> Skip(std::uint64_t requested) override { + ++counts_->skip_calls; + if (!options_.direct_skip) { + return std::nullopt; + } + if (options_.fail_skip) { + counts_->reads_at_error = counts_->owned + counts_->caller_owned; + return absl::DataLossError("injected cursor skip failure"); + } + if (options_.oversized_skip) { + counts_->reads_at_error = counts_->owned + counts_->caller_owned; + return std::optional(requested + 1); + } + const auto count = std::min(requested, bytes_->size() - offset_); + offset_ += static_cast(count); + counts_->positions.push_back(offset_); + return std::optional(count); + } + private: std::shared_ptr bytes_; ReadOptions options_; @@ -164,6 +202,94 @@ struct ArchiveSourceBufferTest : ::testing::Test { } }; +TEST_F(ArchiveSourceBufferTest, ListingSkipsLargeDirectPayloadAndReleasesTheSessionReservation) { + const auto counts = std::make_shared(); + const auto budget = std::make_shared(2 * kReadBlock, 0); + const std::string content(4 * kReadBlock, 'x'); + const auto source = + std::make_shared(MakeTar(content), ReadOptions{.direct_skip = true}, counts, budget); + EXPECT_THAT( + ListMembersOfSource(source), IsOkAndHolds(ElementsAre(Field("path", &Member::path, EqualsText("data.txt"))))); + EXPECT_THAT(counts->skip_calls, Eq(1)); + EXPECT_THAT(counts->positions, Not(IsEmpty())); + EXPECT_THAT(counts->bytes_read, Lt(content.size() / 2)); + EXPECT_THAT(budget->MemoryUsed(), Eq(0)); +} + +TEST_F(ArchiveSourceBufferTest, DirectSkippingAlsoRetainsOwnedBufferCursors) { + const auto counts = std::make_shared(); + const std::string content(4 * kReadBlock, 'x'); + const auto source = std::make_shared( + MakeTar(content), ReadOptions{.caller_owned = false, .direct_skip = true}, counts); + EXPECT_THAT( + ListMembersOfSource(source), IsOkAndHolds(ElementsAre(Field("path", &Member::path, EqualsText("data.txt"))))); + EXPECT_THAT(counts->owned, Gt(0)); + EXPECT_THAT(counts->caller_owned, Eq(0)); + EXPECT_THAT(counts->bytes_read, Lt(content.size() / 2)); +} + +TEST_F(ArchiveSourceBufferTest, ReadingSelectedContentDoesNotSkipItsBytes) { + for (const bool caller_owned : {false, true}) { + const auto counts = std::make_shared(); + const std::string content(4 * kReadBlock, 'x'); + const auto source = std::make_shared( + MakeTar(content), ReadOptions{.caller_owned = caller_owned, .direct_skip = true}, counts); + const auto member = MemberReadSource(source, "data.txt"); + EXPECT_THAT(vfs::ReadSourceBytes(*member, content.size()), IsOkAndHolds(EqualsText(content))); + EXPECT_THAT(counts->skip_calls, Eq(0)); + } +} + +TEST_F(ArchiveSourceBufferTest, SmallAndCompressedInputsAvoidOptionalSkipCalls) { + for (const bool compressed : {false, true}) { + const auto counts = std::make_shared(); + const auto source = + std::make_shared(MakeTar("small", compressed), ReadOptions{.direct_skip = true}, counts); + EXPECT_THAT( + ListMembersOfSource(source), IsOkAndHolds(ElementsAre(Field("path", &Member::path, EqualsText("data.txt"))))); + EXPECT_THAT(counts->skip_calls, Eq(0)); + } +} + +TEST_F(ArchiveSourceBufferTest, UnsupportedSourcesReadSequentiallyWithoutSizeScansOrReplay) { + const auto counts = std::make_shared(); + const auto budget = std::make_shared(2 * kReadBlock, 0); + const std::string content(4 * kReadBlock, 'x'); + const auto source = std::make_shared(MakeTar(content), ReadOptions{}, counts, budget); + EXPECT_THAT( + ListMembersOfSource(source), IsOkAndHolds(ElementsAre(Field("path", &Member::path, EqualsText("data.txt"))))); + EXPECT_THAT(counts->skip_calls, Eq(1)); + EXPECT_THAT(counts->positions, IsEmpty()); + EXPECT_THAT(counts->bytes_read, Gt(content.size())); + EXPECT_THAT(budget->MemoryUsed(), Eq(0)); +} + +TEST_F(ArchiveSourceBufferTest, SkipFailureIsLatchedBeforeTheFallbackReadsAgain) { + const auto counts = std::make_shared(); + const auto source = std::make_shared( + MakeTar(std::string(4 * kReadBlock, 'x')), ReadOptions{.direct_skip = true, .fail_skip = true}, counts); + EXPECT_THAT(ListMembersOfSource(source), StatusIs(absl::StatusCode::kDataLoss, HasSubstr("cursor skip failure"))); + EXPECT_THAT(counts->reads_at_error, Optional(Eq(counts->owned + counts->caller_owned))); +} + +TEST_F(ArchiveSourceBufferTest, OversizedSkipCountIsRejectedBeforeTheFallbackReadsAgain) { + const auto counts = std::make_shared(); + const auto source = std::make_shared( + MakeTar(std::string(4 * kReadBlock, 'x')), ReadOptions{.direct_skip = true, .oversized_skip = true}, counts); + EXPECT_THAT(ListMembersOfSource(source), StatusIs(absl::StatusCode::kDataLoss, HasSubstr("skipped more bytes"))); + EXPECT_THAT(counts->reads_at_error, Optional(Eq(counts->owned + counts->caller_owned))); +} + +TEST_F(ArchiveSourceBufferTest, DirectSkipStopsAtPhysicalBoundsAndRetainsTruncationErrors) { + std::string bytes = MakeTar(std::string(4 * kReadBlock, 'x')); + bytes.resize((2 * kReadBlock) + 123); + const auto length = bytes.size(); + const auto counts = std::make_shared(); + const auto source = std::make_shared(std::move(bytes), ReadOptions{.direct_skip = true}, counts); + EXPECT_THAT(ListMembersOfSource(source), StatusIs(absl::StatusCode::kDataLoss)); + EXPECT_THAT(counts->positions, Each(Le(length))); +} + TEST_F(ArchiveSourceBufferTest, ReusesTheCallerOwnedBufferForFullAndShortBlocks) { const auto counts = std::make_shared(); const auto source = diff --git a/release-site.json b/release-site.json index 17a391f7aae..eab5e956b92 100644 --- a/release-site.json +++ b/release-site.json @@ -82,7 +82,8 @@ "docs/benchmarks/caller-owned-streams-20261103.md": "docs/benchmarks/caller-owned-streams-20261103.html", "docs/benchmarks/direct-source-materialization-20261103.md": "docs/benchmarks/direct-source-materialization-20261103.html", "docs/benchmarks/hash-field-binding-20261103.md": "docs/benchmarks/hash-field-binding-20261103.html", - "docs/benchmarks/hash-field-binding-tag-20261103.md": "docs/benchmarks/hash-field-binding-tag-20261103.html" + "docs/benchmarks/hash-field-binding-tag-20261103.md": "docs/benchmarks/hash-field-binding-tag-20261103.html", + "docs/benchmarks/forward-cursor-skipping-20261103.md": "docs/benchmarks/forward-cursor-skipping-20261103.html" }, "generated_html": { "XFF.md": { diff --git a/tools/archive_source_benchmark.cc b/tools/archive_source_benchmark.cc index 32fbec440e7..b5da9a6d8e5 100644 --- a/tools/archive_source_benchmark.cc +++ b/tools/archive_source_benchmark.cc @@ -131,6 +131,21 @@ void Measure(benchmark::State& state, SourceKind kind, bool read_last) { const auto source = std::move(*built_source); const auto member = MemberReadSource(source, absl::StrCat("file-", members - 1)); const std::string expected(payload, 'x'); + const auto listing = ListMembersOfSource(source); + if (!listing.ok() || listing->size() != members) { + state.SkipWithError("initial archive listing differs from fixture"); + return; + } + for (std::size_t index = 0; index < members; ++index) { + const auto& entry = listing->at(index); + if (entry.path != absl::StrCat("file-", index) || std::cmp_not_equal(entry.size, payload)) { + const auto error = absl::StrCat( + "archive listing differs at ", index, ": path=", entry.path, " size=", entry.size, + " expected-size=", payload); + state.SkipWithError(error); + return; + } + } // NOLINTNEXTLINE(clang-analyzer-deadcode.DeadStores): benchmark loop controls the measured iterations. for (const auto unused : state) { if (read_last) { @@ -152,12 +167,28 @@ void Measure(benchmark::State& state, SourceKind kind, bool read_last) { state.SetItemsProcessed(state.iterations() * static_cast(members)); } -void RegisterAll() { - struct SourceCase { - std::string_view name; - SourceKind kind; - }; +struct SourceCase { + std::string_view name; + SourceKind kind; +}; + +void RegisterLarge(std::span sources) { + for (const auto& source : sources) { + for (const bool read_last : {false, true}) { + const std::string name = absl::StrCat("archive-large/tar/", source.name, "/", read_last ? "last" : "list"); + // XFF_ABI_POINTER: Google Benchmark returns a borrowed mutable registration object. + auto* registration = benchmark::RegisterBenchmark(name, Measure, source.kind, read_last); + for (const auto count : {10, 100}) { + for (const auto payload : {4'096, 65'536, 1'048'576}) { + registration->Args({count, payload}); + } + } + registration->Unit(benchmark::kMicrosecond)->UseRealTime()->MinTime(0.02)->MinWarmUpTime(0.01)->Repetitions(9); + } + } +} +void RegisterAll() { constexpr auto kSources = std::to_array({ {.name = "legacy", .kind = SourceKind::kLegacy}, {.name = "caller-owned", .kind = SourceKind::kCallerOwned}, @@ -179,6 +210,7 @@ void RegisterAll() { registration->Unit(benchmark::kMicrosecond)->UseRealTime()->MinTime(0.02)->MinWarmUpTime(0.01)->Repetitions(9); } } + RegisterLarge(kSources); } } // namespace diff --git a/xff_extras_api/read_source.cc b/xff_extras_api/read_source.cc index 75da835353a..b930ef4f33e 100644 --- a/xff_extras_api/read_source.cc +++ b/xff_extras_api/read_source.cc @@ -72,6 +72,12 @@ class MemoryStream final : public ReadStream { return count; } + absl::StatusOr> Skip(std::uint64_t requested) override { + const auto count = std::min(requested, bytes_->size() - offset_); + offset_ += static_cast(count); + return std::optional(count); + } + private: std::shared_ptr bytes_; std::size_t offset_ = 0; @@ -94,9 +100,14 @@ class MemorySource final : public ReadSource { std::shared_ptr bytes_; }; +// XFF_HOST_IO: translate an immediately preceding read-only syscall; errno is read only on failure. +absl::Status HostReadStatus(std::int64_t result, std::string_view operation) { + return result < 0 ? absl::ErrnoToStatus(errno, operation) : absl::OkStatus(); +} + class HostStream final : public ReadStream { public: - explicit HostStream(int fd) : fd_(fd) {} + HostStream(int fd, std::uint64_t offset) : fd_(fd), position_(offset) {} // XFF_HOST_IO: releases the exclusively owned read-only descriptor. ~HostStream() override { ::close(fd_); } @@ -114,6 +125,7 @@ class HostStream final : public ReadStream { return absl::DataLossError(absl::ErrnoToStatus(errno, "reading container source failed").message()); } result.resize(static_cast(count)); + position_ += static_cast(count); return result; } @@ -125,11 +137,44 @@ class HostStream final : public ReadStream { if (count < 0) { return absl::DataLossError(absl::ErrnoToStatus(errno, "reading container source failed").message()); } + position_ += static_cast(count); return static_cast(count); } + // XFF_HOST_IO: queries and advances only this exclusively owned read-only descriptor. + absl::StatusOr> Skip(std::uint64_t requested) override { + if (requested == 0) { + return std::optional(0); + } + if (access_ == Access::kUnknown) { + // XFF_HOST_IO: descriptor metadata is queried lazily only for an eligible forward skip. + struct stat metadata{}; + // XFF_HOST_IO: inspect the owned descriptor, rather than reopening or probing its path. + MBO_RETURN_IF_ERROR(HostReadStatus(::fstat(fd_, &metadata), "cannot stat container cursor")); + access_ = S_ISREG(metadata.st_mode) ? Access::kRegular : Access::kSequential; + size_ = static_cast(metadata.st_size); + } + if (access_ != Access::kRegular) { + return std::nullopt; + } + if (position_ >= size_) { + return std::optional(0); + } + const std::uint64_t count = std::min(requested, size_ - position_); + // Both the current offset and cached regular-file size fit off_t; count is bounded by size. + // XFF_HOST_IO: change the read cursor position without modifying any host bytes. + MBO_RETURN_IF_ERROR(HostReadStatus( + ::lseek(fd_, static_cast(position_ + count), SEEK_SET), "cannot advance container cursor")); + position_ += count; + return std::optional(count); + } + private: + enum class Access { kUnknown, kSequential, kRegular }; int fd_; + Access access_ = Access::kUnknown; + std::uint64_t position_ = 0; + std::uint64_t size_ = 0; }; class HostSource final : public ReadSource { @@ -144,9 +189,7 @@ class HostSource final : public ReadSource { // XFF_HOST_IO: local metadata record for this read-only host adapter. struct stat metadata{}; // XFF_HOST_IO: query metadata for the read-only source path. - if (::stat(path_.c_str(), &metadata) != 0) { - return absl::ErrnoToStatus(errno, "cannot stat container source"); - } + MBO_RETURN_IF_ERROR(HostReadStatus(::stat(path_.c_str(), &metadata), "cannot stat container source")); return static_cast(metadata.st_size); } @@ -163,7 +206,7 @@ class HostSource final : public ReadSource { if (fd < 0) { return absl::ErrnoToStatus(errno, absl::StrCat("cannot open container source: ", path_)); } - auto stream = std::make_unique(fd); + auto stream = std::make_unique(fd, offset); if (offset != 0 && ::lseek(fd, static_cast(offset), SEEK_SET) < 0) { return absl::ErrnoToStatus(errno, "cannot seek container source"); } diff --git a/xff_extras_api/read_source.h b/xff_extras_api/read_source.h index 49be458826f..41e78ba545c 100644 --- a/xff_extras_api/read_source.h +++ b/xff_extras_api/read_source.h @@ -23,7 +23,12 @@ namespace xff::vfs { // Cursors are independent and single-threaded. Read must return at most max_bytes bytes. class ReadStream { public: + ReadStream() = default; virtual ~ReadStream() = default; + ReadStream(const ReadStream&) = delete; + ReadStream& operator=(const ReadStream&) = delete; + ReadStream(ReadStream&&) = delete; + ReadStream& operator=(ReadStream&&) = delete; virtual absl::StatusOr Read(std::size_t max_bytes) = 0; // Optional allocation-free reads into caller-owned storage. Only [0, returned count) is @@ -34,6 +39,11 @@ class ReadStream { virtual absl::StatusOr ReadInto(std::span) { return absl::UnimplementedError("cursor does not support caller-owned read buffers"); } + + // Optional forward skipping without reading/discarding bytes. A present count is at most + // requested and may be short; zero alone does not prove EOF. Absence means unsupported. + // All operations share this cursor's position. The default never reads or advances. + virtual absl::StatusOr> Skip(std::uint64_t) { return std::nullopt; } }; // An immutable, restartable source. Open can be called concurrently; each cursor owns everything @@ -44,6 +54,10 @@ class ReadSource { : budget_(std::move(budget)) {} virtual ~ReadSource() = default; + ReadSource(const ReadSource&) = default; + ReadSource& operator=(const ReadSource&) = default; + ReadSource(ReadSource&&) noexcept = default; + ReadSource& operator=(ReadSource&&) noexcept = default; virtual absl::StatusOr> Open() const = 0; // Native sources seek directly; sequential sources replay within the shared work budget. // Positions beyond EOF yield an empty stream. Failures remain errors. diff --git a/xff_extras_api/read_source_test.cc b/xff_extras_api/read_source_test.cc index 6fd818c6ccc..b733d344fec 100644 --- a/xff_extras_api/read_source_test.cc +++ b/xff_extras_api/read_source_test.cc @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -50,6 +51,51 @@ class OwnedOnlyStream final : public ReadStream { std::size_t offset_ = 0; }; +// Cursor ownership transfers through unique_ptr; duplicating the object can duplicate a native descriptor. +static_assert(!std::is_copy_constructible_v); +static_assert(!std::is_move_constructible_v); + +TEST_F(ReadSourceTest, UnsupportedSkipNeverConsumesOwnedOnlyStream) { + OwnedOnlyStream stream; + EXPECT_THAT(stream.Skip(3), IsOkAndHolds(Eq(std::nullopt))); + EXPECT_THAT(stream.Read(6), IsOkAndHolds(EqualsText("abcdef"))); +} + +TEST_F(ReadSourceTest, MemorySkipSharesReadPositionAndBoundsCountsWithoutLosingOwners) { + auto source = MemoryReadSource("abcdef"); + MBO_ASSERT_OK_AND_ASSIGN(auto cursor, source->Open()); + MBO_ASSERT_OK_AND_ASSIGN(auto second, source->OpenAt(1)); + source.reset(); + EXPECT_THAT(cursor->Skip(0), IsOkAndHolds(Optional(Eq(0)))); + EXPECT_THAT(cursor->Read(1), IsOkAndHolds(EqualsText("a"))); + EXPECT_THAT(cursor->Skip(2), IsOkAndHolds(Optional(Eq(2)))); + std::array buffer; + EXPECT_THAT(cursor->ReadInto(buffer), IsOkAndHolds(Eq(3))); + EXPECT_THAT(std::string_view(buffer.data(), buffer.size()), EqualsText("def")); + EXPECT_THAT(cursor->Skip(std::numeric_limits::max()), IsOkAndHolds(Optional(Eq(0)))); + EXPECT_THAT(second->Skip(std::numeric_limits::max()), IsOkAndHolds(Optional(Eq(5)))); + EXPECT_THAT(second->Read(1), IsOkAndHolds(IsEmpty())); +} + +TEST_F(ReadSourceTest, HostSkipUsesTheOwnedDescriptorAndPreservesUnsupportedReadErrors) { + MBO_ASSERT_OK_AND_ASSIGN(auto directory, TemporaryDirectory::Create(::testing::TempDir() + "skip-source", {})); + MBO_ASSERT_OK_AND_ASSIGN(auto output, TemporaryOutput::Create(directory->Path() + "/file", {})); + ASSERT_THAT(output->Write("abcdef"), IsOk()); + MBO_ASSERT_OK_AND_ASSIGN(auto cursor, HostReadSource(output->Path())->OpenAt(1)); + MBO_ASSERT_OK_AND_ASSIGN(auto sequential, HostReadSource(directory->Path())->Open()); + output.reset(); + directory.reset(); + EXPECT_THAT(cursor->Skip(0), IsOkAndHolds(Optional(Eq(0)))); + EXPECT_THAT(cursor->Skip(2), IsOkAndHolds(Optional(Eq(2)))); + EXPECT_THAT(cursor->Read(1), IsOkAndHolds(EqualsText("d"))); + EXPECT_THAT(cursor->Skip(std::numeric_limits::max()), IsOkAndHolds(Optional(Eq(2)))); + EXPECT_THAT(cursor->Read(1), IsOkAndHolds(IsEmpty())); + EXPECT_THAT(cursor->Skip(1), IsOkAndHolds(Optional(Eq(0)))); + EXPECT_THAT(sequential->Skip(10), IsOkAndHolds(Eq(std::nullopt))); + EXPECT_THAT(sequential->Skip(10), IsOkAndHolds(Eq(std::nullopt))); + EXPECT_THAT(sequential->Read(1), StatusIs(absl::StatusCode::kDataLoss)); +} + TEST_F(ReadSourceTest, UnsupportedReadIntoNeitherConsumesNorCopiesOwnedOnlyStream) { OwnedOnlyStream stream; EXPECT_THAT(stream.SupportsReadInto(), IsFalse());