Add tools to avoid unnecessary `AssetEvent::Modified` events that lead to rendering performance costs (#16751) #22460

MatrixDev · 2026-01-09T17:39:56Z

Objective

Fixes Add tools to avoid unnecessary AssetEvent::Modified events that lead to rendering performance costs #16751

Solution

Assets::get_mut now returns a wrapper AssetMut type instead of &mut impl Asset.
AssetMut implements Deref and DerefMut.
DerefMut marks assets as changed.
when dropped AssetMut will add AssetEvent::Modified event to a queue only in case asset was marked as changed.

Testing

Did you test these changes? If so, how?
- No unit tests were added, change is pretty straightforward.
- Test project: https://github.com/MatrixDev/bevy-feature-16751-test.
  - With change: ~100 fps.
  - Without change: ~15 fps.
Are there any parts that need more testing?
- I don't really see how this can break anything or add a measurable overhead.
- AssetEvent::Modified will now be sent after the asset was modified instead of before. It should not affect anything but still worth noting.
How can other people (reviewers) test your changes? Is there anything specific they need to know?
- Have a big amount of entities that constantly update their materials.
- Properties of those materials should be animated in a stepped maned (like changing color every 0.1 seconds).
- Update material only if value has actually changed:

if material.base_color != new_color {
    material.base_color = new_color;
}

If relevant, what platforms did you test these changes on, and are there any important ones you can't test?
- tested on macos (Mackbook M1)
- not a platform-specific issue

PS: This is my first PR, so please don’t judge.

github-actions · 2026-01-09T17:40:05Z

Welcome, new contributor!

Please make sure you've read our contributing guide and we look forward to reviewing your pull request shortly ✨

…to rendering performance costs (bevyengine#16751)

MatrixDev · 2026-01-09T19:51:35Z

A little more details on the test app. As mentioned in the original issue, I've noticed this slowdown in the debug mode and above values are also for the debug mode. But it doesn't take much to have similar slowdown also in the release mode as well.

Debug mode, 441 object (21x21 grid):

with change: ~100 fps
without change: ~15 fps

Release mode, 1 681 object (41x41 grid):

with change: ~120 fps
without change: ~30 fps

This PR doesn't actually fix the slowdown itself (which is caused by a huge amount of material extractions) but only provides some tools to avoid it.

HugoPeters1024

Thanks for your contribution! I think this change makes sense and your implementation is quite reasonable. I did want to point out 2 things though.

You mentioned that you weren't really sure how to add a test for this, but you then actually go on to describe a pretty good way to test this new functionality! Perhaps you can check out how_to_test_systems.rs to get some inspiration on how to write a test for it. While the implementation is quite simple and almost certainly correct, I think a test is still very valuable to make sure things keep working going forward.

Secondly, your approached reminds me a lot of ResMut, which is good because it serves a similar goal. But I did notice that ResMut relies on the Ticks struct for change detection. From what I understand this tracks the last tick that a certain change happened. Because this appears to be a general change detection mechanism, I wonder if it makes sense to re-use it here. That said, I'm not very familiar with that part of the codebase but it's something to consider.

Thanks again for the contribution and congratulations on very first Bevy PR! I hope it's the first of many :)

MatrixDev · 2026-01-10T15:04:50Z

@HugoPeters1024 , thanks for your comments.

Test

I've added a test for the AssetMut change detection.

Potentially I can also move slightly modified test from my repo to Stress Tests folder.
Will need to add "enable change detection" toggle in such case to better see if there is some effect.

Ticks approach

I also looked at change detection for Mut/ResMut for inspiration but they work a little different. Both Component (Mut) and Resource (ResMut) are managed directly by the World object. Mut and ResMut are SystemParams, so they have access to Ticks.

Asset on the other hand is just a part of the Assets resource. It cannot be used as a SystemParam and doesn't have access to Ticks. Also, unlike Mut/ResMut, they rely on the events like AssetEvent::Modified for other code to work.

Maybe there is a way to make them use Ticks and DetectChangesMut but it is not straightforward and will require much bigger refactoring.

crates/bevy_asset/src/assets.rs

MatrixDev · 2026-01-11T21:47:41Z

@HugoPeters1024, thanks for your review.

test is updated to use Resource instead of the arc/atomics. also field names should be more clear now. it did simplify the test a little.
not really sure about scheduling and system dependencies because messages become available in the MessageReader only when Messages::update is called. both Update and PostUpdate don't work here (messages are avaialble only on the second frame) but Last did, so I settled with it. I'm not really convinced it is a good solution, ideally we should add dependencies to message_update_system but I don't really know how to do it here. double app.update() was much simpler and more resistant to scheduler changes while still being true to the test.

HugoPeters1024

Thanks for incorporating the feedback, this LGTM! (Make sure you check out any CI failures though)

MatrixDev · 2026-01-13T16:31:43Z

@HugoPeters1024, thanks for your approval. I've also resolved all of the CI errors that you've mentioned.

what should be my next steps for this issue?

HugoPeters1024 · 2026-01-13T17:05:09Z

what should be my next steps for this issue?

Now we wait for a maintainer to label this as "ready for final review", if all is well it will be merged when deemed convenient. When there are breaking changes like this it might happen that the PR is deferred to the next release, as we are currently quite close to v0.18.

So for now just keep an eye out if there is any followup review :). If nothing happens in the next week or 2 feel free to reach out on the discord.

andriyDev

I don't feel strongly about this change. Users can work around it fairly easily by just using get() and then upgrading to get_mut if they realize they need to mutate. Yes there's a perf cost, but it's probably not that big a deal.

Practically with assets-as-entities we'll get this exact thing for free, since we'll likely just use change detection like normal components. So I think it's fine to do this early.

andriyDev · 2026-01-15T01:31:50Z

crates/bevy_asset/src/assets.rs

+            asset_modified_counter: u32,
+        }
+
+        let mut app = App::new();


