Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Merged by Bors] - Remove useless access to archetype in UnsafeWorldCell::fetch_table #7665

Closed
wants to merge 1 commit into from

Conversation

SkiFire13
Copy link
Contributor

@SkiFire13 SkiFire13 commented Feb 13, 2023

Objective

  • [Merged by Bors] - refactor: move internals from entity_ref to World, add SAFETY comments #6402 changed World::fetch_table (now UnsafeWorldCell::fetch_table) to access the archetype in order to get the table_id and table_row of the entity involved. However this is useless since those were already present in the EntityLocation
  • Moreover it's useless for UnsafeWorldCell::fetch_table to return the TableRow too, since the caller must already have access to the EntityLocation which contains the TableRow.
  • The result is that UnsafeWorldCell::fetch_table now only does 2 memory fetches instead of 4.

Solution

@james7132 james7132 added A-ECS Entities, components, systems, and events C-Performance A change motivated by improving speed, memory usage or compile times C-Bug An unexpected or incorrect behavior labels Feb 14, 2023
@BoxyUwU BoxyUwU added this to the 0.10 milestone Feb 14, 2023
@BoxyUwU BoxyUwU added the S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it label Feb 14, 2023
@james7132
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Feb 15, 2023
…7665)

# Objective

- #6402 changed `World::fetch_table` (now `UnsafeWorldCell::fetch_table`) to access the archetype in order to get the `table_id` and `table_row` of the entity involved. However this is useless since those were already present in the `EntityLocation`
- Moreover it's useless for `UnsafeWorldCell::fetch_table` to return the `TableRow` too, since the caller must already have access to the `EntityLocation` which contains the `TableRow`.
- The result is that `UnsafeWorldCell::fetch_table` now only does 2 memory fetches instead of 4.

## Solution

- Revert the changes to the implementation of `UnsafeWorldCell::fetch_table` made in #6402
@bors bors bot changed the title Remove useless access to archetype in UnsafeWorldCell::fetch_table [Merged by Bors] - Remove useless access to archetype in UnsafeWorldCell::fetch_table Feb 15, 2023
@bors bors bot closed this Feb 15, 2023
@SkiFire13 SkiFire13 deleted the fix-fetch-table branch February 15, 2023 09:21
myreprise1 pushed a commit to myreprise1/bevy that referenced this pull request Feb 15, 2023
…evyengine#7665)

# Objective

- bevyengine#6402 changed `World::fetch_table` (now `UnsafeWorldCell::fetch_table`) to access the archetype in order to get the `table_id` and `table_row` of the entity involved. However this is useless since those were already present in the `EntityLocation`
- Moreover it's useless for `UnsafeWorldCell::fetch_table` to return the `TableRow` too, since the caller must already have access to the `EntityLocation` which contains the `TableRow`.
- The result is that `UnsafeWorldCell::fetch_table` now only does 2 memory fetches instead of 4.

## Solution

- Revert the changes to the implementation of `UnsafeWorldCell::fetch_table` made in bevyengine#6402
myreprise1 pushed a commit to myreprise1/bevy that referenced this pull request Feb 15, 2023
…evyengine#7665)

# Objective

- bevyengine#6402 changed `World::fetch_table` (now `UnsafeWorldCell::fetch_table`) to access the archetype in order to get the `table_id` and `table_row` of the entity involved. However this is useless since those were already present in the `EntityLocation`
- Moreover it's useless for `UnsafeWorldCell::fetch_table` to return the `TableRow` too, since the caller must already have access to the `EntityLocation` which contains the `TableRow`.
- The result is that `UnsafeWorldCell::fetch_table` now only does 2 memory fetches instead of 4.

## Solution

- Revert the changes to the implementation of `UnsafeWorldCell::fetch_table` made in bevyengine#6402
myreprise1 pushed a commit to myreprise1/bevy that referenced this pull request Feb 15, 2023
…evyengine#7665)

# Objective

- bevyengine#6402 changed `World::fetch_table` (now `UnsafeWorldCell::fetch_table`) to access the archetype in order to get the `table_id` and `table_row` of the entity involved. However this is useless since those were already present in the `EntityLocation`
- Moreover it's useless for `UnsafeWorldCell::fetch_table` to return the `TableRow` too, since the caller must already have access to the `EntityLocation` which contains the `TableRow`.
- The result is that `UnsafeWorldCell::fetch_table` now only does 2 memory fetches instead of 4.

## Solution

- Revert the changes to the implementation of `UnsafeWorldCell::fetch_table` made in bevyengine#6402
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-ECS Entities, components, systems, and events C-Bug An unexpected or incorrect behavior C-Performance A change motivated by improving speed, memory usage or compile times S-Ready-For-Final-Review This PR has been approved by the community. It's ready for a maintainer to consider merging it
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants