Jakob Degen

mem: Add mini_vec crate and type Summary: `MiniVec` is a `Vec` but is a single pointer on the stack; the length and capacity are bitpacked into the *top* bits of the pointer if they're small, otherwise allocated in the heap. This does introduce high-bit pointer packing to buck2 which we have not had so far. Claude assures me that modern kernels do not hand out high addresses without a user explicitly asking for them and no one complained when I asked, so I guess the team is on board with this. I'll use this in a couple places, but the first thing we'll be able to do is replace all the `ThinBoxSlice` Reviewed By: jtbraun Differential Revision: D105465807 fbshipit-source-id: 65663fb277c1301be55a3b1e94ef831ae80d4f78

mem: Change how `DiceTransaction` -> `DiceComputation` works Summary: We want to have `&'c mut DiceComputations<'d>` return `&'d Value`s. Right now, it can't do that because `DiceTransaction` derefs to `DiceComputations<'static>` and obviously we can't get back `'static` things. We instead need a conversion like `&'d DiceTransaction -> DiceComputations<'d>` and this diff does that. We remove the deref impl (since the signature is no longer right), add `.ctx() -> DiceComputations<'d>` instead, and then go and fix all the compiler errors. As a part of that, we can actually change the representation of `BaseComputeCtx` to just directly hold a `CoreCtx`, which makes a bit more sense. I'll do some renames towards the end of this stack to help justify why things are the way they are (note also that the use of `ModernComputeCtx::Parallel` is fine here and actually has nothing to do with `Parallel` specifically) Reviewed By: NavidQar Differential Revision: D110626399 fbshipit-source-id: 4ba0db607a88e3456ccb7b091b6c2bf5402566c7

Replace Dice task internals Summary: This mostly rewrites the management of the per-key dice task state that manages cancellation, dependents, spawning the worker, etc. Chris initially did this because it unblocked an approach to paging that he was considering (described in the next diff). My understanding is that they have since picked a different direction, so this needs new justification. I do think that despite that change, the next diff is a well-motivated simplification on its own. It fixes a small memory issue I encountered elsewhere, but also having just the single map does seem quite a bit easier to reason about. As to whether this diff would be well-motivated on its own even without the next one is a bit less obvious (claude and I talked and interestingly we disagree somewhat). The new data structure is subtler and more complicated than any one thing that existed before, but in exchange for that there's only one thing; previously there were three differently synchronized things (an atomic and two locks) managing information equivalent to what is now the `DiceTaskInternal` contents. I tend to think that's an improvement. The current version has some sketchy things going on, but at least they're all in one place and I think a person has better odds of reasoning correctly about it (claude tells me the previous version of this was actually probably bug-free but idrk if I believe it). In other words, I like this because I think one explicitly complicated component is better than three implicitly complicated ones. There's one somewhat significant wart here in the form of the `AtomicList` type, which is simple but pretty poorly optimized. I may try to revisit that. ...and then after writing all that explanation I ran benchmarks on the next diff and saw it was -2.5% memory. So now this is all easily justified Reviewed By: jtbraun Differential Revision: D103355606 fbshipit-source-id: f8616468c3e8f94ed2c3a8fe8e5d0276a56b7748

mem: Fix up the linear recompute APIs for lifetime changes Summary: I apologize, I screwed this stack up and wrote it all out of order, it was really hard to keep this all in my head at the same time. I did end up somewhere good at the end, but at this point it's a mess. This diff fixes up the linear recompute APIs to something that we can better support under the new lifetime model and fixes a minor bug around a bad assert in the previous model. I don't really know if it's worth trying to explain this too hard, the way to review this is I guess to look at the changes to the dice APIs, decide that they're reasonable, notice that the `.into_inner().unwrap()` is gone, and trust me that it's useful for the rest of the changes? Reviewed By: jtbraun Differential Revision: D111267628 fbshipit-source-id: 5b5470a0614e8c4b976ae7793707596354a0521c

mem: Migrate most parallel futures to async closures Summary: Complete most of the migration from the previous diff. As a part of this we rename the old APIs to `_boxed`, rename the new APIs to unprefixed, and then migrate nearly everything to use the new APIs. All the things that are migrated here were completely trivial and could have been done by sed, which speaks to the ergonomics wins. There's a handful of remaining holdouts that are not migrated yet because they looked like they'd need a tiny bit more attention, doing those next. Reviewed By: jtbraun Differential Revision: D111267639 fbshipit-source-id: e3347b4ca3e537f431c54298f523e809acf217dc

mem: Implement improved `join_all` and `try_join_all` futures Summary: We currently use `futures` for this which is extremely inefficient for little good reason. Reimplement it better. Details are in the code as always, but I'll say that I actually am pretty happy with where this landed in terms of complexity to benefit ratio. The one little index table trick is basically the only clever thing going on here, and in exchange we have memory use that's like `80 + N * (max(future size, return size) + 1)` (compared to `futures` which allocates like 80 bytes of bookkeeping for each future). I'll send some follow ups later to use this in other parts of buck2 as well, but will stick to the current stuff in dice for a bit longer first. Reviewed By: jtbraun Differential Revision: D111267627 fbshipit-source-id: bb3891e360b50afa622a482c74b4c5c1600900ec

lifetimes: Complex branded `ProviderCollection` Summary: Convert `ProviderCollectionGen<V>` to the branded `ProviderCollection<'v>`. This is half of the old "provider dep" WIP; the `Freeze`-interop and `OwnedFrozen` diffs below are what let it split off from the `Dependency` conversion. Notes: - The complex-value pieces are hand-written rather than `starlark_complex_value_branded!` because empty collections allocate as the statically interned empty collection. - `FrozenProviderCollectionValue` now stores `OwnedFrozen`; `FrozenProviderCollectionValueRef<'f>` uses `'f` as the heap brand. Its API is mostly unchanged, with a new `builtin_provider_value` helper replacing the `value.maybe_map(...)` dance at call sites (which also fixes a latent double-move hazard: `maybe_map` takes `self`). - The typed accessors (`default_info` etc.) moved from `impl FrozenProviderCollection` to `impl<'v> ProviderCollection<'v>` and panic on unfrozen collections; every existing caller reaches them through frozen values. - `Dependency` is not yet converted; it gets a transitional `collection()` rebrand helper that the next diff deletes. Reviewed By: jtbraun Differential Revision: D115108981 fbshipit-source-id: 13d91672abdce3fb86e76b174f74a909d52a5244