Please use the create_app function from crates/bevy_asset/src/lib.rs. It prevents you from doing bad stuff accidentally, and hides away some setup code we don't care about.

andriyDev · 2026-01-15T01:35:10Z

crates/bevy_asset/src/assets.rs

+        app.add_systems(
+            Update,
+            move |mut assets: ResMut<Assets<TestAsset>>, state: Res<TestState>| {
+                let mut asset = assets.get_mut(my_asset_id).unwrap();
+
+                if asset.value != state.asset_target_value {
+                    asset.value = state.asset_target_value;
+                }
+            },
+        );
+        app.add_systems(
+            Last,
+            move |mut reader: MessageReader<AssetEvent<TestAsset>>,
+                  mut state: ResMut<TestState>| {
+                for event in reader.read() {
+                    if event.is_modified(my_asset_id) {
+                        state.asset_modified_counter += 1;
+                    }
+                }
+            },
+        );


These don't need to be systems for this test. Can we just use app.world_mut().resource::<Assets<TestAsset>>() in the loop and access the asset that way? And then we can just access the Messages<AssetEvent<TestAsset>> resource to drain out any events. I prefer to keep systems out of these tests for simplicity (for example we can delete the TestState resource I think).

done. it is really easier to track what happens without the systems. thanks for suggestion.

andriyDev · 2026-01-15T01:37:14Z

examples/gltf/edit_material_on_gltf.rs

                // is best to cache the handle somewhere, as having multiple materials
                // that are identical is expensive
-                let mut new_material = material.clone();
+                let mut new_material = material.into_inner_untracked().clone();


This doesn't seem like we should be using into_inner_untracked here. It seems more like we should stop using get_mut at all for this material.

you're correct, it looks like get_mut here was redundant.

MatrixDev · 2026-01-15T10:16:54Z

@andriyDev, thanks for your review. All your comments hopefully should be resolved now.

