Speedup resource lookup in Res and ResMut

[Trashtalk217]

Objective

See #23039. Resources as components introduced an extra entity lookup for each resource access.

Solution

Store the resource entity in the system param state of Res and ResMut. This is possible because resource entities are stable, so the state doesn't have to be updated.

Benchmarking

I quickly ran

cargo run --release --example bevymark -- --waves 60 --per-wave 500 --benchmark --mode mesh2d

and got a frametime of 33.4 ms (on main 34.6 ms). This is a minor reduction, but it might help. Others should double-check that this isn't a fluke, because my benchmarking isn't the most airtight.

Testing

All tests pass.

[github-actions]

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-23174

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

[github-actions]

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-23174

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

[github-actions]

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-23174

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

[cart]

The redundant work in validate_param, which is done immediately before get_param never felt great to me. This isn't a problem exclusive to Res/ResMut, but it is certainly present.

I think doing validation as part of get_param is more defensible from a performance perspective.

I think it is worth doing some benchmarks where we skip validation (as-in, across all SystemParams), just to see what price we're paying here.

[SkiFire13]

This is possible because resource entities are stable, so the state doesn't have to be updated.

This is not true:

#[test]
fn resource_component_stable() {
    #[derive(Resource)]
    struct Foo;

    let mut world = World::new();

    let component_id = world.register_component::<Foo>();

    world.insert_resource(Foo);

    let res_id1 = world.resource_entities().get(component_id).copied();

    world.remove_resource_by_id(component_id);
    world.insert_resource(Foo);

    let res_id2 = world.resource_entities().get(component_id).copied();

    assert_eq!(res_id1, res_id2); // Currently fails
}

If resource entities are meant to be stable then:

• we should have tests that check this is the case
• there should be documentation/comments in all places that might break this invariant to ensure future changes don't mistakingly break it
• I would also argue that ResourceEntities should not expose a remove method

[hymm]

This is a decent perf improvement. I get about 0.4ms back of the 1.4ms I lost.

Though we probably need to figure out how to actually make the entities stable without any loopholes before we merge this.

[alice-i-cecile]

Though we probably need to figure out how to actually make the entities stable without any loopholes before we merge this.

Or alternatively, have a way to invalidate all of these caches that does not regress perf.