Queued component registration by ElliottjPierce · Pull Request #18173 · bevyengine/bevy

ElliottjPierce · 2025-03-06T02:37:53Z

Objective

This is an alternative to #17871 and #17701 for tracking issue #18155. This thanks to @maniwani for help with this design.

The goal is to enable component ids to be reserved from multiple threads concurrently and with only &World. This contributes to assets as entities, read-only query and system parameter initialization, etc.

What's wrong with #17871 ?

In #17871, I used my proposed staging utilities to allow fully registering components from any thread concurrently with only &Components. However, if we want to pursue components as entities (which is desirable for a great many reasons. See here on discord), this staging isn't going to work. After all, if registering a component requires spawning an entity, and spawning an entity requires &mut World, it is impossible to register a component fully with only &World.

Solution

But what if we don't have to register it all the way? What if it's enough to just know the ComponentId it will have once it is registered and to queue it to be registered at a later time? Spoiler alert: That is all we need for these features.

Here's the basic design:

Queue a registration:

Check if it has already been registered.
Check if it has already been queued.
Reserve a ComponentId.
Queue the registration at that id.

Direct (normal) registration:

Check if this registration has been queued.
If it has, use the queued registration instead.
Otherwise, proceed like normal.

Appllying the queue:

Pop queued items off one by one.
Register them directly.

One other change:

The whole point of this design over #17871 is to facilitate coupling component registration with the World. To ensure that this would fully work with that, I went ahead and moved the ComponentId generator onto the world itself. That stemmed a couple of minor organizational changes (see migration guide). As we do components as entities, we will replace this generator with Entities, which lives on World too. Doing this move early let me verify the design and will reduce migration headaches in the future. If components as entities is as close as I think it is, I don't think splitting this up into different PRs is worth it. If it is not as close as it is, it might make sense to still do #17871 in the meantime (see the risks section). I'll leave it up to y'all what we end up doing though.

Risks and Testing

The biggest downside of this compared to #17871 is that now we have to deal with correct but invalid ComponentIds. They are invalid because the component still isn't registered, but they are correct because, once registered, the component will have exactly that id.

However, the only time this becomes a problem is if some code violates safety rules by queuing a registration and using the returned id as if it was valid. As this is a new feature though, nothing in Bevy does this, so no new tests were added for it. When we do use it, I left detailed docs to help mitigate issues here, and we can test those usages. Ex: we will want some tests on using queries initialized from queued registrations.

Migration Guide

Component registration can now be queued with only &World. To facilitate this, a few APIs needed to be moved around.

The following functions have moved from Components to ComponentsRegistrator:

register_component
register_component_with_descriptor
register_resource_with_descriptor
register_non_send
register_resource
register_required_components_manual

Accordingly, functions in Bundle and Component now take ComponentsRegistrator instead of Components.
You can obtain ComponentsRegistrator from the new World::components_registrator.
You can obtain ComponentsQueuedRegistrator from the new World::components_queue, and use it to stage component registration if desired.

Open Question

Can we verify that it is enough to queue registration with &World? I don't think it would be too difficult to package this up into a Arc<MyComponentsManager> type thing if we need to, but keeping this on &World certainly simplifies things. If we do need the Arc, we'll need to look into partitioning Entities for components as entities, so we can keep most of the allocation fast on World and only keep a smaller partition in the Arc. I'd love an SME on assets as entities to shed some light on this.

…liottjPierce/bevy into queued-component-registration

maniwani · 2025-03-06T04:25:08Z

I've looked over this and it seems good. atm I can't think of anything to add on top of what we discussed on Discord.

The general strategy of "claim the ID now and its registration will be completed before we actually try to use it" isn't too different from how systems can specify commands to insert components on entities that don't exist yet, so I think it'll carry over to assets as well.

Thanks for the really thorough yet concise description and comments!

alice-i-cecile · 2025-03-06T05:33:37Z

I'm in favor of this direction. Marking as Blessed, but I don't have the energy right this moment for a review. Pester me if I don't get to it within a week :)

crates/bevy_ecs/src/world/mod.rs

crates/bevy_ecs/src/component.rs

andriyDev · 2025-03-06T07:36:37Z

