Fix double indirection when applying command queues #11822
Conversation
JoJoJet
added
A-ECS
alice-i-cecile
added
the
C-Code-Quality
| @@ -157,7 +157,7 @@ impl CommandQueue { | |||
| // SAFETY: The data underneath the cursor must correspond to the type erased in metadata, | |||
| // since they were stored next to each other by `.push()`. | |||
| // For ZSTs, the type doesn't matter as long as the pointer is non-null. | |||
| let size = unsafe { (meta.consume_command_and_get_size)(cmd, &mut world) }; | |||
| let size = unsafe { (meta.consume_command_and_get_size)(cmd, world.as_deref_mut()) }; | |||
There was a problem hiding this comment.
Great catch on this, though I wonder if the compiler will lift the branch out of the loop. Either way, I don't think this is the bottleneck when it comes to command application given what commands typically entail, so probably not a blocker.
There was a problem hiding this comment.
This seems to remove two mov instructions from outside of the hot loop: james7132/bevy_asm_tests@main...remove-double-deref#diff-8e36b64d5c51682bb463a51ec6e670b2ec99545945bc4753f4e77ef274c5faa0L27. This likely has a stronger impact inside of the commands themselves.
There was a problem hiding this comment.
I don't think this introduces a branch -- as_deref_mut() should be a no-op here
There was a problem hiding this comment.
I bet the moves getting removed are due to the fact that the compiler no longer has to deal with the possibility of a command swapping the world from None to Some or vice versa.
alice-i-cecile
added
the
S-Ready-For-Final-Review
Objective
When applying a command, we currently use double indirection for the world reference
&mut Option<&mut World>. Since this is used across afnpointer boundary, this can't get optimized away.Solution
Reborrow the world reference and pass
Option<&mut World>instead.