Jeremy Braun

Codemod `ok_or_else(|| internal_error!(...))` to `Option::internal_error` Summary: Mechanical adoption of the `BuckErrorOptionContext` API from the previous diff: 345 call sites across 142 files convert `.ok_or_else(|| internal_error!(...))` into the documented spelling — `.internal_error("...")` for plain messages, `.with_internal_error(|| format!(...))` where the message has format arguments. Message shape (`... (internal error)`), `InternalError` tag, and caller source location are identical before and after. Import fallout handled: 109 now-dead `use buck2_error::internal_error;` lines removed (40 files keep it for remaining non-`Option` macro uses); imports scoped into `mod tests` where the only converted sites are `cfg(test)`-gated, so the lib build stays warning-free. One `#[cfg(windows)]` site (`buck2/src/check_user_allowed.rs`) is deliberately left unconverted since it cannot be compile-verified on Linux. Reviewed By: christolliday Differential Revision: D116804875 fbshipit-source-id: 879fcb502cb449cca4e7e7cf608c08ccff0e1819

Fix clippy warnings for fbcode//buck2/scripts:buck2-oss-lint Summary: Was breaking oss lints: https://www.internalfb.com/intern/test/562950276062710 `useless_borrows_in_formatting` is now included in clippy::all under the Rust 1.97 toolchain. Our internal clippy lints are -W only for this, but OSS are error all, so they need fixed. Reviewed By: NavidQar Differential Revision: D113775014 fbshipit-source-id: 5c05fda200a7154d8ca8668d8b9d9dbfa90dc9bd

buck2: add `buck2 log snoop` to watch in-progress commands' event logs Summary: Adds a tail-mode event log reader and a `buck2 log snoop` subcommand that tails the event log of a running command and renders it in a Superconsole, catching up on past events and then following along live — a way to watch a build's progress from another terminal. The tail reader (`TailReader` in `buck2_event_log`) retries on EOF instead of ending the stream, relying on the writer's per-tick flushes making the compressed stream incrementally decodable. It only reads through the already-open handle and never re-opens or locks the file, so the owning client remains free to delete the log (including on Windows, where the Rust standard library opens files with delete sharing). A deleted or abandoned log ends the stream after `--idle-timeout` seconds without growth. Event content always comes from log files, but when a daemon is already running, snoop additionally opens a read-only `SubscribeToActiveCommands` subscription (via `BuckdConnectOptions::ExistingOnly` — it never spawns a daemon) to know which commands are live: - The Superconsole header shows `(snoop <i>/<n> running: <command line>)` for the attached invocation, and the `Buck UI` link shows its trace id. - The replay speed keys switch invocations instead: `k` attaches to a newer running command and `j` to an older one (implemented via a snoop-specific `Clock` that reinterprets `scale_speed`, so no changes to the shared console input handling are needed). Without daemon info, or when viewing a finished invocation, they navigate the retained event logs instead. - By default snoop attaches to the newest running command; when nothing is running it waits for the next command to start rather than instantly replaying the newest finished log, and when the snooped command ends it keeps waiting indefinitely (Ctrl-C to exit) for whatever runs next. With an explicit `--trace-id`/`--recent`/path selector it replays that invocation and exits when it ends. `--idle-timeout` only bounds how long a non-growing log is retried before its command is treated as gone. Reviewed By: JakobDegen Differential Revision: D115659505 fbshipit-source-id: f808b7b8901f1dc03621316b840857177ab2e68b