crates/bevy_ecs/src/component.rs

+        while let Some((id, mut registrator)) = {
+            let queued = self
+                .queued
+                .get_mut()
+                .unwrap_or_else(PoisonError::into_inner);
+            queued.components.keys().next().copied().map(|type_id| {
+                // SAFETY: the id just came from a valid iterator.
+                unsafe { queued.components.remove(&type_id).debug_checked_unwrap() }
+            })
+        } {
+            // SAFETY: we own this value and it is being dropped, and the id came from the unique queue.
+            unsafe {
+                registrator.register(self, id);
+            }
+        }


Wow this is a very complicated drain implementation :P Is the intent here to limit how long the lock is taken for? I wonder if this is worth it, since we're now locking multiple times rather than just locking once, registering all the components, and moving on.

In other words, should we just use .drain() instead? As long as we explicitly drop the lock after this loop, we should be good to go (at the minor cost of limiting how quickly components can be queued).

Good thoughts. I should add a comment to make this more clear.

We need to go one at a time for non dynamic registration. The dynamic registration is completely anonymous, so we can just drain it. (We actually need to use mem::take to preserve lifetimes. This could be side-stepped by moving around who owns QueuedComponents, but I don't think it's worth it.)

Anyway, for non-dynamic registration, it has a specific label attached to it, currently the TypeId. This can be a problem for recursive registration. Because once we do the drain (or in this case, the mem::take), that leaves Components with missing information.

For example, say ComponentA and ComponentB are queued, and ComponentA requires ComponentB. If we drain directly, and ComponentA was the first to be registered, then, when ComponentA registers ComponentB in Component::register_required_components, Components will not know that ComponentB was queued (since it will have been drained from the queue.) If that happened, Components would assign a new ComponentId to ComponentB which would be different than the id it was assigned in the queue. Then, when the drain iterator gets to ComponentB, it would be unsafely registering ComponentB, which is already registered.

That could produce some crazy undefined behavior. So, we have to pop the queue exactly one at a time, otherwise Components is left with incomplete or missing information when registering the queue.

Ahhhh this was a very tricky issue. Thank you for explaining and the accompanying comment!

crates/bevy_ecs/src/component.rs

andriyDev · 2025-03-06T08:21:13Z

For my context: for components-as-entities, the idea is that you would reserve an Entity (the same way commands does today), but that entity would not be initialized until we apply the queue here?

@andriyDev

Thanks for the idea @andriyDev

ElliottjPierce · 2025-03-06T15:27:13Z

For my context: for components-as-entities, the idea is that you would reserve an Entity (the same way commands does today), but that entity would not be initialized until we apply the queue here?

AFAIK, that is correct. Of course, if we have &mut World anyway, we can just spawn and initialize the entity directly, but we'll cross that bridge when we get there.

The goal of this PR isn't necessarily to advance components as entities (though it technically does). The goal is be able to know the id of a component/resource before it is registered and to facilitate that without blocking components as entities.

Prevented allocation when dynamic_registrations is empty.

Co-Authored-By: andriyDev <andriydzikh@gmail.com>

chescock

Oh, yay, this looks much simpler!

I left a few initial thoughts, but I've only skimmed the code so I may just be misunderstanding things that should be obvious. I can try to do a more complete review tomorrow if it doesn't get merged first.

chescock · 2025-03-06T18:03:25Z

crates/bevy_ecs/src/component.rs

+}
+
+/// A [`Components`] wrapper that enables additional features, like registration.
+pub struct ComponentsRegistrator<'w> {


Is this only necessary in order to borrow components and component_ids simultaneously? Would it be possible to put component_ids inside Components instead of World, and then just use &mut Components and &Components again? I think that would remove most of the changes outside of World and Components.

Going a step farther, it looks like we only ever call ids.next() immediately before queued.write(). Should we just make next be an ordinary usize inside QueuedComponents?

All great points, and I'm happy to do that if that's what is decided. I mentioned this a little in my description, but the reason I did this is that it prepares us better for components as entities. For that, we will need access to &Entities from the world during registration. By keeping the generator on the world, it will save some headache latter, and it helps enforce that that coupling continues to be available until we do components as entities. (Which I feel is getting pretty close).

