docs(xet): memory-derived download buffer defaults - #2731
Draft
rajatarya wants to merge 2 commits into
Draft
Conversation
The three HF_XET_RECONSTRUCTION_DOWNLOAD_BUFFER_* defaults are derived from usable memory (min of host RAM and container cgroup limit) as of huggingface/xet-core#943, replacing the fixed 2gb/512mb/8gb constants. High performance mode is memory-aware too and no longer overrides explicitly set env vars. Adds the HF_XET_MEMORY_DERIVED_DOWNLOAD_BUFFERS kill switch row. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGuU1K7rpjFfBGrA3SvCxL
|
The docs for this PR live here. All of your documentation changes will be reflected on that endpoint. The docs are available until 30 days after the last update. |
HF_XET_DISABLE_MEMORY_DERIVED_DOWNLOAD_BUFFERS=1 (disable flag) replaces the enable-flag form, and the derivation floors are now 64mb/16mb/264mb. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MGuU1K7rpjFfBGrA3SvCxL
rajatarya
added a commit
to huggingface/xet-core
that referenced
this pull request
Aug 28, 2026
…ry (#943) # Memory-derived download buffer defaults Fixes #927. Related: huggingface_hub#3300. ## Problem The three `HF_XET_RECONSTRUCTION_DOWNLOAD_BUFFER_*` defaults are compile-time constants (2 GB / 512 MB / 8 GB). Issue #927 measured the cost on both tails: a 20 Gbit/s, 2 TB host leaves **2.7x throughput** on the table (6709 → 18114 Mbit/s from raising only these three knobs), while a 1 Gi Kubernetes container gets **OOM-killed** (huggingface_hub#3300). A default that is simultaneously too small for a 2 TB host and too large for a 1 Gi container should be a function of available memory, not a constant. ## Design overview Three cooperating changes, all inside `xet_runtime`: 1. **A memory probe** (`utils/system_memory.rs`, new): usable memory = min(host total, effective cgroup limit), cgroup-aware in the way the issue prescribes — resolving the path from `/proc/self/cgroup` rather than reading the controller root, via sysinfo 0.39's `Process::cgroup_limits()`. This requires **sysinfo 0.38.4 → 0.39.6** and a **Rust toolchain bump 1.94.1 → 1.95.0** (sysinfo 0.39's MSRV), both part of this PR. 2. **Derived defaults**: each buffer knob becomes `clamp(fraction × usable, floor, ceiling)`, anchored so a 32 GB machine reproduces today's defaults and a ≥256 GB machine reproduces today's high-performance preset. Explicit env vars override, exactly as before. 3. **Coherence fixes** in the same machinery: setting `SIZE > LIMIT` no longer panics (normalized with a warning); the HP preset becomes memory-aware and no longer silently stomps explicit env vars. ### The memory probe ```mermaid flowchart TD A(["USABLE_MEMORY probe"]) --> B{"LazyLock cached?"} B -->|yes| Z(["return cached UsableMemory"]) B -->|no| C["host_total via sysinfo<br/>RefreshKind memory only"] C --> D["refresh own PID entry then<br/>Process cgroup_limits<br/>from sysinfo 0.39"] D --> E{"limits returned?<br/>Linux only"} E -->|"no: macOS via Mach sysctl<br/>Windows via GlobalMemoryStatusEx<br/>wasm stub - no info"| N["cgroup_limit = None"] E -->|yes| F["sysinfo resolves the path in<br/>/proc/self/cgroup and takes the<br/>min limit across ancestor cgroups<br/>v2 memory.max or v1 limit_in_bytes"] F --> G{"limit below host total?"} G -->|"no - unlimited"| N G -->|yes| H["cgroup_limit = limit"] N --> L H --> L["usable = min of host_total<br/>and cgroup_limit"] L --> M["cache UsableMemory struct<br/>host_total / cgroup_limit / usable"] M --> Z ``` Why sysinfo 0.39: the previously pinned sysinfo 0.38.4 only offered `System::cgroup_limits()`, which reads controller-root paths (`/sys/fs/cgroup/memory.max`) — fine under Docker/k8s cgroup namespaces but blind to nested cgroups (systemd slices, Slurm steps), the exact failure modes the issue reporter hit downstream. sysinfo 0.39.0 added `Process::cgroup_limits()`, which resolves the process's own cgroup path from `/proc/<pid>/cgroup` and takes the tightest bound across ancestor cgroups, for both cgroup v1 and v2, treating `max`/PAGE_COUNTER_MAX as unbounded. Depending on the maintained implementation beats maintaining a hand-rolled `/proc` parser here; sysinfo carries its own fixture tests for the walk. sysinfo still supplies the host total; the probe adds no new dependency. The probe is `LazyLock`-cached (the `config_group!` macro evaluates default expressions on every `XetConfig::new()`, which runs per session, in the legacy runtime, twice in git_xet, and in xtool). `Process::cgroup_limits` returns `None` off-Linux by contract, so the probe needs no OS-specific gating of its own; wasm gets a stub returning no memory info, which falls back to today's static defaults. When no cgroup limit binds, the reported limit equals the host total and is filtered out, keeping `cgroup_limit = None` meaning "no limit set" in logs. ### Platform behavior The probe has two independent inputs — host total and container limit — and each platform fills in what it can. `usable = min(present values)`; if neither is available, derivation is skipped entirely and today's static defaults apply (fail-safe, never worse than current behavior). | Platform | Host total | Container/limit awareness | Net effect | |---|---|---|---| | Linux (bare, k8s, Docker, WSL2 distro) | sysinfo (`/proc/meminfo`) | Full via `Process::cgroup_limits` (sysinfo 0.39): cgroup v2 + v1, nested paths, namespaced roots | Both tails fixed — the issue's target | | macOS | sysinfo (Mach kernel API / `sysctl hw.memsize`) — physical RAM | None (no cgroup concept; Docker Desktop runs a Linux VM, inside which the Linux path applies) | Laptops scale by RAM: 16 GB → 1 GB/248 MB/4 GB; 128 GB Mac Studio → 8 GB/2 GB/32 GB | | Windows | sysinfo (`GlobalMemoryStatusEx`) — physical RAM | **Not probed**: Job Object / Windows-container memory limits are not read (sysinfo has no API for it; rare deployment; documented limitation — recourse is the env vars or kill switch). WSL2 goes through the Linux path. | Same RAM-proportional scaling as macOS | | wasm | stub → `None` | `None` | Static defaults (2 GB/512 MB/8 GB), exactly today | | Any platform, sysinfo returns 0/fails | `None` | — | Static defaults | The derivation formula itself is platform-independent and unit-tested everywhere. cgroup resolution is delegated to sysinfo, which is tested upstream (including nested-ancestor fixtures); this repo carries no OS-specific probe code beyond the wasm stub. ### The derivation For standard mode: `limit = clamp(u/4, 264 MB, 64 GB)`, `size = clamp(u/16, 64 MB, 16 GB)`, `perfile = clamp(u/64, 16 MB, 2 GB)`, each then rounded down to a multiple of 8 MB (decimal units throughout, matching `ByteSize`). The floors are the 1 GiB derivation values; the ceilings are the historical HP constants. High-performance mode uses 2x-aggressive fractions with the same floors/ceilings: `u/2`, `u/8`, `u/32`. | usable memory | size (u/16) | perfile (u/64) | limit (u/4) | note | |---|---|---|---|---| | ≤ 1 GiB | 64 MB | 16 MB | 264 MB | floors; **the #3300 container: was 2 GB base / 6.1 GB target → OOM** | | 8 GB | 496 MB | 120 MB | 2 GB | small VM / CI runner | | 16 GB | 1 GB | 248 MB | 4 GB | typical laptop | | **32 GB** | **2 GB** | **496 MB** | **8 GB** | **≈ today's defaults (512 MB → 496 MB from rounding)** | | 64 GB | 4 GB | 1 GB | 16 GB | workstation | | 128 GB | 8 GB | 2 GB (ceil) | 32 GB | | | ≥ 256 GB | 16 GB (ceil) | 2 GB (ceil) | 64 GB (ceil) | **= today's HP preset = the issue's 2.70x arm** | Properties, verified analytically at every region boundary for both fraction sets: - **Coherence invariant**: `size + max_concurrent_file_downloads(8) × perfile ≤ limit` holds everywhere (mid-range: 3u/16 ≤ u/4; floors: 64+128=192 ≤ 264 MB; ceilings: 16+16=32 ≤ 64 GB; HP: 3u/8 ≤ u/2). The three numbers describe one allocation, as the issue requested. - **Monotonic** in usable memory; anchored to reproduce current behavior at 32 GB and current HP at ≥256 GB (the issue's measured 2.70x configuration). - The 32 GB anchor is exact for size and limit; perfile lands at 496 MB vs today's 512 MB (u/64 kept clean rather than u/62.5). Floors note: per review, the floors are the 1 GiB derivation values — environments below ~1 GiB get the same triple rather than a smaller dedicated tier. At the floors a single maximum-size term (one unpacked xorb block, ≤ 64 MiB) fits the budget once one file download is active (64 + 16 = 80 MB); as a backstop, `acquire_many` clamps an oversized single term to total permits, so it proceeds serially rather than deadlocking. ### Startup flow — how system info reaches the consumers ```mermaid sequenceDiagram autonumber participant PY as Python hf_xet participant XS as PyXetSession participant CFG as XetConfig participant MEM as system_memory probe participant CTX as XetContext participant COM as XetCommon participant SEM as AdjustableSemaphore participant FR as FileReconstructor PY->>XS: XetSession constructor XS->>CFG: XetConfig::new CFG->>CFG: Default::default builds groups CFG->>MEM: USABLE_MEMORY (first read) Note over MEM: cgroup walk then sysinfo host total<br/>cached in LazyLock for process lifetime MEM-->>CFG: usable bytes Note over CFG: derived defaults<br/>size = clamp of usable/16<br/>perfile = clamp of usable/64<br/>limit = clamp of usable/4 CFG->>CFG: with_high_performance if HP flag<br/>memory-aware HP fractions CFG->>CFG: with_env_overrides<br/>explicit env vars always win CFG-->>XS: XetConfig XS->>CTX: XetContext::with_config CTX->>CTX: normalize reconstruction group<br/>ensure limit >= size then freeze as Arc CTX->>COM: XetCommon::new COM->>SEM: new with initial size<br/>floor size and ceiling limit Note over SEM: one permit equals one byte<br/>caps in-flight unwritten bytes XS-->>PY: session ready PY->>FR: download file (later) FR->>SEM: grow target to min of<br/>size plus n_active times perfile<br/>and limit SEM-->>FR: seed permit for growth FR->>SEM: acquire_many term_size per term Note over FR,SEM: permits released only after<br/>bytes are written to disk ``` The same `XetConfig::new()` → `XetContext::new()` funnel serves every entry point (Python `XetSession`, the legacy `download_files` runtime still used by huggingface_hub, git_xet's LFS agent, xtool), so all of them inherit derived defaults with no per-caller changes. `XetContext::new` is the single choke point where the normalized config is frozen into an `Arc` — guaranteeing the semaphore bounds (built once in `XetCommon::new`) and the per-download #666 target formula (recomputed in `FileReconstructor`) always read identical values. ### Ordering and preset fixes `XetConfig::new()` becomes **defaults → HP preset → env overrides** (today env runs before HP, so `HF_XET_HP` silently discards explicit `HF_XET_RECONSTRUCTION_*` settings — the issue's benchmark matrix would literally have been impossible to run as intended with both set). Enabling this requires `apply_env_overrides` to fall back to the *current* field value rather than re-evaluating the macro default — a one-line macro change (`let default_value = self.$name.clone()`), audited safe: all existing callers outside `new()` are tests invoking it immediately after construction, and it halves default-expression evaluations as a bonus. `with_high_performance()` keeps its static fetch-size and concurrency literals (metadata-scale, not RAM-scale) but takes its three buffer values from the HP derivation — so `HF_XET_HP` on an 8 GB laptop now yields 1 GB / 248 MB / 4 GB instead of a physically impossible 16 GB base buffer. **Panic fix**: `AdjustableSemaphore::new` release-asserts `min ≤ max`, so `HF_XET_RECONSTRUCTION_DOWNLOAD_BUFFER_SIZE=16gb` alone crashes today (default limit 8 GB). Normalization (`if limit < size { warn!; limit = size }`) runs once in `XetContext::new`; `XetCommon::new` additionally applies `limit.max(base)` locally since it is `pub` and must never panic. Derived values can never trigger it by construction. ### Small containers and large machines - **1 Gi container** (the #3300 report): derived 64 MB / 16 MB / 264 MB. The semaphore caps in-flight-not-yet-written bytes, and the permit is released only after bytes hit disk, so buffer RSS stays ≤264 MB vs ~6.1 GB target today. Remaining known slack outside the semaphore: up to one decompressed xorb block (≤64 MiB) per active connection may be resident while only partially permitted — pre-existing behavior, unchanged, bounded by download concurrency; documented in the api_changes note. - **2 TB / 20 Gbit/s host** (the #927 measurement): derived values equal the HP preset = the issue's isolated 2.70x arm (18114 Mbit/s, 14.2 G peak RSS on a 2 TB machine). - **Prefetch/fetch knobs** (`min_prefetch_buffer`, `min/max_reconstruction_fetch_size`) are deliberately **not** derived: traced to be metadata-scale — they size reconstruction-term prefetch (term descriptions, URLs), not data buffers; actual data bytes acquire a buffer permit before the download task spawns. The issue's own benchmark confirms: those knobs moved throughput ≤2%, within drift. ### Escape hatches and observability - Every `HF_XET_RECONSTRUCTION_DOWNLOAD_BUFFER_*` env var still overrides its derived default (macro semantics, unchanged). - New kill switch `HF_XET_DISABLE_MEMORY_DERIVED_DOWNLOAD_BUFFERS=1` (bool, unset by default) restores the static 2 GB / 512 MB / 8 GB (and static HP) values — read per-call (cheap) so it is testable with `EnvVarGuard`; only the probe and derived numbers are cached. - One `info!` line emitted by `logging::init` after the subscriber is installed: host total, cgroup limit, usable, and the three derived values. (It cannot be logged where the values are computed — the LazyLock is first forced while building the logging config itself, before any subscriber exists, so an event there would be silently dropped and never fire again.) Values are also visible in Python via `XetConfig.__repr__` and per-field `Config: name = value (default)` log lines. ### Risks - Defaults become machine-dependent: nothing in xet-core asserts the literals (verified repo-wide), but downstream (huggingface_hub CI) is flagged in the PR/api_changes note. - Larger default in-flight budget on big hosts (up to the 64 GB ceiling ≥256 GB RAM): ceilings equal the existing opt-in HP values; kill switch available. - `with_env_overrides` no longer resets env-absent fields to macro defaults — documented; audited zero behavioral change for existing callers. - Users who set both `HF_XET_HP` and explicit buffer env vars previously got HP values; now env wins (the intended fix, but a behavior change). --- ## Testing - TDD throughout: derivation-table tests (11 memory sizes x standard/HP fraction sets), a multiplicative 256MB-2.5TB sweep asserting the coherence invariant, 8MB rounding, clamps, and monotonicity; integration tests for env-beats-derived, env-beats-HP, kill switch, and the panic fix. cgroup-walk correctness is covered by sysinfo's own upstream fixture tests (nested ancestors, v1/v2, unlimited sentinels). - On Rust 1.95.0 with sysinfo 0.39.6: `cargo test -p xet-runtime` (232 passed) and `cargo test -p xet-data` (340 passed), zero failures; `cargo clippy` clean on both libs; `hf_xet` and both wasm sub-workspaces (`hf_xet_thin_wasm`, `hf_xet_wasm` on wasm32) compile, exercising the wasm stub path. - Toolchain: all CI workflow pins moved 1.94.1 → 1.95.0 (sysinfo 0.39 MSRV); the wasm sub-workspace stays on its pinned nightly. - Per review: sysinfo is a `cfg(not(target_family = "wasm"))` target dependency of `xet_runtime`, so wasm builds no longer compile it at all (`cargo tree --target wasm32-unknown-unknown` shows zero sysinfo entries; all three consumers — `system_monitor`, `logging::init`, the memory probe — were already wasm-gated). Side benefit: wasm consumers are not subject to sysinfo's 1.95 MSRV. ## Note for downstream `huggingface_hub` CI or downstream tests asserting the literal 2GB/512MB/8GB defaults will see machine-derived values; `HF_XET_DISABLE_MEMORY_DERIVED_DOWNLOAD_BUFFERS=1` restores the static defaults. User-facing documentation for the changed env vars is updated in huggingface/hub-docs#2731 (draft until this ships in an `hf_xet` release). This implementation was AI-assisted (design and code reviewed and directed by @rajatarya). 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01MGuU1K7rpjFfBGrA3SvCxL <!-- CURSOR_SUMMARY --> --- > [!NOTE] > **Medium Risk** > Machine-dependent defaults may surprise downstream tests; config ordering and HP/env interaction changed. Memory sizing affects OOM risk in small containers and in-flight I/O on large hosts, though env vars and a kill switch remain. > > **Overview** > Reconstruction download buffer defaults (`HF_XET_RECONSTRUCTION_DOWNLOAD_BUFFER_*`) are no longer fixed 2 GB / 512 MB / 8 GB. They are computed from **usable memory** (min of host RAM and effective cgroup limit via **sysinfo 0.39** `Process::cgroup_limits`), with floors/ceilings so ~32 GB hosts match old defaults and large hosts match the old HP preset. **`HF_XET_DISABLE_MEMORY_DERIVED_DOWNLOAD_BUFFERS=1`** restores the static values. > > **Config behavior changes:** `XetConfig::new()` applies **defaults → high performance → env overrides** so explicit env vars beat `HF_XET_HP`; `with_env_overrides` keeps preset-adjusted fields when an env var is absent. **`normalize()`** raises `download_buffer_limit` to at least `download_buffer_size` (in `XetConfig::new` and `XetContext::new`); **`XetCommon`** avoids panics on incoherent configs. > > **Infra:** Rust **1.94.1 → 1.95.0** in CI/release workflows; **`sysinfo`** is a non-wasm-only dependency of `xet_runtime`. Lockfiles pick up transitive updates (e.g. `chacha20`, objc2 stack for sysinfo). > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 242f99a. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup> <!-- /CURSOR_SUMMARY --> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Documents the download-buffer changes landing in huggingface/xet-core#943:
HF_XET_RECONSTRUCTION_DOWNLOAD_BUFFER_*defaults are now derived from the memory usable by the process (the smaller of host RAM and the container cgroup limit), as a fraction with fixed floor/ceiling bounds, instead of the fixed2gb/512mb/8gbconstants. A 32 GB machine keeps effectively the previous defaults; small containers and large hosts scale accordingly.HF_XET_HIGH_PERFORMANCEbuffer values are memory-derived too, so the "at least 64 GB of RAM" caveat is replaced: the flag is now safe on smaller machines. Explicitly set env vars now take precedence over the flag.HF_XET_MEMORY_DERIVED_DOWNLOAD_BUFFERS=0restores the fixed values.Draft until xet-core#943 merges and ships in an
hf_xetrelease — the table should go live with the release that contains it.🤖 Generated with Claude Code
https://claude.ai/code/session_01MGuU1K7rpjFfBGrA3SvCxL