Degrade corrupt-span resolution to diagnostics instead of panicking Summary: Starlark call-stack frames and compiler spans can, rarely, carry a `Span` that does not lie within their own `CodeMap`. Resolving one goes through `CodeMap::find_line` (an `assert!`) or `CodeMap::source_span` (a raw slice), and either panics — killing the buck2 daemon mid-command: in production while compiling BUCK files (`Compiler::eval_module`), and locally while formatting an unrelated PACKAGE-file error. P2457607755 catalogs the production occurrences and the open root-cause leads. The root cause is unknown, so this diff has three goals: stop the crashes, make each occurrence visible and auditable, and record enough detail to be a clue later. Stop the crashes. `CodeMap::find_line`, `find_line_col`, and `source_span` clamp instead of panicking: a position past the end resolves to the last line, and a span that is not a character range grows to the enclosing character boundaries rather than yielding nothing. Make them visible. Each clamp reports through a new reporter hook in `starlark_syntax`, which `starlark` bridges to a process-global `SoftErrorHandler` slot (`set_global_soft_error_handler`), which buck2 installs in `init_late_bindings` as the existing `Buck2StarlarkSoftErrorHandler` — so occurrences land in Scuba rather than vanishing. Contexts holding an `Evaluator` keep using the evaluator-scoped handler; the global slot exists for span resolution and freeze-time optimization, where none is in reach. `FrozenFileSpan::new` reports at construction too, catching corruption at the moment a bad pairing is built rather than when it is next read, and `Frame::write_two_lines` prints a self-describing line in place of the source snippet. Record clues. Reports name the resolution path, the offending position or span, the file length, and whether the `CodeMap` is a real source file or a native one — that last distinction matters, because a span resolved against a native codemap it cannot belong to is what a pointer landing on unrelated memory looks like, `<native>` being a fixed 8-byte source. Reporting is capped at eight occurrences per process, since resolution is hot and one corrupt span is typically resolved many times, but is not capped at one, so several distinct corruptions stay visible. Debug builds still fail fast at `FrozenFileSpan::new`. This intentionally does not fix the corruption itself. This is a mitigation, not a feature: a `TODO(jtbraun)` at the reporting machinery in `codemap.rs` marks the whole diff for back-out once the invalid spans are root caused. That is also why resolution clamps rather than surfacing "invalid position" through the resolvers' return types — sentinel-bearing signatures would thread through every consumer and make this hard to remove. Reviewed By: JakobDegen Differential Revision: D115579787 fbshipit-source-id: ec52e040c65e842e31fc91154823eb65c53f1a43

Move SQLite page-out writes to shard writers Summary: Today, the page-out workers will flush a sqlite shard if it fills. That stops that worker from finishing anything else that may be in it's queue. When we move the serialization to tokio futures, we do NOT want the tokio async workers pausing for long periods of time to flush sqlite, we want them to continue serializing into the next output buffer so that the sqlite threads can flush on their own later. -- Replace synchronous sqlite flushes from page-out workers with per-shard `page-sql-N` writer threads and bounded active/spare/pending write buffers. Default the sqlite shard count to 10 while preserving `BUCK2_PAGABLE_SQLITE_SHARDS`, and keep aggregate writer/backpressure counters for validation. Reviewed By: christolliday Differential Revision: D109892236 fbshipit-source-id: 589d64d1ba0cc00413f2b5c2f1bbfc18615fd16b

Use stable type names for pagable typetag tags Summary: Typetags are persisted with serialized data, but generic registrations built them with `std::any::type_name`, whose output has no stability guarantee across compiler releases. A toolchain upgrade could silently change every generic tag and invalidate previously written data. The pre-existing `#[pagable_tagged]` wrapper path had the same problem. Introduce `PagableStableName`, which composes a name from `module_path!()` and source identifiers — for generics, recursively through type arguments plus `Display`-formatted const arguments. The result changes only when a type is renamed or moved, never with a compiler upgrade. `#[derive(Pagable)]` / `#[derive(PagableSerialize)]` implement the trait automatically (skipped for types with lifetime parameters, since composed names are cached by `TypeId`). Generic monomorphizations cannot hold the composed name in a per-instantiation static, so `memoized_stable_name` interns it, leaking exactly one `String` per instantiation per process. Generic `#[pagable_typetag]` impls get the `PagableStableName` requirement added to their where clause by the macro; hand-written impls of `#[pagable_tagged]` wrappers need `Self: PagableStableName` spelled out alongside the existing `PagableRegisteredFor` bound. The starlark `TypeMatcher` combinators and `TypeMatcherAlloc` factory methods get the bound threaded through accordingly. Reviewed By: JakobDegen Differential Revision: D115599856 fbshipit-source-id: 8d63133f67290a4bff1f06a1b38334c4ab0aecd0