lifetimes: Complex branded `TransitiveSet` Summary: This does the remaining transitive set types. The noteworthy thing is that there's lots of code which used to use `OwnedFrozenValueTyped<FrozenTransitiveSet>` and friends, and most of that is now being upated to use `OwnedFrozen` instead. This does come with some nominal usability hits, primarily around the need for `by_ref` to access the contents of an `OwnedFrozen`; that's fundamental though, the existing API is unsound Reviewed By: jtbraun Differential Revision: D115108998 fbshipit-source-id: f8bf9b74f05c131832c43cd80eb3e2a1bb7f520e

mem: Analysis and deferred results by reference Summary: The remaining calculations: `get_analysis_result` returns `ResultMaybeCompatible<&'d AnalysisResult>` (same shape as the configured node diff), bxl and dynamic lambda results are handed out by reference through their late bindings, and `EVAL_ANON_TARGET` stops forcing a dupe that its impl never needed. The payoff is `DeferredHolder`, which is looked up in every `BuildKey` compute: it's now a `Copy` enum of `&'d` refs. The detailed-metrics tracker keeps holders in a daemon-lifetime map, so an `OwnedDeferredHolder` twin dupes the inner `Arc`s at that boundary. That's also why `DynamicLambdaResult` keeps its `Arc` - `RecordedAnalysisValues` isn't shareable, so the metrics path genuinely needs the handle. Un-Arced along the way: `EnabledOptionalValidationsKey` (the `Either<Cow>` dance dies), `TestExecutionKey`, `TemplatePlaceholderInfoQueryKey` (resolves the TODO complaining about that clone), and the `Arc<ArtifactFs>` wrappers in aquery now that `ArtifactFs` is dupe. The `BxlComputeResult` newtype was pure dupe boilerplate and is gone. Reviewed By: jtbraun Differential Revision: D114339462 fbshipit-source-id: 471db89f830e08c6043cf1f5780d88309b4cc71e

lifetimes: Delete the legacy value APIs Summary: This is the phase-4 payoff of the lifetimes/branding project: everything the branded world replaced is now deleted. Every consumer in fbsource - buck2 and the seven external components - was migrated to the replacements earlier in this stack, so this diff deletes rather than deprecates. What dies, and what replaced it: - `OwnedFrozenValue` and `OwnedFrozenValueTyped`, wholesale (the entire `values/owned.rs` module). `OwnedFrozen` / `OwnedFrozenRef` replace them, with brand-scoped access (`by_ref`, `add_to_heap`, `add_to_frozen_heap`) in place of the `value`/`owned_value`/`unchecked_frozen_value` family. - `Heap::{access_owned_frozen_value, access_owned_frozen_value_typed}` - `OwnedFrozen::add_to_heap` does the same thing from the other side. - `FrozenModule::{get, get_option, get_any_visibility, owned_extra_value}` - the `_owned` family (and `get_option_ref`) replaces them. - The Gen macros `starlark_complex_value!` and `starlark_complex_values!`, and the legacy alloc path they fed: `AValueComplex`, `Heap::alloc_complex`, and the `ComplexValue` trait alias. `starlark_complex_value_branded!` + `alloc_complex_branded` replace them. `alloc_complex_no_freeze` stays - it has no frozen form, so branding is orthogonal to it. - `ValueTypedComplex` - `ValueTyped<'v, X<'v>>` subsumes it for branded types, since a branded downcast matches frozen and unfrozen instances alike. - Residue the above exposed: `Value::cast_lifetime` (only `ValueTypedComplex` used it) and the `Either`-returning `from_value` that the plural Gen macro emitted. The one piece of the legacy family that had to be reimplemented rather than deleted is the `_owned` module lookups, which were built on top of the legacy ones (`get_option` then `.into()`). They now go straight from `get_slot_any_visibility` to a single private `FrozenModule::own_value` helper that carries the one SAFETY argument - the module's heap keeps its own slot values alive, directly or through its heap references for values that arrived via `load()` - which is the same argument `get_option_ref` already documents. `load_symbol` likewise adds the reference itself instead of routing through an owned value. The pagable impls on `OwnedFrozen` were also partly built on the legacy ones. Their wire format is unchanged and must stay that way: it is now written once, in `serialize_owned_frozen` / `deserialize_owned_frozen`, in exactly the byte order the deleted code used (owner heap ref, then frozen value), shared by the `Value` and `ValueTyped` forms so the two remain interchangeable at a field. Net effect on `unsafe`: six `pub unsafe fn`s and one `unsafe impl` leave the public API (`OwnedFrozenValue::{new, unchecked_frozen_value, owned_frozen_value}`, `OwnedFrozenValueTyped::{new, to_frozen_value, value_typed}`, `Trace for ValueTypedComplex`), plus `Value::cast_lifetime` internally; 68 lines mentioning `unsafe` go away against 30 added, and the added ones are all `OwnedFrozen::unchecked_new` standing in for `OwnedFrozenValue::new` under the identical contract. Several test sites lost their `unsafe` outright by moving to `OwnedFrozen::build` or `add_to_frozen_heap`. Documentation is brought to the state it would have if written fresh: the `ComplexValue` doc essay on the Gen pattern is gone (`StarlarkValue` now just points at `FreezeBranded` and the branded macro), the `branding` module's worked example is `add_to_heap`, and `docs/heaps.md` describes `OwnedFrozen`. The OSS changelog is deliberately untouched - it is written at release time, and recording history is its job. Reviewed By: jtbraun Differential Revision: D115801878 fbshipit-source-id: 1669596a01efbe1b0daad6755e46eeb935be6734