diff --git a/CHANGELOG.md b/CHANGELOG.md index 24071bf6c1f..743414a8b95 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,8 @@ # 0.11.0 +- Reduce temporary allocations and copies when lazily decoding the embedded language database. + - Transfer validated rg output directly into matcher results, reusing retained buffers while preserving native callback boundaries, binary suppression, and read-error behavior. diff --git a/TODO.md b/TODO.md index fbb0a6612d6..f1dd364d6c2 100644 --- a/TODO.md +++ b/TODO.md @@ -1437,9 +1437,12 @@ this as intentional accounting. Both hermetic Clang 23.1.4 libc++ implementations only read/write the prior-state byte. Keep the current runtime; no buffering change or measured gain follows from adding the call. Revisit for a different standard-library implementation or output architecture. -- [ ] O25: Independently measure direct final-string decoding and owned-result transfer for - the lazy embedded language JSON. The current payload is 109,261 bytes, not an ordinary - startup megabyte cost. Preserve corruption/length errors and narrow ABI adapters. +- [x] O25: Integrate qualified direct final-string decoding and owned-result transfer for + the lazy 109,261-byte language JSON. Current actual-function kernels improve the combined + decode/retention path 9.87% Mac/3.90% Linux; fresh whole-command controls remain inconclusive. + Both 323-target release suites, both focused ASan suites and Linux MSan pass. Preserve + corruption/length errors and narrow ABI adapters; current-main PR CI remains required. + Retain [all variants, source identities and raw data](docs/benchmarks/language-json-storage-20261107.md). - [x] O28: [Evaluate compile-time archive observation mode](docs/benchmarks/archive-observation-mode-20261105.md). Actual release assembly retains the dynamic mode branch, but its removal fails to establish a consistent cross-platform standalone benefit. Preserve both complete screens, diff --git a/docs/benchmarks/language-json-storage-20261107.json.gz b/docs/benchmarks/language-json-storage-20261107.json.gz new file mode 100644 index 00000000000..39256a5b509 Binary files /dev/null and b/docs/benchmarks/language-json-storage-20261107.json.gz differ diff --git a/docs/benchmarks/language-json-storage-20261107.md b/docs/benchmarks/language-json-storage-20261107.md new file mode 100644 index 00000000000..4d00844bc46 --- /dev/null +++ b/docs/benchmarks/language-json-storage-20261107.md @@ -0,0 +1,127 @@ +# Final language JSON storage, 2026-11-07 + +Decode the embedded language database directly into its final owning string, then +move that string into lazy static storage. The 109,261-byte database previously +passed through a zero-initialized byte vector, a separate string, and another owned +string copy. This is a small one-time ownership cleanup. It does not establish a +whole-command startup speedup, and ordinary listing does not initialize this database. + +## Implementation and contracts + +`std::string::resize_and_overwrite` gives the existing Brotli C decoder writable final +storage without first filling it with zeroes. The required C callback/output pointer +stays inside a narrowly annotated adapter. The callback returns only the successfully +written prefix, or zero on failure. Invalid data and exact decoded-length mismatch +retain their existing `DataLoss` statuses. Empty valid payloads and an undersized +destination are regression-tested. No unwritten byte is read or published. + +The lazy initializer uses a mutable `StatusOr` solely to move its owned value. +The static string still owns the returned database view. No environment/configuration, +public API or CLI behavior changes. There is no new runtime wrapper, permanent benchmark +target or benchmark module dependency. + +## Current production decoder benchmark + +Fresh baseline/candidate binaries call each tree's actual production `Decode` through +an identical harness and byte-exact copy of the existing compressed payload. Source, +configuration, corpus and image hashes are retained. Both use `clang_release` and +`xff_docs`, without sanitizers. The temporary root/manual target uses the existing +Google Benchmark dependency; its rule and sources are archived and were removed +from both worktrees before preparing the public change. + +Each host completes three sessions of nine repetitions per case/participant, retaining +the fastest seven and averaging them per session. Participant order rotates by +session, with per-case random interleaving inside the benchmark. The control repeats +the exact baseline image. Linux is pinned to one allowed CPU; Mac is unpinned. No +build, lint or sanitizer runs concurrently with timing on the same host. Positive +percentages are slower. + +| Same retained ownership behavior | Mac candidate | Mac baseline repeat | Linux candidate | Linux baseline repeat | +| :------------------------------- | ------------: | ------------------: | --------------: | --------------------: | +| Decode followed by owned copy | -6.05% | -0.12% | -2.87% | -0.12% | +| Decode followed by move | -5.09% | +0.79% | -2.54% | -0.62% | + +The same-method comparisons isolate the decoder storage change. Comparing the old +production-style decode-plus-copy path with the new decode-plus-move path improves +9.87% on Mac and 3.90% on Linux. This latter comparison includes both storage and +owned-result transfer; it does not isolate one mechanism or claim a CLI speedup. +All raw repetitions, session values and both slower/faster control movements remain +in the artifact. No native RSS or peak-memory value was measured. + +## Earlier decode-stage diagnostics + +The corrected research harness compares the former vector/decode/string/copy sequence +with production direct decoding followed by either another owned copy or a move. The +November 6 populations below have nine repetitions per case on each host; these values +are the Google Benchmark median real times. They are not the separate three-session +zero-initialized-string experiment, which is also preserved under its own name. + +| Diagnostic | Mac microseconds | Linux microseconds | +| :------------------------------------------- | ---------------: | -----------------: | +| Former vector, decoded string, retained copy | 158.611 | 108.313 | +| Direct final-buffer decode, retained copy | 151.908 | 95.547 | +| Direct final-buffer decode, retained move | 146.704 | 93.534 | + +Linux was pinned to CPU 2; Mac was unpinned. These are modeled initialization kernels, +not current-base CLI measurements. The retained v2 JSON identifies the executable path +and benchmark-library build but lacks a complete source/compiler/image-hash receipt; +its research harness and original plan are preserved as supporting diagnostics rather +than final current-head performance qualification. No native RSS or peak-memory value +was measured. The removed vector/owned copies are visible in the source change; the +kernel results do not quantify overall application memory use. + +## Current-base whole-command controls + +Fresh baseline and candidate full CLI binaries were built from `9bf1aad5` using +`clang_release` and `xff_docs`, with source and image hashes recorded. A native +fork/exec/wait launcher excludes Python launcher startup from the measured interval; +the complete xff invocation remains included. Each of three sessions has nine +rotating baseline/identical-baseline-repeat/candidate rounds after discarded warmups. +Retain the fastest seven per case/session, then report the median paired change. +Linux is pinned to one allowed CPU; Mac is unpinned. No build or sanitizer runs on +the measured host during timing. Every invocation passes a complete output SHA-256 +oracle; the listing fixture has 100 known files, and the full type list includes C++. +Positive percentages are slower. + +| Command | Mac candidate | Mac same-binary repeat | Linux candidate | Linux same-binary repeat | +| :-------------- | ------------: | ---------------------: | --------------: | -----------------------: | +| type-list | -0.24% | +1.26% | +0.58% | +0.36% | +| listing-control | +1.28% | -2.07% | +0.59% | -0.01% | + +These controls establish no material whole-command improvement. The 500-launch +historical Mac summary was also inconclusive, but does not retain its individual +samples or complete measured-source identity, so it is preserved separately and +is not substituted for this current-base population. Every current raw sample, +per-session aggregate and oracle digest is included. + +Mac full images are both 8,275,000 bytes. Linux changes from 18,443,224 to 18,442,904 +bytes (-320). These unstripped experiment images are not release-download sizes. +The two binaries within each host have distinct hashes despite matching Mac sizes. + +## Correctness and remaining integration + +Both hosts pass all 323 core plus registered-extension release targets under +`clang_release` and `xff_docs`; Linux requires real FUSE tests. Both pass canonical +Clang+ASan language registration, language matching and language-database integration +targets. Linux also passes those three targets under MSan, checking the newly +uninitialized destination buffer path. Exact commands and complete logs are retained. +A sandbox-only Mac invocation failed before Bazel analysis because it could not +write the existing cache; the authorized invocation passed. That setup failure is +kept separately rather than described as a candidate test failure. + +The new destination-boundary/empty-payload tests and existing invalid-data/size +checks pass. Test-only formatting was applied after development runs; current-head +PR CI, including changed-source clang-tidy, GCC and coverage, remains required. +No local clang-tidy was run. The successor integration on `c4d21aea2f6640386d49c4a2f66503472bd64f96` +(#1127) passes the three optimized language targets on Mac. Module implementation, +regression source and BUILD inputs match the qualified payload. This is a development +integration check, separate from the future pushed-head PR context. The current production-function results qualify a focused ownership cleanup; +no user-visible speedup or measured RSS gain is promised. + +## Evidence + +[`language-json-storage-20261107.json.gz`](language-json-storage-20261107.json.gz) is 154,700 bytes, SHA-256 +`59d7645aea9397b4bba4443a903e5e3dfd295ca590ac2333a4f816288ab431d8`. Its 108 source/raw/log records retain exact byte counts and hashes. +The earlier zero-initialized-string artifact remains byte-exact, encoded separately +inside the bundle. Full executables stay in the local/remote task directories; +their recorded hashes identify the measured images. diff --git a/docs/performance-analysis.md b/docs/performance-analysis.md index cffa65b4306..e7c7bdbb356 100644 --- a/docs/performance-analysis.md +++ b/docs/performance-analysis.md @@ -4796,3 +4796,27 @@ and four focused canonical ASan targets. These development results require the current integration PR's own coverage, sanitizer, GCC and CI clang-tidy checks. O18's byte admission, ordered completion, oversized output and cancellation work remains open. + +### O25: final language database string storage + +[Current production-function qualification](benchmarks/language-json-storage-20261107.md) +compares the actual baseline/candidate decoder with identical byte-exact fixtures, +source/configuration/image identities and a repeated-baseline control. Three sessions +of nine repetitions retaining seven show direct-buffer decode-plus-copy gains of +6.05% on Mac and 2.87% on Linux. Comparing the old decode/retained-copy path with the +new decode/retained-move path improves 9.87%/3.90%; that comparison includes both +mechanisms. The lazy payload is 109,261 bytes and ordinary listing does not load it. + +Current full-command type-list changes are -0.24% Mac/+0.58% Linux, with same-binary +controls +1.26%/+0.36%. Unchanged listing costs +1.28%/+0.59% remain visible. There is +no material startup-speed or measured RSS claim. Earlier kernels and an inconclusive +500-launch summary remain separately labeled with their provenance limitations. +Temporary benchmark rules/fixtures were archived and removed; no module dependency +or permanent research target is added. + +Both hosts pass all 323 documented-surface/core/registered-extension release targets +and three relevant canonical ASan targets; Linux also passes those three under MSan. +Empty valid output, too-small destinations, corrupt data and exact-size mismatch +retain committed coverage. Current integration PR checks remain authoritative. This +small ownership cleanup does not complete the broader immutable-catalog/first-parse +investigation or imply a user-visible command improvement. diff --git a/extra_modules/language_db/BUILD.bazel b/extra_modules/language_db/BUILD.bazel index 6b78554ee3f..d98745a59c2 100644 --- a/extra_modules/language_db/BUILD.bazel +++ b/extra_modules/language_db/BUILD.bazel @@ -49,6 +49,7 @@ cc_test( ":language_db_data_cc", ":language_db_register_cc", "@googletest//:gtest_main", + "@mboworks_mbo//mbo/testing:matchers_cc", "@mboworks_mbo//mbo/testing:status_cc", "@xff_extras_api//:language_database_cc", "@xff_extras_api//:license_notice_cc", diff --git a/extra_modules/language_db/language_database.cc b/extra_modules/language_db/language_database.cc index b18f04c2601..df37268d831 100644 --- a/extra_modules/language_db/language_database.cc +++ b/extra_modules/language_db/language_database.cc @@ -10,7 +10,7 @@ #include #include #include -#include +#include #include "absl/log/check.h" #include "absl/status/status.h" @@ -21,10 +21,18 @@ namespace xff::language_db { absl::StatusOr Decode(std::span compressed, std::size_t expected_uncompressed_size) { - std::vector result(expected_uncompressed_size); - std::size_t size = result.size(); - const BrotliDecoderResult decode = - BrotliDecoderDecompress(compressed.size(), compressed.data(), &size, result.data()); + std::size_t size = 0; + BrotliDecoderResult decode = BROTLI_DECODER_RESULT_ERROR; + std::string result; + // XFF_ABI_POINTER: Brotli's C API writes decoded bytes through a uint8_t pointer. + result.resize_and_overwrite(expected_uncompressed_size, [&](char* buffer, std::size_t writable_bytes) noexcept { + size = writable_bytes; + decode = BrotliDecoderDecompress( + compressed.size(), compressed.data(), &size, + // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) -- Brotli writes into char storage as bytes. + reinterpret_cast(buffer)); + return decode == BROTLI_DECODER_RESULT_SUCCESS ? size : 0; + }); if (decode != BROTLI_DECODER_RESULT_SUCCESS) { return absl::DataLossError("the embedded language database is not valid Brotli data"); } @@ -33,16 +41,16 @@ absl::StatusOr Decode(std::span compressed, std absl::StrCat( "the embedded language database decoded to ", size, " bytes, expected ", expected_uncompressed_size)); } - return std::string(result.begin(), result.end()); + return result; } namespace { std::string_view Json() { static const std::string kJson = [] { - const absl::StatusOr json = Decode(data::Compressed(), data::UncompressedSize()); + absl::StatusOr json = Decode(data::Compressed(), data::UncompressedSize()); CHECK_OK(json); // LCOV_EXCL_BR_LINE: fatal invariant; generated payload is verified at build time. - return *json; + return std::move(*json); }(); // LCOV_EXCL_BR_LINE: compiler-generated static-local initialization branches. return kJson; } diff --git a/extra_modules/language_db/language_db_register_test.cc b/extra_modules/language_db/language_db_register_test.cc index db0fe5d7311..6dfca96bd75 100644 --- a/extra_modules/language_db/language_db_register_test.cc +++ b/extra_modules/language_db/language_db_register_test.cc @@ -2,11 +2,13 @@ // SPDX-License-Identifier: Apache-2.0 #include +#include #include "gmock/gmock.h" #include "gtest/gtest.h" #include "language_database.h" #include "language_db_data_cc_generated.h" +#include "mbo/testing/matchers.h" #include "mbo/testing/status.h" #include "xff/license/notice.h" #include "xff/matching/language/language_database_api.h" @@ -14,6 +16,7 @@ namespace xff::language_db { namespace { +using ::mbo::testing::EqualsText; using ::mbo::testing::IsOkAndHolds; using ::mbo::testing::StatusIs; using ::testing::Contains; @@ -23,8 +26,11 @@ using ::testing::HasSubstr; struct LanguageDbRegisterTest : ::testing::Test {}; TEST_F(LanguageDbRegisterTest, RegistersCompressedDataAndNotices) { - EXPECT_THAT(language::Databases(), Contains(Field(&language::Database::name, "github-linguist 9.6.0"))); - EXPECT_THAT(license::Notices(), Contains(Field(&license::Notice::component, "github-linguist 9.6.0"))); + EXPECT_THAT( + language::Databases(), Contains(Field("name", &language::Database::name, EqualsText("github-linguist 9.6.0")))); + EXPECT_THAT( + license::Notices(), + Contains(Field("component", &license::Notice::component, EqualsText("github-linguist 9.6.0")))); } TEST_F(LanguageDbRegisterTest, DecodesTheEmbeddedVocabulary) { @@ -36,10 +42,18 @@ TEST_F(LanguageDbRegisterTest, DecodesTheEmbeddedVocabulary) { TEST_F(LanguageDbRegisterTest, RejectsInvalidBrotliAndWrongDeclaredSize) { constexpr std::array kInvalid = {1, 2, 3}; EXPECT_THAT(Decode(kInvalid, 20), StatusIs(absl::StatusCode::kDataLoss, HasSubstr("not valid Brotli data"))); + EXPECT_THAT( + Decode(data::Compressed(), data::UncompressedSize() - 1), + StatusIs(absl::StatusCode::kDataLoss, HasSubstr("not valid Brotli data"))); EXPECT_THAT( Decode(data::Compressed(), data::UncompressedSize() + 1), StatusIs(absl::StatusCode::kDataLoss, HasSubstr("decoded to 109261 bytes, expected 109262"))); } +TEST_F(LanguageDbRegisterTest, DecodesAnEmptyPayloadIntoAZeroLengthString) { + constexpr std::array kCompressedEmpty = {0x3f}; + EXPECT_THAT(Decode(kCompressedEmpty, 0), IsOkAndHolds(EqualsText(""))); +} + } // namespace } // namespace xff::language_db diff --git a/release-site.json b/release-site.json index 5d681b2432b..eef458b3985 100644 --- a/release-site.json +++ b/release-site.json @@ -135,7 +135,8 @@ "docs/benchmarks/archive-member-keys-20261105.md": "docs/benchmarks/archive-member-keys-20261105.html", "docs/benchmarks/shared-stage-workers-20261107.md": "docs/benchmarks/shared-stage-workers-20261107.html", "docs/benchmarks/archive-index-insertion-20261107.md": "docs/benchmarks/archive-index-insertion-20261107.html", - "docs/benchmarks/transactional-rg-records-20261107.md": "docs/benchmarks/transactional-rg-records-20261107.html" + "docs/benchmarks/transactional-rg-records-20261107.md": "docs/benchmarks/transactional-rg-records-20261107.html", + "docs/benchmarks/language-json-storage-20261107.md": "docs/benchmarks/language-json-storage-20261107.html" }, "generated_html": { "XFF.md": { @@ -158,6 +159,7 @@ } ], "files": { - "docs/benchmarks/transactional-rg-records-20261107.json.gz": "docs/benchmarks/transactional-rg-records-20261107.json.gz" + "docs/benchmarks/transactional-rg-records-20261107.json.gz": "docs/benchmarks/transactional-rg-records-20261107.json.gz", + "docs/benchmarks/language-json-storage-20261107.json.gz": "docs/benchmarks/language-json-storage-20261107.json.gz" } }