Emit generic pagable typetag registrations Summary: Enable `#[pagable_typetag]` on generic impl blocks, completing the stack. For a generic impl, each macro expansion emits (scoped inside `const _`): - `__pagable_do_register` — a generic helper that pushes a `TypetagRegistration` for the concrete `Self` (box and Arc-payload deserializers) into the trait's accumulator. - `__pagable_registration_anchor` — a monomorphized `extern "C"` fn whose body emits a pointer to the helper into the platform's program-constructor section via the emission macro from the previous diffs, so the loader registers the instantiation when the image containing it is loaded. - A manual `PagableTagged` impl. Generic impls can't go through the `PagableTypeTag` blanket impl (there is no per-instantiation static tag), so this impl provides the tag, body-serialize, and tagged-Arc-payload paths directly. The tag fn references the anchor through `core::hint::black_box`, which keeps the constructor record linked for exactly the monomorphizations whose serialize path is in the binary. Until `#[used(linker)]` stabilizes, a type that is never serialized in a given binary emits no record — same-binary round trips are always safe; deserialize-only readers of another binary's data are not. Registration runs at image load; the trait registry drains its accumulator when first built (lazily, on first `dyn Trait` deserialization) and merges the generic entries with the `inventory` ones. Unsupported targets get a `compile_error!` (supported: 64-bit Linux/macOS, x86_64 Windows), and non-path self types are rejected at expansion time. Round-trip tests cover type generics, const generics, mixed type+const generics, `Arc<dyn Trait>` fields through a generic impl, and name collisions across modules and across same-named traits. Reviewed By: JakobDegen Differential Revision: D113680820 fbshipit-source-id: a832c9abffaaef1ae70e6b7995f0bca85ce00932