? Would it be possible to put component_ids inside Components instead of World, and then just use &mut Components and &Components again

I'd prefer this!

Going a step farther, it looks like we only ever call ids.next() immediately before queued.write(). Should we just make next be an ordinary usize inside QueuedComponents?

Yes please to this too :) Anything we can do to simplify these internals and guide authors into the correct patterns is appreciated here.

@alice-i-cecile 100% not opposed to this, but I'm not sure my reply was visible when you wrote this. Is this still the direction you'd like to take? (I promise I'm not trying to be difficult. Just don't want to have to undo the undo.)

Hmm. Okay fine, we can leave it as is for now :)

Sounds good!

(Although I wonder whether we'll want to put the Entity allocator behind an Arc anyway, so that we don't have to manage references to multiple structs like this for each thing that we everything-as-entities.)

(Although I wonder whether we'll want to put the Entity allocator behind an Arc anyway, so that we don't have to manage references to multiple structs like this for each thing that we everything-as-entities.)

If I understand components as entities correctly, we'll be putting the data of ComponentsInfo onto the entity itself. That lest us, for example, make requirements run on relations. If that understanding is correct, registration will need &mut Storages anyway. But, if, that's not the case, and we would put the entity allocator in an Arc anyway, then I agree.

Hmm. Okay fine, we can leave it as is for now :)

Awesome! Do let me know if you change your mind. There's good reasons to go either way.

If I understand components as entities correctly, we'll be putting the data of ComponentsInfo onto the entity itself. That lest us, for example, make requirements run on relations.

For safety reasons, there needs to be some component info that user code cannot touch. Like, we don't want people swapping the drop functions of components A and B.

So I was simply thinking to duplicate the information onto the components' entities for user code to consume, while still keeping a private copy for the world's internal operations.

For safety reasons, there needs to be some component info that user code cannot touch. Like, we don't want people swapping the drop functions of components A and B.

That would be scary. I would guess though, this would be better enforced with immutable components, if that's possible. Or we could keep the component private and expose a readonly query item that just forwards to &the_component.

So I was simply thinking to duplicate the information onto the components' entities for user code to consume, while still keeping a private copy for the world's internal operations.

Makes sense, but then why not #17871? Unless we want more data to be inserted on the component too.

Makes sense, but then why not #17871?

I don't get how that's related?

The change to get "components as entities" is to just surgically replace ComponentId with Entity. That shouldn't imply any big changes to how the world currently performs its internal operations (i.e. where it stores drop functions, etc.)

Each component will just so happen to have a representative entity that users can do stuff with. The world doesn't need to rely on anything about these entities besides them staying alive. (Maybe it can later, but not a priority.)

crates/bevy_ecs/src/component.rs

chescock · 2025-03-06T18:18:05Z

crates/bevy_ecs/src/component.rs

+
+        // Note:
+        //
+        // This is not just draining the queue. We need to empty the queue without removing the information from `Components`.


This might be more clear if you stored the type-to-id mapping separately from the queue of things to register. Something like component_ids: TypeIdMap<ComponentId> and queued_components: Vec<QueuedRegistration>. For that matter, I think you could share a single Vec for resources, components, and dynamic components (or maybe dynamic components are separate if you store the ComponentDescriptors directly.)

Then you could just drain the Vec, since the ID mapping will still be available. And you could either remove the IDs from the mapping as the components get registered, or clear it all at once at the end of apply_queued_registrations.

This would probably be safe. I think. I did consider that originally, but my concern was that it makes a lot of assumptions about how Components will work. That would probably be fine for today's implementation, but for the sake of future changes, I think keeping Components as the full source of truth is important. Hopefully that makes sense. If anyone feels strongly otherwise, I can try it out. Otherwise, I think this is another think to reconsider after the dust settles.

I think keeping Components as the full source of truth is important.

Sure, but Components already needs to account for the TypeIdMaps in QueuedComponents. That's what this comment is explaining, right?

Hmm, if I think about it in terms of wanting to prevent Components from needing to access QueuedComponents, then maybe what you want is an API on components to push in the TypeIdMap<ComponentId> as reserved ids. If you reserve the IDs with Components first, then you could iterate over the queued registrations at your leisure.