I agree that "assets-as-entities" feature will solve this problem by default and will make this change obsolete but I don't know what is current progress on it nor when it is expected to be released.

I also agree that users have ability to achieve this functionality by themself (I even wrote how to do it in the original issue) but it is a somewhat cumbersome, not user-friendly and has some overhead (even if very minimal). Personally, on my projects, I use exactly the same implementation, just calling get_mut_untracked to get initial asset and get_mut in Drop and it works fine for my usecase.

Anyways, I'd be happy if this can be a part of the assets API itself even if just as a temporary fix until "assets-as-entities" arrive.

Add tools to avoid unnecessary AssetEvent::Modified events that lead …

031302f

…to rendering performance costs (bevyengine#16751)

MatrixDev force-pushed the feature/16751-assets-get_mut-change-tracking branch from f5c7534 to 031302f Compare January 9, 2026 19:25

HugoPeters1024 suggested changes Jan 10, 2026

View reviewed changes

added unit tests for AssetMut change detection

412b115

MatrixDev force-pushed the feature/16751-assets-get_mut-change-tracking branch from 4a39b8f to 412b115 Compare January 10, 2026 15:11

MatrixDev requested a review from HugoPeters1024 January 10, 2026 15:17

HugoPeters1024 reviewed Jan 11, 2026

View reviewed changes

crates/bevy_asset/src/assets.rs Show resolved Hide resolved

updated unit test to use resources instead of arc/atomics

3b95149

MatrixDev requested a review from HugoPeters1024 January 13, 2026 10:29

HugoPeters1024 approved these changes Jan 13, 2026

View reviewed changes

MatrixDev added 5 commits January 13, 2026 16:45

Merge branch 'main' into feature/16751-assets-get_mut-change-tracking

1e900ae

fixed compilation errors

77d13a1

fixed more compilation errors

4b19d64

fixed more compilation errors

27cee2c

fixed formatting and lint errors

9ea57ee

andriyDev suggested changes Jan 15, 2026

View reviewed changes

fixed comments

6c4eed9

MatrixDev force-pushed the feature/16751-assets-get_mut-change-tracking branch from 8ce97a9 to 6c4eed9 Compare January 15, 2026 10:01

MatrixDev requested a review from andriyDev January 15, 2026 10:18

andriyDev approved these changes Jan 17, 2026

View reviewed changes

Uh oh!

Add tools to avoid unnecessary AssetEvent::Modified events that lead to rendering performance costs (#16751) #22460

Are you sure you want to change the base?

Add tools to avoid unnecessary AssetEvent::Modified events that lead to rendering performance costs (#16751) #22460

Conversation

MatrixDev commented Jan 9, 2026

Objective

Solution

Testing

Uh oh!

github-actions bot commented Jan 9, 2026

Uh oh!

MatrixDev commented Jan 9, 2026

Uh oh!

HugoPeters1024 left a comment

Choose a reason for hiding this comment

Uh oh!

MatrixDev commented Jan 10, 2026

Test

Ticks approach

Uh oh!

Uh oh!

MatrixDev commented Jan 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

HugoPeters1024 left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

MatrixDev commented Jan 13, 2026

Uh oh!

HugoPeters1024 commented Jan 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

andriyDev left a comment

Choose a reason for hiding this comment

Uh oh!

andriyDev Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

MatrixDev Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

andriyDev Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

MatrixDev Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

andriyDev Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

MatrixDev Jan 15, 2026

Choose a reason for hiding this comment

Uh oh!

MatrixDev commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Add tools to avoid unnecessary `AssetEvent::Modified` events that lead to rendering performance costs (#16751) #22460

Add tools to avoid unnecessary `AssetEvent::Modified` events that lead to rendering performance costs (#16751) #22460

MatrixDev commented Jan 11, 2026 •

edited

Loading

HugoPeters1024 left a comment •

edited

Loading

HugoPeters1024 commented Jan 13, 2026 •

edited

Loading

MatrixDev commented Jan 15, 2026 •

edited

Loading