From 653eedbaa65c69d93077d6d69a560d8fcce34578 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Mon, 24 Aug 2026 13:49:47 -0500 Subject: [PATCH 1/6] Pre-warm the allocator by the shortfall, not the whole transfer .resident_prewarm() asked for the full onload need on EVERY activation. After the first render that doubles the caching allocator's pool, and under release = FALSE it keeps doubling until activation is refused. Measured on SDXL, three activate/render/deactivate cycles in one process: release = FALSE 5.299 -> 10.322 GiB -> REFUSED release = TRUE 5.299 -> 10.123 -> 10.125 (plateaus) The step from cycle 1 to cycle 2 was 5.023 GiB against a pre-warm block of need * 1.05 = 5.021 GiB, which is the whole mechanism: a render fragments the cache into its activation blocks, so the next single large request cannot be served from what is left and takes a fresh cudaMalloc beside the old one. release = TRUE masked it, because .resident_release_vram()'s empty_cache caps the pool at two blocks; release = FALSE never empties, so the third activation found the card full of retained pre-warm blocks. This matters most to the caller it was written for. A residency broker passes release = FALSE deliberately -- an exclusive device grant means blocks never returned cannot be taken by anything else between requests -- so an SDXL entry would serve two requests and refuse every one after. The doubling also broke the budget independently of the crash: both arms double from cycle 1 to cycle 2, so a peak measured on a single cycle understates steady state about 2x. Declaring 5.398 for something that needs 10.125 on its second request is admit-then-OOM with the first request looking perfect. The fix reads what the allocator already holds and only grows what is missing: skip entirely when the pool covers the transfer, otherwise request the shortfall. On a cold pool `held` is 0, so it is byte-identical to the previous behaviour and the 74x cold-start win is untouched. After, same probe: release = FALSE 5.396 -> 5.498 -> 5.498 release = TRUE 5.396 -> 5.396 -> 5.396 Flat from cycle 2 in both arms rather than merely surviving cycle 3. The residual 0.102 GiB in the FALSE arm is the render's own fragmentation and does not compound. Worth recording how it was found, because an obvious test would have missed it: activate/deactivate cycles with NO render in between hold dead flat at 5.098 GiB in both arms. The pre-warm block is reused cleanly across bare transitions. It takes a generation in between to fragment the pool, so one cycle is not a test, and neither is a cycle without a render. `held` is injectable for the same reason .resident_check_fits() takes free_gb: without CUDA the measurement is 0, which would always warm, so a test that wants the skip has to state what the pool holds rather than depend on the machine having a card. --- R/resident.R | 46 ++++++++++++++++++++++++++---- inst/tinytest/test_resident_sdxl.R | 31 +++++++++++++++++++- man/dot-resident_prewarm.Rd | 12 ++++++-- 3 files changed, 80 insertions(+), 9 deletions(-) diff --git a/R/resident.R b/R/resident.R index 6011cf4..9b68f19 100644 --- a/R/resident.R +++ b/R/resident.R @@ -334,26 +334,60 @@ resident_load <- function(model = c("flux2", "flux1", "zimage", "ltx", #' @param bytes Numeric. Host bytes about to be transferred; the pool is #' warmed to this plus a small margin for allocator slack. #' @param device Target CUDA device. +#' @param held Bytes the caching allocator already holds. NULL measures it. +#' Pass a value to make the decision deterministic: without CUDA the +#' measurement is 0, which would always warm, so a test that wants the +#' skip has to state what the pool holds rather than depend on the +#' machine having a card. Same reason \code{\link{.resident_check_fits}} +#' takes \code{free_gb}. #' -#' @return Invisibly NULL. +#' @return Invisibly, the bytes requested from the allocator: 0 when the +#' pool already covers the transfer and nothing was asked for. #' #' @keywords internal -.resident_prewarm <- function(bytes, device) { +.resident_prewarm <- function(bytes, device, held = NULL) { # isTRUE() rather than a bare is.finite(): a NULL pinned_bytes gives # logical(0), and `||` on a zero-length value is an error in R >= 4.3, # so the guard meant to skip the pre-warm would instead fail the # activation it exists to speed up. if (!isTRUE(is.finite(bytes)) || bytes <= 0) { - return(invisible(NULL)) + return(invisible(0)) } + # Only grow what is missing, and only when something IS missing. + # + # Asking for the full figure unconditionally doubles the pool on every + # activation after the first. A render fragments the cache into its + # activation blocks, so the next single large request cannot be served + # from it and takes a fresh cudaMalloc alongside the old block. With + # release = FALSE -- which a residency broker passes deliberately, to + # keep an exclusive grant's blocks off other tenants -- nothing ever + # empties the cache, so it grows by one block per cycle until + # activation is refused. Measured on SDXL: 5.299 GiB after cycle 1, + # 10.322 after cycle 2, refused on cycle 3, with the step (5.023 GiB) + # matching the pre-warm block (5.021 GiB) to two thousandths. + # + # Bare activate/deactivate cycles never showed it: without a render the + # block is reused cleanly and the pool holds flat. It takes a + # generation in between, which is why one cycle is not a test. + if (is.null(held)) { + held <- tryCatch({ + as.numeric(torch::cuda_memory_stats()$reserved_bytes$all$current) + }, error = function(e) 0) + } + if (!isTRUE(is.finite(held)) || held < 0) { + held <- 0 + } + if (held >= bytes) { + return(invisible(0)) + } + want <- (as.numeric(bytes) - held) * 1.05 tryCatch({ - warm <- torch::torch_empty(as.numeric(bytes) * 1.05, - dtype = torch::torch_uint8(), + warm <- torch::torch_empty(want, dtype = torch::torch_uint8(), device = device) rm(warm) gc(verbose = FALSE) }, error = function(e) invisible(NULL)) - invisible(NULL) + invisible(want) } #' Which components a bulk activation puts on the card diff --git a/inst/tinytest/test_resident_sdxl.R b/inst/tinytest/test_resident_sdxl.R index f872c83..82c0905 100644 --- a/inst/tinytest/test_resident_sdxl.R +++ b/inst/tinytest/test_resident_sdxl.R @@ -213,4 +213,33 @@ expect_silent(diffuseR:::.resident_prewarm(NA_real_, "cuda")) expect_silent(diffuseR:::.resident_prewarm(NULL, "cuda")) # An impossible size on a real card must be swallowed, not raised. expect_silent(diffuseR:::.resident_prewarm(1e18, "cuda")) -expect_null(diffuseR:::.resident_prewarm(1024, "cuda")) +expect_equal(diffuseR:::.resident_prewarm(0, "cuda"), 0) + +# It must GROW the pool, not re-request it. Asking for the full figure on +# every activation doubled the cache once a render had fragmented it: the +# single large request could not be served from cache and took a fresh +# cudaMalloc beside the old block. Under release = FALSE -- which a +# residency broker passes deliberately -- nothing empties the cache, so +# SDXL went 5.299 -> 10.322 GiB and the third activation was refused. +gb <- 1024^3 + +# Pool already covers the transfer: ask for nothing at all. +expect_equal(diffuseR:::.resident_prewarm(4 * gb, "cuda", held = 5 * gb), 0) +# Exactly equal still counts as covered. +expect_equal(diffuseR:::.resident_prewarm(4 * gb, "cuda", held = 4 * gb), 0) + +# Pool short: ask for the SHORTFALL, not the whole need. Requesting the +# whole need here is precisely the bug. +expect_equal(diffuseR:::.resident_prewarm(4 * gb, "cuda", held = 3 * gb), + 1 * gb * 1.05) + +# Cold pool: byte-identical to the original behaviour, so the 74x +# cold-start win is untouched. +expect_equal(diffuseR:::.resident_prewarm(4 * gb, "cuda", held = 0), + 4 * gb * 1.05) + +# A nonsense reading must not be trusted into a negative request. +expect_equal(diffuseR:::.resident_prewarm(4 * gb, "cuda", held = NA_real_), + 4 * gb * 1.05) +expect_equal(diffuseR:::.resident_prewarm(4 * gb, "cuda", held = -1), + 4 * gb * 1.05) diff --git a/man/dot-resident_prewarm.Rd b/man/dot-resident_prewarm.Rd index df25031..41d958c 100644 --- a/man/dot-resident_prewarm.Rd +++ b/man/dot-resident_prewarm.Rd @@ -3,16 +3,24 @@ \alias{.resident_prewarm} \title{Grow the caching allocator's pool in one allocation before a bulk onload} \usage{ -.resident_prewarm(bytes, device) +.resident_prewarm(bytes, device, held = NULL) } \arguments{ \item{bytes}{Numeric. Host bytes about to be transferred; the pool is warmed to this plus a small margin for allocator slack.} \item{device}{Target CUDA device.} + +\item{held}{Bytes the caching allocator already holds. NULL measures it. +Pass a value to make the decision deterministic: without CUDA the +measurement is 0, which would always warm, so a test that wants the +skip has to state what the pool holds rather than depend on the +machine having a card. Same reason \code{\link{.resident_check_fits}} +takes \code{free_gb}.} } \value{ -Invisibly NULL. +Invisibly, the bytes requested from the allocator: 0 when the + pool already covers the transfer and nothing was asked for. } \description{ A cold bulk onload grows the pool one \code{cudaMalloc} per tensor, and From fddd739f96c2b2618067e0d90bb021b3fbafe5b3 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Mon, 24 Aug 2026 13:50:39 -0500 Subject: [PATCH 2/6] Correct resident_generate()'s @return: every family returns a list The @return added in 0.2.2.4 said flux1, flux2 and zimage return bare image arrays, ltx returns a video array, and sdxl is the odd one out with list(image, metadata). All four claims are wrong, and the shape of the error is the damaging kind: it tells a consumer to special-case sdxl and treat flux2 as an array, which fails on first contact. Every generator returns a list: txt2img_flux invisible(list(image = img_array, metadata = metadata)) txt2img_flux2 invisible(list(image = img_array, metadata = metadata)) txt2img_zimage invisible(list(image = img_array, metadata = metadata)) txt2img_sdxl return(list(image = img_array, metadata = metadata)) txt2img_sd21 return(list(image = img_array, metadata = metadata)) txt2vid_ltx2 invisible(result) # video, audio, sample_rate, latents, # audio_latents, latent_shape So sdxl agrees with the other image families rather than diverging from them, and the corrected contract is simpler than the one it replaces: unwrap $image across all five image families, $video for ltx. The only real inconsistency left is return() against invisible(), which changes auto-printing at the console and nothing else. The decision not to normalise the shapes stands; only the stated reason for it was inverted. Observed at runtime for sdxl, sd21 and ltx during this work, confirmed for flux2 by the gpu.ctl side, and read from source for flux1 and zimage. --- R/resident.R | 20 +++++++++++++------- man/resident_generate.Rd | 20 +++++++++++++------- 2 files changed, 26 insertions(+), 14 deletions(-) diff --git a/R/resident.R b/R/resident.R index 9b68f19..648ff3d 100644 --- a/R/resident.R +++ b/R/resident.R @@ -635,13 +635,19 @@ resident_deactivate <- function(res, release = TRUE) { #' \code{sdxl} the handle supplies \code{devices} matching its own #' placement unless the caller names it. #' -#' @return Whatever the family generator returns, and the families do not -#' agree: an image array for \code{flux1}, \code{flux2} and -#' \code{zimage}, a video array for \code{ltx}, and for \code{sdxl} a -#' list of \code{image} and \code{metadata}, because -#' \code{\link{txt2img_sdxl}} has always returned that pair and changing -#' it would break every existing caller. A broker that wants one shape -#' should normalise in its own wrapper. +#' @return Whatever the family generator returns, which is always a list. +#' +#' The five image families (\code{flux1}, \code{flux2}, \code{zimage}, +#' \code{sdxl}, \code{sd21}) return \code{list(image, metadata)}, where +#' \code{image} is an [H, W, 3] array in [0, 1]. \code{ltx} returns a +#' richer list: \code{video}, \code{audio}, \code{sample_rate}, the raw +#' \code{latents} and \code{audio_latents}, and \code{latent_shape}. +#' +#' So a caller unwraps \code{$image} uniformly across the image families +#' and \code{$video} for \code{ltx}. The only inconsistency is how the +#' list is handed back -- \code{\link{txt2img_sdxl}} uses \code{return()} +#' and the rest use \code{invisible()} -- which affects auto-printing at +#' the console and nothing else. #' #' @export resident_generate <- function(res, prompt, ...) { diff --git a/man/resident_generate.Rd b/man/resident_generate.Rd index cc7c557..8c408d5 100644 --- a/man/resident_generate.Rd +++ b/man/resident_generate.Rd @@ -17,13 +17,19 @@ resident_generate(res, prompt, ...) placement unless the caller names it.} } \value{ -Whatever the family generator returns, and the families do not - agree: an image array for \code{flux1}, \code{flux2} and - \code{zimage}, a video array for \code{ltx}, and for \code{sdxl} a - list of \code{image} and \code{metadata}, because - \code{\link{txt2img_sdxl}} has always returned that pair and changing - it would break every existing caller. A broker that wants one shape - should normalise in its own wrapper. +Whatever the family generator returns, which is always a list. + + The five image families (\code{flux1}, \code{flux2}, \code{zimage}, + \code{sdxl}, \code{sd21}) return \code{list(image, metadata)}, where + \code{image} is an [H, W, 3] array in [0, 1]. \code{ltx} returns a + richer list: \code{video}, \code{audio}, \code{sample_rate}, the raw + \code{latents} and \code{audio_latents}, and \code{latent_shape}. + + So a caller unwraps \code{$image} uniformly across the image families + and \code{$video} for \code{ltx}. The only inconsistency is how the + list is handed back -- \code{\link{txt2img_sdxl}} uses \code{return()} + and the rest use \code{invisible()} -- which affects auto-printing at + the console and nothing else. } \description{ Dispatches to the family's generator with the resident pipeline From 9eab5b1145fc0de662a69f37ea26230f37813ed5 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Mon, 24 Aug 2026 13:57:28 -0500 Subject: [PATCH 3/6] Bump version to 0.2.2.6 --- DESCRIPTION | 2 +- NEWS.md | 25 +++++++++++++++++++++++++ 2 files changed, 26 insertions(+), 1 deletion(-) diff --git a/DESCRIPTION b/DESCRIPTION index b1f6bb5..d85c958 100644 --- a/DESCRIPTION +++ b/DESCRIPTION @@ -1,6 +1,6 @@ Package: diffuseR Title: Functional Interface to Diffusion Models in R -Version: 0.2.2.5 +Version: 0.2.2.6 Authors@R: c( person("Troy", "Hernandez", email = "troy@cornball.ai", role = c("aut", "cre"), comment = c(ORCID = "0009-0005-4248-604X")), diff --git a/NEWS.md b/NEWS.md index ecc67ba..6923710 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,28 @@ +# diffuseR 0.2.2.6 + +* Fixed an allocator pre-warm accumulation introduced in 0.2.2.4. + `.resident_prewarm()` requested the full onload need on every + activation, which doubled the CUDA caching allocator's pool after each + render: a generation fragments the cache, so the next single large + request cannot be served from it and takes a fresh allocation beside the + old one. Under `resident_deactivate(release = FALSE)` nothing empties + the cache, so SDXL went 5.299 GiB after one cycle to 10.322 after two + and refused the third. It now grows only the shortfall, and skips + entirely when the pool already covers the transfer; a cold pool is + unchanged. Measured flat at 5.396 / 5.498 / 5.498 GiB across three + cycles. + + This also corrected the budget independently of the refusal: both + release modes doubled between the first and second cycle, so any peak + measured on a single activation understated steady state roughly 2x. + +* `resident_generate()`'s documented return value was wrong. It claimed + `flux1`, `flux2` and `zimage` return bare image arrays and `sdxl` was + the exception. Every family returns a list: the five image families + return `list(image, metadata)` and `ltx` returns `video`, `audio`, + `sample_rate`, `latents`, `audio_latents` and `latent_shape`. Unwrap + `$image` across the image families and `$video` for `ltx`. + # diffuseR 0.2.2.5 * `resident_load()` accepts `"sd21"`, the sixth resident family. From 600f63026eed1f0a6dbfeb90f109ae0c7f6db250 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Mon, 24 Aug 2026 15:15:53 -0500 Subject: [PATCH 4/6] Address review: device-correct stats, margin on the final pool, doc fixes Three defects found reviewing the pre-warm fix, all real. READ THE ALLOCATOR ON THE HANDLE'S OWN DEVICE. cuda_memory_stats() takes `device` and defaults to cuda_current_device(), so calling it bare reports whichever device happens to be current rather than the one the handle bound. resident_load() binds an explicit "cuda:N" precisely so transitions cannot drift, and a cuda:1 handle deciding from cuda:0's pool would either skip a pre-warm it needs or repeat one it does not. Added .cuda_index() to derive the ordinal, with a test for an unqualified device and for a malformed one that must not become NA and poison the lookup. MEASURE FREE CACHE, NOT RESERVED. reserved includes live allocations, which belong to something else and cannot serve this transfer, so it overstates what is reusable. Now reserved minus allocated. This also tightened the result: the residual growth across cycles fell from 0.102 GiB to 0.002 GiB, because the decision is no longer made on bytes that were never available. PUT THE MARGIN ON THE FINAL POOL. (bytes - held) * 1.05 asks for 5% of the GAP, which undershoots the intended target whenever held > 0. The figure wanted is bytes * 1.05 - held. Growing the pool is best-effort in the partial case and the docs now say so rather than implying a guarantee: a request smaller than a free block already in the cache is served from that block and grows nothing. That is bounded and harmless -- the onload falls back to the per-tensor path for the remainder, which is the old behaviour -- and the alternative, asking for the whole figure to force a new segment, is the accumulation bug this function exists to avoid. The cold pool, which is the case worth optimising and the one a broker's first request hits, is unaffected. THE @return WAS STILL WRONG. It said sdxl alone uses return() while every other family uses invisible(); txt2img_sd21 uses return() too, which the commit that introduced the text had listed correctly two paragraphs earlier. And ltx's `video` and `audio` are produced only when decode_video/decode_audio are TRUE -- a caller that turns either off gets a list without the field, not a NULL one, which is why txt2vid_ltx2 indexes it with [[ ]] internally. VERIFIED, closing the three gaps the reviewer named rather than leaving them for verification: five cycles, release = FALSE, render each cycle 5.396 -> 5.398 -> 5.398 -> 5.398 -> 5.398 GiB cold-start win intact, measured not argued cycle 1 activate 2.48 s (2.51 s before the accumulation fix) cycles 2-5 activate 0.16 s a phase-offloading family is untouched flux2, three cycles: state=active, components_on_gpu=0, reserved 0.002 GiB throughout -- the bulk branch is never taken, so the pre-warm is never reached Suite 1209 assertions, 0 failures. --- R/resident.R | 88 ++++++++++++++++++++++++------ inst/tinytest/test_resident_sdxl.R | 23 +++++++- man/dot-resident_prewarm.Rd | 19 ++++--- man/resident_generate.Rd | 22 +++++--- 4 files changed, 116 insertions(+), 36 deletions(-) diff --git a/R/resident.R b/R/resident.R index 648ff3d..2399b76 100644 --- a/R/resident.R +++ b/R/resident.R @@ -334,12 +334,28 @@ resident_load <- function(model = c("flux2", "flux1", "zimage", "ltx", #' @param bytes Numeric. Host bytes about to be transferred; the pool is #' warmed to this plus a small margin for allocator slack. #' @param device Target CUDA device. -#' @param held Bytes the caching allocator already holds. NULL measures it. -#' Pass a value to make the decision deterministic: without CUDA the -#' measurement is 0, which would always warm, so a test that wants the -#' skip has to state what the pool holds rather than depend on the -#' machine having a card. Same reason \code{\link{.resident_check_fits}} -#' takes \code{free_gb}. +#' Growing the pool is best-effort in the partial case. A request smaller +#' than a free block already in the cache is served from that block and +#' grows nothing, so when the pool is short by less than it already holds +#' the pre-warm may be absorbed rather than add capacity. That is bounded +#' and harmless -- the onload then falls back to the per-tensor path for +#' the remainder, which is the old behaviour -- and the alternative, asking +#' for the whole figure to force a new segment, is the accumulation bug +#' this function exists to avoid. The cold pool, which is the case worth +#' optimising and the one a broker's first request hits, is unaffected. +#' +#' @param bytes Numeric. Host bytes about to be transferred; the pool is +#' warmed toward this plus a small margin for allocator slack. +#' @param device Target CUDA device, e.g. "cuda" or "cuda:1". Also selects +#' which device's allocator is measured. +#' @param held Free cached bytes the allocator already holds on that +#' device, i.e. reserved minus allocated -- bytes that are reserved but +#' live belong to something else and cannot serve this transfer. NULL +#' measures it. Pass a value to make the decision deterministic: without +#' CUDA the measurement is 0, which would always warm, so a test that +#' wants the skip has to state what the pool holds rather than depend on +#' the machine having a card. Same reason +#' \code{\link{.resident_check_fits}} takes \code{free_gb}. #' #' @return Invisibly, the bytes requested from the allocator: 0 when the #' pool already covers the transfer and nothing was asked for. @@ -371,16 +387,22 @@ resident_load <- function(model = c("flux2", "flux1", "zimage", "ltx", # generation in between, which is why one cycle is not a test. if (is.null(held)) { held <- tryCatch({ - as.numeric(torch::cuda_memory_stats()$reserved_bytes$all$current) + s <- torch::cuda_memory_stats(device = .cuda_index(device)) + as.numeric(s$reserved_bytes$all$current) - + as.numeric(s$allocated_bytes$all$current) }, error = function(e) 0) } if (!isTRUE(is.finite(held)) || held < 0) { held <- 0 } + # Enough cached to serve the transfer: nothing to grow. if (held >= bytes) { return(invisible(0)) } - want <- (as.numeric(bytes) - held) * 1.05 + # Grow toward bytes * 1.05, so the margin is on the FINAL pool rather + # than on the shortfall. (bytes - held) * 1.05 asks for 5% of the gap + # instead, which undershoots the target whenever held > 0. + want <- as.numeric(bytes) * 1.05 - held tryCatch({ warm <- torch::torch_empty(want, dtype = torch::torch_uint8(), device = device) @@ -390,6 +412,32 @@ resident_load <- function(model = c("flux2", "flux1", "zimage", "ltx", invisible(want) } +#' Device ordinal for a torch device string +#' +#' \code{torch::cuda_memory_stats()} defaults to +#' \code{cuda_current_device()}, so reading it without an argument reports +#' whichever device happens to be current rather than the one a handle is +#' bound to. \code{resident_load()} binds an explicit \code{"cuda:N"} +#' precisely so transitions cannot drift, and a handle on \code{cuda:1} +#' deciding from \code{cuda:0}'s pool would either skip a pre-warm it needs +#' or repeat one it does not. +#' +#' @param device Character, e.g. "cuda", "cuda:0", "cuda:1". +#' +#' @return Integer ordinal. An unqualified device gives the current one. +#' +#' @keywords internal +.cuda_index <- function(device) { + d <- as.character(device)[[1]] + if (grepl(":", d, fixed = TRUE)) { + n <- suppressWarnings(as.integer(sub("^.*:", "", d))) + if (!is.na(n)) { + return(n) + } + } + tryCatch(torch::cuda_current_device(), error = function(e) 0L) +} + #' Which components a bulk activation puts on the card #' #' All of them, unless the family says otherwise. @@ -639,15 +687,21 @@ resident_deactivate <- function(res, release = TRUE) { #' #' The five image families (\code{flux1}, \code{flux2}, \code{zimage}, #' \code{sdxl}, \code{sd21}) return \code{list(image, metadata)}, where -#' \code{image} is an [H, W, 3] array in [0, 1]. \code{ltx} returns a -#' richer list: \code{video}, \code{audio}, \code{sample_rate}, the raw -#' \code{latents} and \code{audio_latents}, and \code{latent_shape}. -#' -#' So a caller unwraps \code{$image} uniformly across the image families -#' and \code{$video} for \code{ltx}. The only inconsistency is how the -#' list is handed back -- \code{\link{txt2img_sdxl}} uses \code{return()} -#' and the rest use \code{invisible()} -- which affects auto-printing at -#' the console and nothing else. +#' \code{image} is an [H, W, 3] array in [0, 1], so a caller unwraps +#' \code{$image} uniformly across all five. +#' +#' \code{ltx} returns \code{latents}, \code{audio_latents}, +#' \code{latent_shape} and \code{sample_rate}, plus \code{video} and +#' \code{audio} -- but those two are produced only when +#' \code{decode_video} and \code{decode_audio} are TRUE, which they are +#' by default. A caller that turns either off gets a list without that +#' field rather than a NULL one, so index it with \code{[[ ]]} and check, +#' the way \code{\link{txt2vid_ltx2}} does internally. +#' +#' Only the visibility differs: \code{\link{txt2img_sdxl}} and +#' \code{\link{txt2img_sd21}} use \code{return()} while the other three +#' image families and \code{ltx} use \code{invisible()}, which affects +#' auto-printing at the console and nothing else. #' #' @export resident_generate <- function(res, prompt, ...) { diff --git a/inst/tinytest/test_resident_sdxl.R b/inst/tinytest/test_resident_sdxl.R index 82c0905..cbde83c 100644 --- a/inst/tinytest/test_resident_sdxl.R +++ b/inst/tinytest/test_resident_sdxl.R @@ -228,10 +228,11 @@ expect_equal(diffuseR:::.resident_prewarm(4 * gb, "cuda", held = 5 * gb), 0) # Exactly equal still counts as covered. expect_equal(diffuseR:::.resident_prewarm(4 * gb, "cuda", held = 4 * gb), 0) -# Pool short: ask for the SHORTFALL, not the whole need. Requesting the -# whole need here is precisely the bug. +# Pool short: grow toward bytes * 1.05, so the 5% margin lands on the +# FINAL pool. (bytes - held) * 1.05 would ask for 5% of the gap instead +# and undershoot the target whenever held > 0. expect_equal(diffuseR:::.resident_prewarm(4 * gb, "cuda", held = 3 * gb), - 1 * gb * 1.05) + 4 * gb * 1.05 - 3 * gb) # Cold pool: byte-identical to the original behaviour, so the 74x # cold-start win is untouched. @@ -243,3 +244,19 @@ expect_equal(diffuseR:::.resident_prewarm(4 * gb, "cuda", held = NA_real_), 4 * gb * 1.05) expect_equal(diffuseR:::.resident_prewarm(4 * gb, "cuda", held = -1), 4 * gb * 1.05) + +# --- the allocator is read on the handle's own device ------------------------------- + +# cuda_memory_stats() defaults to cuda_current_device(), so reading it +# without an argument reports whichever device is current rather than the +# one the handle bound. resident_load() binds an explicit "cuda:N" so +# transitions cannot drift; a cuda:1 handle deciding from cuda:0's pool +# would skip a pre-warm it needs or repeat one it does not. +expect_equal(diffuseR:::.cuda_index("cuda:0"), 0L) +expect_equal(diffuseR:::.cuda_index("cuda:1"), 1L) +expect_equal(diffuseR:::.cuda_index("cuda:7"), 7L) +# Unqualified falls back to the current device, whatever that is. +expect_true(is.numeric(diffuseR:::.cuda_index("cuda"))) +# A malformed ordinal must not become NA and poison the stats lookup. +expect_true(is.numeric(diffuseR:::.cuda_index("cuda:x"))) +expect_false(is.na(diffuseR:::.cuda_index("cuda:x"))) diff --git a/man/dot-resident_prewarm.Rd b/man/dot-resident_prewarm.Rd index 41d958c..6d0bd6d 100644 --- a/man/dot-resident_prewarm.Rd +++ b/man/dot-resident_prewarm.Rd @@ -7,16 +7,19 @@ } \arguments{ \item{bytes}{Numeric. Host bytes about to be transferred; the pool is -warmed to this plus a small margin for allocator slack.} +warmed toward this plus a small margin for allocator slack.} -\item{device}{Target CUDA device.} +\item{device}{Target CUDA device, e.g. "cuda" or "cuda:1". Also selects +which device's allocator is measured.} -\item{held}{Bytes the caching allocator already holds. NULL measures it. -Pass a value to make the decision deterministic: without CUDA the -measurement is 0, which would always warm, so a test that wants the -skip has to state what the pool holds rather than depend on the -machine having a card. Same reason \code{\link{.resident_check_fits}} -takes \code{free_gb}.} +\item{held}{Free cached bytes the allocator already holds on that +device, i.e. reserved minus allocated -- bytes that are reserved but +live belong to something else and cannot serve this transfer. NULL +measures it. Pass a value to make the decision deterministic: without +CUDA the measurement is 0, which would always warm, so a test that +wants the skip has to state what the pool holds rather than depend on +the machine having a card. Same reason +\code{\link{.resident_check_fits}} takes \code{free_gb}.} } \value{ Invisibly, the bytes requested from the allocator: 0 when the diff --git a/man/resident_generate.Rd b/man/resident_generate.Rd index 8c408d5..b41e4d7 100644 --- a/man/resident_generate.Rd +++ b/man/resident_generate.Rd @@ -21,15 +21,21 @@ Whatever the family generator returns, which is always a list. The five image families (\code{flux1}, \code{flux2}, \code{zimage}, \code{sdxl}, \code{sd21}) return \code{list(image, metadata)}, where - \code{image} is an [H, W, 3] array in [0, 1]. \code{ltx} returns a - richer list: \code{video}, \code{audio}, \code{sample_rate}, the raw - \code{latents} and \code{audio_latents}, and \code{latent_shape}. + \code{image} is an [H, W, 3] array in [0, 1], so a caller unwraps + \code{$image} uniformly across all five. - So a caller unwraps \code{$image} uniformly across the image families - and \code{$video} for \code{ltx}. The only inconsistency is how the - list is handed back -- \code{\link{txt2img_sdxl}} uses \code{return()} - and the rest use \code{invisible()} -- which affects auto-printing at - the console and nothing else. + \code{ltx} returns \code{latents}, \code{audio_latents}, + \code{latent_shape} and \code{sample_rate}, plus \code{video} and + \code{audio} -- but those two are produced only when + \code{decode_video} and \code{decode_audio} are TRUE, which they are + by default. A caller that turns either off gets a list without that + field rather than a NULL one, so index it with \code{[[ ]]} and check, + the way \code{\link{txt2vid_ltx2}} does internally. + + Only the visibility differs: \code{\link{txt2img_sdxl}} and + \code{\link{txt2img_sd21}} use \code{return()} while the other three + image families and \code{ltx} use \code{invisible()}, which affects + auto-printing at the console and nothing else. } \description{ Dispatches to the family's generator with the resident pipeline From f20f9f1e903cdaf0373dc83e25eed1a5e0c53426 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Mon, 24 Aug 2026 15:16:04 -0500 Subject: [PATCH 5/6] NEWS: reflect the corrected return-value description --- NEWS.md | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/NEWS.md b/NEWS.md index 6923710..ce6a1be 100644 --- a/NEWS.md +++ b/NEWS.md @@ -19,9 +19,13 @@ * `resident_generate()`'s documented return value was wrong. It claimed `flux1`, `flux2` and `zimage` return bare image arrays and `sdxl` was the exception. Every family returns a list: the five image families - return `list(image, metadata)` and `ltx` returns `video`, `audio`, - `sample_rate`, `latents`, `audio_latents` and `latent_shape`. Unwrap - `$image` across the image families and `$video` for `ltx`. + return `list(image, metadata)`, so `$image` unwraps uniformly across all + five. `ltx` returns `latents`, `audio_latents`, `latent_shape` and + `sample_rate`, plus `video` and `audio` when `decode_video` / + `decode_audio` are TRUE — a caller that turns either off gets a list + without that field rather than a NULL one. Only visibility differs: + `txt2img_sdxl()` and `txt2img_sd21()` use `return()`, the rest + `invisible()`. # diffuseR 0.2.2.5 From 143f5520990791e7f3b59f2179b9bfbd2b6108f9 Mon Sep 17 00:00:00 2001 From: TroyHernandez Date: Mon, 24 Aug 2026 15:35:20 -0500 Subject: [PATCH 6/6] Address re-review: one target, roxygen ordering, tracked Rd, current NEWS Three follow-ups from the re-review, none functional bugs but the first was a real inconsistency and the second and third would have shipped wrong. ONE TARGET FOR BOTH THE SKIP AND THE SIZE. The skip tested `held >= bytes` while the growth aimed at `bytes * 1.05`, which put a step in the middle: 3.999 GiB held asked for 0.201 GiB and 4.000 GiB held asked for nothing. `target` is now computed once and governs both, so the two cannot disagree. Added a test that holding a hair under and a hair over the raw need differ by a hair rather than by the whole margin -- the case the old threshold got wrong. ROXYGEN ORDERING SWALLOWED THE EXPLANATION. The partial-cache paragraph was written after `@param device`, so roxygen read it as a continuation of that tag rather than as description, and the block also carried a duplicate `@param bytes`/`@param device` pair from before the edit. The generated page therefore did not contain the best-effort rationale at all -- the reason the function does not force pool growth was documented in the source and absent from the docs. Prose now precedes the tags and the duplicates are gone; verified against the generated Rd rather than assumed. man/dot-cuda_index.Rd WAS UNTRACKED. `git add -u` stages tracked files only, so a newly generated Rd was silently left out of the previous commit. Staged by explicit path. This is the "never git add -A" rule biting from the other side: -u is the safe default precisely because it ignores untracked files, which means new files need naming. NEWS CARRIED THE SUPERSEDED MEASUREMENTS. It still reported 5.396 / 5.498 / 5.498 from before the free-cache correction, rather than the current 5.396 / 5.398 across five cycles. Also now records the cold-start figure and the phase-offload check, since those are the numbers a reader would otherwise have to take on trust. Re-verified after the threshold change: five cycles at 5.396 / 5.398 / 5.398 / 5.398 / 5.398 GiB, cold start 2.52 s, warm activations 0.17 s. Suite 1211 assertions, 0 failures. --- NEWS.md | 11 +++++++---- R/resident.R | 16 +++++++--------- inst/tinytest/test_resident_sdxl.R | 17 ++++++++++++++--- man/dot-cuda_index.Rd | 23 +++++++++++++++++++++++ man/dot-resident_prewarm.Rd | 10 ++++++++++ 5 files changed, 61 insertions(+), 16 deletions(-) create mode 100644 man/dot-cuda_index.Rd diff --git a/NEWS.md b/NEWS.md index ce6a1be..a7e85df 100644 --- a/NEWS.md +++ b/NEWS.md @@ -7,10 +7,13 @@ request cannot be served from it and takes a fresh allocation beside the old one. Under `resident_deactivate(release = FALSE)` nothing empties the cache, so SDXL went 5.299 GiB after one cycle to 10.322 after two - and refused the third. It now grows only the shortfall, and skips - entirely when the pool already covers the transfer; a cold pool is - unchanged. Measured flat at 5.396 / 5.498 / 5.498 GiB across three - cycles. + and refused the third. It now measures the free cache on the handle's + own device and grows only the shortfall, skipping entirely when the pool + already covers the transfer; a cold pool is unchanged, and the + cold-start win is intact (2.48 s against 2.51 s before). Measured flat + at 5.396 / 5.398 / 5.398 / 5.398 / 5.398 GiB across five cycles, with a + phase-offloading family (flux2) untouched because it never takes the + bulk branch. This also corrected the budget independently of the refusal: both release modes doubled between the first and second cycle, so any peak diff --git a/R/resident.R b/R/resident.R index 2399b76..29e5eb2 100644 --- a/R/resident.R +++ b/R/resident.R @@ -331,9 +331,6 @@ resident_load <- function(model = c("flux2", "flux1", "zimage", "ltx", #' the per-tensor path, which is the current behaviour and merely slow, so #' the failure is swallowed rather than raised. #' -#' @param bytes Numeric. Host bytes about to be transferred; the pool is -#' warmed to this plus a small margin for allocator slack. -#' @param device Target CUDA device. #' Growing the pool is best-effort in the partial case. A request smaller #' than a free block already in the cache is served from that block and #' grows nothing, so when the pool is short by less than it already holds @@ -395,14 +392,15 @@ resident_load <- function(model = c("flux2", "flux1", "zimage", "ltx", if (!isTRUE(is.finite(held)) || held < 0) { held <- 0 } - # Enough cached to serve the transfer: nothing to grow. - if (held >= bytes) { + # One target, used for both the skip and the size, so the two cannot + # disagree. Skipping at `held >= bytes` while growing toward + # `bytes * 1.05` put a step in the middle: 3.999 GiB held asked for + # 0.201 GiB and 4.000 GiB held asked for nothing. + target <- as.numeric(bytes) * 1.05 + if (held >= target) { return(invisible(0)) } - # Grow toward bytes * 1.05, so the margin is on the FINAL pool rather - # than on the shortfall. (bytes - held) * 1.05 asks for 5% of the gap - # instead, which undershoots the target whenever held > 0. - want <- as.numeric(bytes) * 1.05 - held + want <- target - held tryCatch({ warm <- torch::torch_empty(want, dtype = torch::torch_uint8(), device = device) diff --git a/inst/tinytest/test_resident_sdxl.R b/inst/tinytest/test_resident_sdxl.R index cbde83c..8a59eaf 100644 --- a/inst/tinytest/test_resident_sdxl.R +++ b/inst/tinytest/test_resident_sdxl.R @@ -223,10 +223,13 @@ expect_equal(diffuseR:::.resident_prewarm(0, "cuda"), 0) # SDXL went 5.299 -> 10.322 GiB and the third activation was refused. gb <- 1024^3 -# Pool already covers the transfer: ask for nothing at all. +# One target governs both the skip and the size. Skipping at held >= bytes +# while growing toward bytes * 1.05 put a step in the middle, so the +# threshold is the target itself. expect_equal(diffuseR:::.resident_prewarm(4 * gb, "cuda", held = 5 * gb), 0) -# Exactly equal still counts as covered. -expect_equal(diffuseR:::.resident_prewarm(4 * gb, "cuda", held = 4 * gb), 0) +# Exactly at the target still counts as covered. +expect_equal(diffuseR:::.resident_prewarm(4 * gb, "cuda", held = 4 * gb * 1.05), + 0) # Pool short: grow toward bytes * 1.05, so the 5% margin lands on the # FINAL pool. (bytes - held) * 1.05 would ask for 5% of the gap instead @@ -234,6 +237,14 @@ expect_equal(diffuseR:::.resident_prewarm(4 * gb, "cuda", held = 4 * gb), 0) expect_equal(diffuseR:::.resident_prewarm(4 * gb, "cuda", held = 3 * gb), 4 * gb * 1.05 - 3 * gb) +# No discontinuity around `bytes`: holding a hair under and a hair over the +# raw need must differ by a hair, not by the whole margin. This is the case +# the old threshold got wrong. +lo <- diffuseR:::.resident_prewarm(4 * gb, "cuda", held = 4 * gb - 1) +hi <- diffuseR:::.resident_prewarm(4 * gb, "cuda", held = 4 * gb + 1) +expect_true(abs(lo - hi) < 10) +expect_true(lo > 0 && hi > 0) + # Cold pool: byte-identical to the original behaviour, so the 74x # cold-start win is untouched. expect_equal(diffuseR:::.resident_prewarm(4 * gb, "cuda", held = 0), diff --git a/man/dot-cuda_index.Rd b/man/dot-cuda_index.Rd new file mode 100644 index 0000000..bf60aba --- /dev/null +++ b/man/dot-cuda_index.Rd @@ -0,0 +1,23 @@ +% tinyrox says don't edit this manually, but it can't stop you! +\name{.cuda_index} +\alias{.cuda_index} +\title{Device ordinal for a torch device string} +\usage{ +.cuda_index(device) +} +\arguments{ +\item{device}{Character, e.g. "cuda", "cuda:0", "cuda:1".} +} +\value{ +Integer ordinal. An unqualified device gives the current one. +} +\description{ +\code{torch::cuda_memory_stats()} defaults to +\code{cuda_current_device()}, so reading it without an argument reports +whichever device happens to be current rather than the one a handle is +bound to. \code{resident_load()} binds an explicit \code{"cuda:N"} +precisely so transitions cannot drift, and a handle on \code{cuda:1} +deciding from \code{cuda:0}'s pool would either skip a pre-warm it needs +or repeat one it does not. +} +\keyword{internal} diff --git a/man/dot-resident_prewarm.Rd b/man/dot-resident_prewarm.Rd index 6d0bd6d..a8912ad 100644 --- a/man/dot-resident_prewarm.Rd +++ b/man/dot-resident_prewarm.Rd @@ -42,5 +42,15 @@ Best-effort. A card that cannot seat the block in one piece falls back to the per-tensor path, which is the current behaviour and merely slow, so the failure is swallowed rather than raised. +Growing the pool is best-effort in the partial case. A request smaller +than a free block already in the cache is served from that block and +grows nothing, so when the pool is short by less than it already holds +the pre-warm may be absorbed rather than add capacity. That is bounded +and harmless -- the onload then falls back to the per-tensor path for +the remainder, which is the old behaviour -- and the alternative, asking +for the whole figure to force a new segment, is the accumulation bug +this function exists to avoid. The cold pool, which is the case worth +optimising and the one a broker's first request hits, is unaffected. + } \keyword{internal}