But what you have seems fine to start with! I imagine it will have to be reworked for components-as-entities anyway. I do hope we eventually get to something that's clear enough not to require quite so many lines of comments, though :).

But what you have seems fine to start with! I imagine it will have to be reworked for components-as-entities anyway. I do hope we eventually get to something that's clear enough not to require quite so many lines of comments, though :).

100% Agree. Your comments here are valid, I just think compressing the safety to right here is less complicated than having to think about "What if this registration was queued in this particular way?" though out Components, even though that does make this particular function very non-trivial. But that's just my opinion.

IIUC, this part of Bevy is going to change a lot over 0.17. So I think we can revisit making this faster with Vec and drain later. It's certainly not a regression since queueing is new. If this is merged, I'll make an issue to track cleaning this up a bit once the dust settles.

crates/bevy_ecs/src/component.rs

ElliottjPierce · 2025-03-07T03:57:33Z

I know y'all don't have the time to follow every detail here, so here's a status update:

Reviews:

There are a few ways code quality can be improved, but doing so would make assumptions for how this can evolve in the future. Consensus at the moment seems to be to revisit these after the dust settles. But keep the constructive criticism coming as you see issues! It only makes the code better.

@alice-i-cecile:

The issue is marked as waiting on author. Is there anything I'm missing that I still need to do? (I don't mean to bug you; I know you're busy, but if there's something I should be working on, I'd like to get to it!)

Working assumptions:

The current state of the PR is based on two assumptions. First, I am assuming that registration with components as entities will require access to Storages. If that is not the case, we can simplify even more, but my impression is that it is. Second, I am assuming that we only need to support queueing with &World; we don't need to provide something like Arc<RemoteComponentQueueing>. If that assumption is wrong, I can 100% still add that. But it might be best done in a follow up PR.

I'd love SME clarification on these assumptions before merging. Thanks!

# Objective #18173 allows components to be queued without being fully registered. But much of bevy's debug logging contained `components.get_name(id).unwrap()`. However, this panics when the id is queued. This PR fixes this, allowing names to be retrieved for debugging purposes, etc, even while they're still queued. ## Solution We change `ComponentInfo::descriptor` to be `Arc<ComponentDescriptor>` instead of not arc'd. This lets us pass the descriptor around (as a name or otherwise) as needed. The alternative would require some form of `MappedRwLockReadGuard`, which is unstable, and would be terribly blocking. Putting it in an arc also signifies that it doesn't change, which is a nice signal to users. This does mean there's an extra pointer dereference, but I don't think that's an issue here, as almost all paths that use this are for debugging purposes or one-time set ups. ## Testing Existing tests. ## Migration Guide `Components::get_name` now returns `Option<Cow<'_, str>` instead of `Option<&str>`. This is because it now returns results for queued components. If that behavior is not desired, or you know the component is not queued, you can use `components.get_info().map(ComponentInfo::name)` instead. Similarly, `ScheduleGraph::conflicts_to_string` now returns `impl Iterator<Item = (String, String, Vec<Cow<str>>)>` instead of `impl Iterator<Item = (String, String, Vec<&str>)>`. Because `Cow<str>` derefs to `&str`, most use cases can remain unchanged. --------- Co-authored-by: Chris Russell <8494645+chescock@users.noreply.github.com>

# Objective - #19504 showed a 11x regression in getting component values for unregistered components. This pr should fix that and improve others a little too. - This is some cleanup work from #18173 . ## Solution - Whenever we expect a component value to exist, we only care about fully registered components, not queued to be registered components since, for the value to exist, it must be registered. - So we can use the faster `get_valid_*` instead of `get_*` in a lot of places. - Also found a bug where `valid_*` did not forward to `get_valid_*` properly. That's fixed. ## Testing CI

# Objective - bevyengine#19504 showed a 11x regression in getting component values for unregistered components. This pr should fix that and improve others a little too. - This is some cleanup work from bevyengine#18173 . ## Solution - Whenever we expect a component value to exist, we only care about fully registered components, not queued to be registered components since, for the value to exist, it must be registered. - So we can use the faster `get_valid_*` instead of `get_*` in a lot of places. - Also found a bug where `valid_*` did not forward to `get_valid_*` properly. That's fixed. ## Testing CI

