Skip to content

Scratch param#24140

Open
loreball wants to merge 5 commits intobevyengine:mainfrom
loreball:scratch-param
Open

Scratch param#24140
loreball wants to merge 5 commits intobevyengine:mainfrom
loreball:scratch-param

Conversation

@loreball
Copy link
Copy Markdown
Contributor

@loreball loreball commented May 5, 2026

Objective

Introduce a system param for collections holding scratch data.
Closes #23775

Solution

Implement two new items to bevy_ecs: Scratch and ClearableCollection. Scratch is the new system param for hodling scratch data. It holds a ClearableCollection which it clears on every system run.

I went with a trait based solution instead of ScratchVec<T> to prevent duplication of the capacity management code. That said I tried to not over-complicate the capacity management code itself. It currently just runs a 10 000 tick timer that resets every time the allocation capacity grows. If it runs down we try to halve the allocation size.

Testing

The PR comes with 2 new tests, but I still want to benchmark how using this affects render asset extraction (which is why this pr is a draft for now).


Showcase

TODO

@loreball
Copy link
Copy Markdown
Contributor Author

loreball commented May 5, 2026

There was a suggestion on the original issue to make Scratch<T> always Send since it's always empty when running a system. We can't do that. We can't run custom SystemParam code after a system runs so we do the clearing when creating the param for a system run. If the system fills the collection on thread A and then runs on thread B, we'll end up dropping the data on thread B. Dropping !Send data on a different thread could lead to UB.

@loreball loreball force-pushed the scratch-param branch 3 times, most recently from 052f442 to b8ee91e Compare May 5, 2026 19:03
@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-ECS Entities, components, systems, and events S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels May 5, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in ECS May 5, 2026
@alice-i-cecile alice-i-cecile added C-Usability A targeted quality-of-life change that makes Bevy easier to use A-Rendering Drawing game state to the screen S-Needs-Benchmarking This set of changes needs performance benchmarking to double-check that they help and removed C-Feature A new feature, making something new possible labels May 5, 2026
@github-project-automation github-project-automation Bot moved this to Needs SME Triage in Rendering May 5, 2026
@alice-i-cecile alice-i-cecile added the X-Contentious There are nontrivial implications that should be thought through label May 6, 2026
@beicause
Copy link
Copy Markdown
Member

beicause commented May 6, 2026

We can't run custom SystemParam code after a system runs so we do the clearing when creating the param for a system run.

Is it possible to overcome this?

Also Scratch<T> should not require Sync like Local<T> does.

@loreball
Copy link
Copy Markdown
Contributor Author

loreball commented May 7, 2026

Doing a quick benchmark it seems pretty neutral for asset extraction.
image
(Ignore the +2% my system is pretty noisy I've seen it jump +- 4%)

@loreball
Copy link
Copy Markdown
Contributor Author

We can't run custom SystemParam code after a system runs so we do the clearing when creating the param for a system run.

Is it possible to overcome this?

I don't think so. Not at least while maintaining panic safety.

The way it would work is we would add a new method to SystemParam and friends. Let's call it post_system_run. Then we'd add a new safety requirement to get_param. post_system_run must always be run between calls to get_param. So let's take look at where we run a system

// ...
let params = unsafe {
    F::Param::get_param(&mut state.param, &self.system_meta, world, change_tick)
}?;

/// <ignore hotpatching impl>
let out = self.func.run(input, params);

/// We would add a `post_system_run` call here

self.system_meta.last_run = change_tick;

If self.func.run panics we'd never execute post_system_run. Catching the panic would then allow you to trigger the whole thing twice. The standard solution is to define a new struct that maintains safety invariants on drop, since panics run drop code. This is where our own permissiveness bites us in the ass. The SystemParam::Item is allowed to borrow the SystemParam::State meaning our drop handler which also needs mutable access to the state cannot be alive before the system runs.

@loreball
Copy link
Copy Markdown
Contributor Author

I'm pulling this out of draft since this version is mostly done. I'll hold off on writing the showcase until I get at least one review. Hopefully, the safety issue can be hashed out and we get to push the best versions of this across the finish line.

@loreball loreball marked this pull request as ready for review May 10, 2026 11:38
Comment thread crates/bevy_ecs/src/system/scratch.rs Outdated
}

/// SAFETY: We only access world change ticks immutably. We can't conflict with anything.
unsafe impl<'a, T: ClearableCollection + FromWorld + Send + Sync + 'static> SystemParam
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Local does not require Sync, why Scratch require it?

@loreball loreball requested a review from beicause May 10, 2026 15:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-ECS Entities, components, systems, and events A-Rendering Drawing game state to the screen C-Usability A targeted quality-of-life change that makes Bevy easier to use S-Needs-Benchmarking This set of changes needs performance benchmarking to double-check that they help S-Needs-Review Needs reviewer attention (from anyone!) to move forward X-Contentious There are nontrivial implications that should be thought through

Projects

Status: Needs SME Triage
Status: Needs SME Triage

Development

Successfully merging this pull request may close these issues.

Create a Scratch<T> system param for the "reused Local" pattern

3 participants