Share ParametersSpec shape across def/lambda instantiations Summary: **Why:** Every evaluation of a `def`/`lambda` statement rebuilt a complete `ParametersSpec` in `InstrDef`: a cloned function-name `String`, an owned `String` per parameter, a freshly hashed `SymbolMap` (one heap `Symbol` per name), and `DefParamIndices` — all of which are a pure function of the compiled parameter list. Only the default *values* can differ between instantiations of the same site (def-time evaluation semantics). For code that creates closures in a loop — notably the prelude's record-of-lambdas "objects" like `Soname` (two lambdas per `SharedLibrary`) and the fbcode macros' per-attribute `selects.apply` lambdas — this cost ~250 B inline plus ~148 B and several heap allocations per parameter, per instance, and ~400ns/param of instantiation time. Measured impact (isolated 10-host A/B, 190 paired samples, `cquery deps(fbcode//buck2:buck2 + thrift/cpp2:server)`): wall −4.8%, buckd max RSS −3.0%, jemalloc waste −20%; lambda creation microbenchmarks reach CPython parity (from 4–13× slower). Full details in the test plan. **How:** - `ParametersSpec<V>` becomes `{ prototype: triomphe::Arc<ParametersSpecPrototype>, defaults: Box<[V]> }`. `ParametersSpecPrototype` — named after the C sense of "prototype": parameter names and kinds, no defaults — holds `function_name`, `param_names`, `param_kinds`, `names: SymbolMap`, and `indices`; everything value-independent. `triomphe::Arc` (no weak refs) keeps the shared header to 8 bytes; the prototype contains no starlark values, so it is plain `Pagable` and gets identity-preserving `Arc` dedup from the base pagable crate rather than the starlark bridge. - The packed `u32` `ParameterKind` with out-of-line dense `defaults` (D117857143) already made the kinds slice value-free; this diff moves it into the prototype and switches the `Defaulted` lookup from safe indexing to `get_unchecked` under the `from_prototype` length check below. - The prototype's `function_name` is the `ArcStr` introduced in D117857142, sharing one allocation with `DefCompiled::function_name` instead of two per-site `String` copies. It stays *inside* the prototype: carrying it per-`ParametersSpec` (measured) costs ~16–24 B per lambda instance and ~10% creation time for no benefit short of prototype interning, which is left as a possible follow-up. - The prototype is carried by `ParametersCompiled` as a private field derived in `ParametersCompiled::new`, so a params/prototype mismatch is unrepresentable: `new` builds the prototype from the same parameter list (the builder-driving loop moved there from `InstrDef`), and `map_exprs` — the only shape-preserving transform, used by `write_bc` — carries the `Arc` across. `InstrDef` now only pops the evaluated defaults, runs the default-type checks, and calls `ParametersSpec::from_prototype`. - Bounds safety for `Defaulted` index lookups is release-checked: the prototype records `num_defaults` (indices are dense `0..num_defaults` by builder construction) and `from_prototype` asserts `defaults.len()` against it — one predicted compare per instantiation. - `Arc` rather than a frozen-heap reference because native functions and `ParametersSpec<Arc<CoercedAttr>>` (buck2 `attr_spec`) build specs with no `FrozenHeap` in reach. - `FreezeBranded` now freezes only the defaults; the `as_value` transmute is unchanged (the prototype is `V`-independent). The public API (`new_parts`, `new_named_only`, `collect*`, `parser`, `documentation*`) is unchanged; `buck2_interpreter_for_build` / `buck2_build_api` compile untouched. Known tradeoff: for single-instantiation `def` sites (most module-level functions) the split adds one `Arc` allocation while sharing nothing — the A/B shows live jemalloc allocated +0.8%, more than offset by the −20% waste from eliminating the per-instantiation small-allocation churn. Prototype interning (prototypes are now name-light and highly repetitive across sites) is the natural follow-up to reclaim it. Reviewed By: NavidQar Differential Revision: D117708077 fbshipit-source-id: 2481464bfc1042f120289b42eb516c166b7d4302

Refresh daemon inactivity timeout on streamed requests Summary: Refresh the daemon inactivity timer when bidirectional server commands receive decoded client requests, instead of only when the command starts. This keeps long-lived stdin-driven commands like `buck2 lsp`, `buck2 subscribe`, and `buck2 dap` from being treated as idle while they are still actively serving requests. Add focused LSP and subscribe regression tests that verify repeated requests keep the same daemon alive across the testing inactivity timeout window. Reviewed By: JakobDegen Differential Revision: D102630751 fbshipit-source-id: e002d2beed56b205108725938568802b2a20fbf4