ElliottjPierce and others added 17 commits March 5, 2025 13:18

added id generator.

995307c

separated id generation form registration

63b0931

handled the major errors

3efef9c

fixed all remaining errors and warning

35d9493

small improvements

f786616

outlined queuing

a49dade

rearanged data

95ec1d5

small move

825d573

respect the queued registrations

53f9d96

apply registrations

3aab1a4

fully implemented queueing

6a976ef

fixed docs

1b98032

final touches

2741c77

simplified as_queued

e84684d

Merge branch 'main' into queued-component-registration

cbea213

fixed ci hopefully

afdd3c1

Merge branch 'queued-component-registration' of https://github.com/El…

83513b7

…liottjPierce/bevy into queued-component-registration

maniwani approved these changes Mar 6, 2025

View reviewed changes

This was referenced Mar 6, 2025

Generalized and atomic staging utils #17701

Closed

Staged Components #17871

Closed

alice-i-cecile requested review from alice-i-cecile and andriyDev March 6, 2025 05:32

andriyDev suggested changes Mar 6, 2025

View reviewed changes

Redid queued registration without trait indirection

a329e2d

Thanks for the idea @andriyDev

ElliottjPierce and others added 2 commits March 6, 2025 10:33

comments and reduced allocations

ed4dc99

Prevented allocation when dynamic_registrations is empty.

clarified access to queued

a7523be

Co-Authored-By: andriyDev <andriydzikh@gmail.com>

chescock reviewed Mar 6, 2025

View reviewed changes

alice-i-cecile added S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged and removed S-Needs-Review Needs reviewer attention (from anyone!) to move forward labels Mar 6, 2025

added ways to check registration easily

3c9943f

ElliottjPierce mentioned this pull request Mar 6, 2025

Tracking: Take advantage of ComponentId's being discoverable before registration #18184

Open

5 tasks

fixed docs

dddd24c

chescock reviewed Mar 7, 2025

View reviewed changes

crates/bevy_ecs/src/component.rs Outdated Show resolved Hide resolved

change component storage back to vec

2d5baaa

andriyDev approved these changes Mar 7, 2025

View reviewed changes

alice-i-cecile added S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it and removed S-Waiting-on-Author The author needs to make changes or address concerns before this can be merged labels Mar 7, 2025

chescock approved these changes Mar 7, 2025

View reviewed changes

alice-i-cecile added this pull request to the merge queue Mar 10, 2025

Merged via the queue into bevyengine:main with commit 246ce59 Mar 10, 2025
31 checks passed

This was referenced Mar 12, 2025

Allow readonly queries and system parameters to be initialized with only &World. #18276

Open

Revisit Queued Component Registration code quality after Component as Entities #18278

Open

ElliottjPierce mentioned this pull request Mar 20, 2025

Get names of queued components #18451

Merged

This was referenced Jun 6, 2025

bevy_ecs performance regression in 0.16.0 release (get + insertion) #19504

Open

Only get valid component ids #19510

Merged

ElliottjPierce mentioned this pull request Jan 26, 2026

Make WorldQuery use &World for initialization #22670

Open

Uh oh!

Conversation

ElliottjPierce commented Mar 6, 2025 • edited by alice-i-cecile Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Objective

What's wrong with #17871 ?

Solution

Risks and Testing

Migration Guide

Open Question

Uh oh!

maniwani commented Mar 6, 2025

Uh oh!

alice-i-cecile commented Mar 6, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andriyDev Mar 6, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

andriyDev commented Mar 6, 2025

Uh oh!

ElliottjPierce commented Mar 6, 2025

Uh oh!

chescock left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ElliottjPierce Mar 6, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

ElliottjPierce commented Mar 7, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ElliottjPierce commented Mar 6, 2025 •

edited by alice-i-cecile

Loading

andriyDev Mar 6, 2025 •

edited

Loading

ElliottjPierce Mar 6, 2025 •

edited

Loading