Repository navigation
GPU: keep an effect chain's result on the device for the compositor instead of uploading it again - #1441
GPU: keep an effect chain's result on the device for the compositor instead of uploading it again#1441MatejKozlovsky wants to merge 3 commits into
Conversation
…instead of being uploaded again after its readback
…er reuses it An effect chain's result handed to the upload cache (adopt_upload) is read by the frame's encoder, which may not be submitted yet. When the cache entry went (its layer buffer died mid-frame, e.g. a 4K export whose layer cache can't keep the buffers), the texture went straight back to the owner Enc's private list, skipping the open-encoder guard, and with a lent owner (storytold#1523) the next same-size chain overwrote it before the frame read it. The lease now remembers it escaped and goes through Pool::release. Tests: a pool-level check that a handed-off texture is not reacquired while another encoder is open, and a frame of five same-size blur + glow layers whose buffers die mid-frame (asserting results really were released mid-frame), vs the CPU and vs the hand-off off. On storytold#1441 alone only the pool test fails without the fix; the frame test covers the interaction with storytold#1523's lent owners. EFFECTCRAFT_NO_HANDOFF is now an AtomicBool so tests can switch it (restored by a guard, also on panic).
… of a thread-local; the CLI reads EFFECTCRAFT_NO_HANDOFF Review points on storytold#1441: - No thread-local LAST_CHAIN: Accelerator::effects_held returns the device copy with the result, the renderer's CpuFx owns it for the stack and drops it when a CPU effect runs, so nothing is pinned past the call that made it (paths that use effects() keep nothing). - ChainHold is an opaque struct instead of Box<dyn Any>: it records the pixels it was made for and the renderer checks ChainHold::is_for (address and length) before adopt. - The library no longer reads EFFECTCRAFT_NO_HANDOFF: Gpu::set_handoff, set by the CLI.
37ce989 to
d55dd74
Compare
|
Follow-up measurements with this PR at d55dd74, now pushed: rebased onto main 8f02f52, plus the open-encoder fix and Setup: RTX 3090 24 GB (Vulkan, driver 595.97) / Ryzen 7 5800X, Windows 10, sample d (30 blur + glow layers), 30 frames, H.264, On main (8f02f52; main has since moved, with no GPU or renderer changes), one session:
On #1523 (73312c2). The 4K and 1080p columns come from two different sessions, so compare within a column:
Limits: one GPU, one scene, Vulkan only. Not checked on an 8 GB card, on AMD or Intel, or on DX12. |
|
A data point from AMD, since the caveats say it hasn't been checked there: RX 6900 XT 16 GB, Windows 11, Adrenalin 32.0.21045.5002, Vulkan, head I don't have sample d, so this is a stand-in: 30 animated 300 px solids (scaled ×2 at 4K), each with Gaussian Blur (15–35) + Glow (radius 40), 30 frames, H.264. It's much lighter than yours, so read it as a check for correctness and memory, not as a reproduction of your timings.
Not checked: DX12 ( Generated with Claude Code |
|
Thanks a lot, this is really useful. Seeing byte-identical output and lower peak VRAM with the hand-off on is good news for the AMD path. Here's sample d. Save the two scripts below as (On Windows without bash, concatenate the two files into one
var FPS=30, W=3840, H=2160, DUR=10;
function rnd(i){ var x=Math.sin(i*12.9898)*43758.5453; return x-Math.floor(x); }
function comp(name){ return app.project.items.addComp(name, W, H, 1, DUR, FPS); }
function T(l,n){ return l.property("ADBE Transform Group").property(n); }
function anim(l,i){
T(l,"ADBE Position").setValueAtTime(0,[rnd(i)*W, rnd(i+1)*H]);
T(l,"ADBE Position").setValueAtTime(DUR,[rnd(i+2)*W, rnd(i+3)*H]);
T(l,"ADBE Rotate Z").setValueAtTime(0,0); T(l,"ADBE Rotate Z").setValueAtTime(DUR,360*rnd(i+4));
T(l,"ADBE Scale").setValueAtTime(0,[60,60]); T(l,"ADBE Scale").setValueAtTime(DUR,[60+80*rnd(i+5),60+80*rnd(i+5)]);
}
function shape(c,i){
var sh=c.layers.addShape();
var g=sh.property("ADBE Root Vectors Group").addProperty("ADBE Vector Group");
var r=g.property("ADBE Vectors Group").addProperty(i%2?"ADBE Vector Shape - Ellipse":"ADBE Vector Shape - Rect");
r.property("Size").setValue([200+300*rnd(i),200+300*rnd(i+9)]);
var f=g.property("Contents").addProperty("ADBE Vector Graphic - Fill");
f.property("Color").setValue([rnd(i),rnd(i+7),rnd(i+8)]);
return sh;
}
function fx(l,name,props){ var e=l.property("ADBE Effect Parade").addProperty(name); for(var k in props){ e.property(k).setValue(props[k]); } return e; }
function text(c,i,s){ var t=c.layers.addText(s); var sp=t.property("ADBE Text Properties").property("ADBE Text Document"); var d=sp.value; d.fontSize=120+80*rnd(i); d.fillColor=[1,rnd(i),rnd(i+3)]; sp.setValue(d); return t; }
var c=comp("Bench");
c.layers.addSolid([0,0,0],"BG",W,H,1);
for(var i=0;i<30;i++){ var l=(i%3==0)?text(c,i,"GLOW"):shape(c,i); anim(l,i);
fx(l,"Gaussian Blur",{"Blurriness":80+150*rnd(i)}); fx(l,"Glow",{"Glow Radius":200+300*rnd(i),"Glow Intensity":2});
if(i%3==1) fx(l,"Fast Box Blur",{"Blur Radius":100,"Repeat Edge Pixels":true}); }My exact render command was the following, with One heads-up: on |
This branch has not been deployed
2 participants
Base:
main. Three commits: the hand-off itself, a fix so a handed-off result waits for open encoders before it is reused, and the chain hold returned fromAccelerator::effects_heldinstead of a thread-local. It no longer stacks on #1415 (closed in favour of #1523). It works onmainalone, but its 4K gain shows only on top of #1523 (see Results and "Relationship to #1523").Problem
With GPU acceleration, a layer whose effects run on the GPU (
Gpu::effects→run_chain) makes three trips:Blur and glow make the buffer bigger by their reach, so the result is larger than the input. Step 3 uploads that largest buffer again, though the GPU already had it.
Change
run_chain_heldreturns the chain's result and, when the hand-off is on, the device image that was read back.run_chainis now a wrapper that drops the image. The newAccelerator::effects_held(default:effectswith no hold) wraps that image in an opaqueChainHoldthat records the address and length of the pixels it was read back into. The caller owns the hold, and nothing is kept in the accelerator or in a thread-local. Results served through deferred readback (the browser worker path) carry no hold.CpuFx) callseffects_heldand keeps the hold only while its buffer is still that chain's result: any CPU effect after the chain drops it.content_bufwraps the stack's result in the layerArcand, only ifChainHold::is_forconfirms theArcholds those very pixels, passes it to the newAccelerator::adopt(default: nothing). Other callers of the effect stack (layer_input,apply_effects_timed) drop the hold.Gpu::adoptfiles the image in the existing upload cache under thatArc(adopt_upload, size-checked). The cache identifies buffers byArcidentity, checked through aWeak, and adopted entries count against its budget and are swept like uploads. The compositor'supload_bufthen finds the texture and skips step 3. The readback (step 2) still happens.escaped. When it is dropped it goes throughPool::release, which waits for every open encoder, instead of straight back to theEncthat made it. Without this, the frame's encoder could still have an unsubmitted read of the texture while the owner's next chain overwrites it.Gpu::set_handoff(on by default). For A/B comparisons, the CLI turns it off for--gpusessions andbench --gpuwhenEFFECTCRAFT_NO_HANDOFFis set. Other hosts (the app) leave it on. With it off, no image is held.handed_off_texture_waits_for_open_encoders, andhandoff_survives_layer_buffers_dying_mid_frame(frame matches the CPU and the hand-off-off render while handed-off results die mid-frame). render:chain_hold_matches_only_its_own_pixels.Results
RTX 3090 24 GB (Vulkan, driver 595.97), Ryzen 7 5800X, Windows 10. Sample d (30 blur + glow layers), 30 frames, H.264,
--gpu,EFFECTCRAFT_GPU_INFLIGHT=1(the default), 3 runs per cell. "Off" is the same exe withEFFECTCRAFT_NO_HANDOFF=1. Wall time is mean (min–max). Peak VRAM is device-wide (nvidia-smi, includes about 0.9 GiB idle). Full measurements and caveats: #1441 (comment)On
main(8f02f52), one session:On #1523 (73312c2), with the one-line
Enc::lentchange below. The 4K and 1080p columns are separate sessions (the 1080p one ran about 4.5 % slower overall), so compare within a column:mainalone: no gain at 4K, about 10 % faster at 1080p (35.5 → 31.8 s). At 4K, main fills the card and its effects most likely end up on the CPU. A CPU-only render of main gives a byte-identical file in about the same time (238 s, one run, later session). I didn't capture the fallback in a log. Without a GPU chain result there is nothing to hand off.Relationship to #1523
Gpu::effects_heldhas to useEnc::lentlike GPU: free effect working textures after each call instead of at frame end #1523'sGpu::effects. That is a one-line change fromEnc::new.Enc::lentexists only in GPU: free effect working textures after each call instead of at frame end #1523, so the change isn't on this branch. It follows here once GPU: free effect working textures after each call instead of at frame end #1523 has merged. WithEnc::newthere, the stack builds and the focused tests pass (none of them catches it), but effect chains skip GPU: free effect working textures after each call instead of at frame end #1523's per-call freeing, and 4K goes back to main's level (about 240 s, 23.4 GiB).crates/gpu/src/context.rs(both add anescapedflag onLease; I kept GPU: free effect working textures after each call instead of at frame end #1523's field and this PR'sDrophandling) andcrates/gpu/src/tests.rs(both add tests). The "on GPU: free effect working textures after each call instead of at frame end #1523" rows include that hand merge.Testing
At d55dd74:
cargo fmt --all -- --check: clean.cargo clippy --no-deps -p effectcraft-gpu -p effectcraft-render -p effectcraft-cli --all-targets -- -D warnings: clean.cargo test -p effectcraft-gpu --lib(llvmpipe): 214 passed, 2 failed. The two failures (tests_fx_pixel2::stylize,tests_fx_stylize::posterize_thresholds_strobe_vignette) are known llvmpipe failures that also fail onmain.cargo test -p effectcraft-render: 221 passed, 1 ignored (plusmotion_blur_probe1 passed).Not in this PR
GpuFx); unchanged.Caveats