Fix StarlarkHashValue collisions on sequential names Summary: `StarlarkHasher` truncated `FxHasher64` output to its low 32 bits. The truncation predates the hasher: it was written for `DefaultHasher` (SipHash, well-mixed everywhere) and survived the switches to FNV and fxhash, both of which finish with a multiply that drives entropy into the high bits. Names differing only in bytes that land in the high half of the final 8-byte chunk therefore collide in the low 32 bits deterministically, and fxhash's weak rotate-xor-multiply chunk mixing also produces full 64-bit collisions when sequential digits span chunk boundaries. Replace the inner hasher with `Fx64Hasher`, a port of `rustc_hash::FxHasher` 2.x (Orson Peters' polynomial-hash rewrite, used by rustc; MIT OR Apache-2.0) with platform-independent output: the state is pinned to `u64` where upstream uses the pointer width, and byte mixing is pinned to the 128-bit widening multiply where upstream substitutes a different-valued mix on architectures without fast wide multiplication. `finish_small` folds the two output halves together instead of truncating. **Why a vendored port instead of the `rustc-hash` dependency:** `StarlarkHashValue` requires two properties. First, identical output on every platform (endianness, pointer width, architecture) -- upstream varies all three by design. Second, output that never changes except deliberately: these hashes are persisted by pagable serialization (`VecMap`/`SmallMap`/`ImmutableMap` store each entry's hash), so a hash change is a data-format change. Upstream explicitly documents hash stability across versions as a non-goal -- that is their license to keep improving the algorithm, and it is incompatible with depending on them for a persisted hash. Pinning an exact crate version is worse than the ~100 vendored lines: fbsource has one global `rustc-hash` version any team may bump, and an exact pin in the OSS `starlark_map` crate would conflict with downstream resolvers. The port is guarded by tests in both directions: - `fx64::tests::matches_rustc_hash` cross-checks bit-for-bit equality against the real crate (now a test-only dependency) on x86-64/arm64, where upstream takes the identical code path. If upstream changes their algorithm, this fails with instructions: do not silently update the port; adopting the new algorithm is a `StarlarkHashValue` format change requiring golden-value updates and versioning/invalidation of persisted pagable data. - `fx64::tests::stable_across_platforms` and `hasher::tests::starlark_hash_value_is_stable` lock golden hash values (u64 and folded u32) on every platform. - `small_map::tests::pagable_bytes_embed_stable_hashes` locks the exact pagable byte encoding of a map, which embeds per-entry hashes, tying hash stability to the persisted format explicitly. Duplicate 32-bit hashes among 20000 distinct keys per naming pattern, before vs after: | pattern | fx64 low32 (before) | rustc-hash 2 fold32 (after) | | ---- | ---- | ---- | | `filtered_res_{i}` (prelude Android, one output per resource) | 11354 | 0 | | `rule_{i}` | 11256 | 0 | | `lib{i}` | 9970 | 0 | | `assets/secondary-program-dex-jars/secondary-{i}.dex.jar` (prelude dex outputs) | 9235 | 0 | | `fbcode//some/package/path:generated_rule_name_{i}` | 4832 | 0 | | `debug_info_file_{i}` (prelude dist-ThinLTO dict keys) | 24 | 0 | | random hex | 0 | 0 | These are not hypothetical name shapes: the buck2 prelude generates exactly such counter-suffixed families at scale during analysis of large builds -- `filtered_res_{i}` in `prelude/android/android_binary_resources_rules.bzl` (one per Android resource dep, thousands on large apps), `secondary-{i}.dex.jar` and `batch_{i}` families in `prelude/android/dex_rules.bzl` (hundreds of secondary dexes), `debug_info_file_{i}` dict keys in `prelude/cxx/dist_lto/darwin/dist_lto.bzl` (one per object file in a distributed ThinLTO link, 10k+ for large binaries), and `{name}-types_split{i}` sibling targets in thrift Rust codegen. Under the old hasher the Android output-name families ran at 46-57% duplicate hashes, degrading every `SmallMap` keyed by them to long equal-hash probe chains. Hand-written target names are only mildly affected (`fbcode//unicorn/if:`, 9802 targets: 1 collision before, 0 after; `fbcode//glow:`, 123023 labels: 18 before, 1 after with 1.76 expected by birthday bound). Hashing itself gets faster through the `Hash for str` path (7.2ns vs 9.2ns per 48-byte key, 3.0ns vs 4.4ns per 7-byte key), and end-to-end buck2 workloads without such generated families are unaffected: interleaved cold-eval A/B runs of `targets fbcode//unicorn/if:` (n=20 per side) measured +0.12% trimmed mean, and a 3-workload suite (`targets` on unicorn/if and glow, `cquery deps(buck2-bin)`, n=6 each) measured -0.9% to +0.6% medians, all within devserver noise with byte-identical outputs. The port reads bytes little-endian, retiring the old endian-dependence TODO, and is strictly more portable than both the old `FxHasher64` (endian-dependent) and the upstream crate (width- and architecture-dependent). Anything persisting `StarlarkHashValue` (e.g. pagable data) must not be reused across binaries built before and after this change. Reviewed By: JakobDegen Differential Revision: D115452993 fbshipit-source-id: 7233984f2c8d2baac371621f3b20ef2d12630a11