Skip to content

Commit

Permalink
Remove a ptr-to-int cast in CommandQueue::apply (#10475)
Browse files Browse the repository at this point in the history
# Objective

- `CommandQueue::apply` calculates the address of the end of the
internal buffer as a `usize` rather than as a pointer, requiring two
casts of `cursor` to `usize`. Casting pointers to integers is generally
discouraged and may also prevent optimizations. It's also unnecessary
here.

## Solution

- Calculate the end address as a pointer rather than a `usize`.

Small note:

A trivial translation of the old code to use pointers would have
computed `end_addr` as `cursor.add(self.bytes.len())`, which is not
wrong but is an additional `unsafe` operation that also needs to be
properly documented and proven correct. However this operation is
already implemented in the form of the safe `as_mut_ptr_range`, so I
just used that.
  • Loading branch information
SkiFire13 authored and cart committed Nov 30, 2023
1 parent 79da6fd commit f5ccf68
Showing 1 changed file with 5 additions and 5 deletions.
10 changes: 5 additions & 5 deletions crates/bevy_ecs/src/system/commands/command_queue.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,18 +97,18 @@ impl CommandQueue {
// flush the previously queued entities
world.flush();

// Pointer that will iterate over the entries of the buffer.
let mut cursor = self.bytes.as_mut_ptr();
// The range of pointers of the filled portion of `self.bytes`.
let bytes_range = self.bytes.as_mut_ptr_range();

// The address of the end of the buffer.
let end_addr = cursor as usize + self.bytes.len();
// Pointer that will iterate over the entries of the buffer.
let mut cursor = bytes_range.start;

// Reset the buffer, so it can be reused after this function ends.
// In the loop below, ownership of each command will be transferred into user code.
// SAFETY: `set_len(0)` is always valid.
unsafe { self.bytes.set_len(0) };

while (cursor as usize) < end_addr {
while cursor < bytes_range.end {
// SAFETY: The cursor is either at the start of the buffer, or just after the previous command.
// Since we know that the cursor is in bounds, it must point to the start of a new command.
let meta = unsafe { cursor.cast::<CommandMeta>().read_unaligned() };
Expand Down

0 comments on commit f5ccf68

Please sign in to comment.