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] - Use a bounded channel in the multithreaded executor #7829

Closed
wants to merge 3 commits into from

Conversation

james7132
Copy link
Member

@james7132 james7132 commented Feb 27, 2023

Objective

This is a follow-up to #7745. An unbounded async_channel occasionally allocates whenever it exceeds the capacity of the current buffer in it's internal linked list. This is avoidable.

This also used to be a bounded channel before stageless, which was introduced in #4919.

Solution

Use a bounded channel to avoid allocations on system completion.

This shouldn't conflict with #7745, as it's impossible for the scheduler to exceed the channel capacity, even if somehow every system completed at the same time.

@james7132 james7132 added A-ECS Entities, components, systems, and events C-Performance A change motivated by improving speed, memory usage or compile times labels Feb 27, 2023
@james7132 james7132 added this to the 0.10 milestone Feb 27, 2023
@github-actions
Copy link
Contributor

Example alien_cake_addict failed to run, please try running it locally and check the result.

@github-actions
Copy link
Contributor

Example alien_cake_addict failed to run, please try running it locally and check the result.

@james7132 james7132 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 27, 2023
@alice-i-cecile
Copy link
Member

bors r+

bors bot pushed a commit that referenced this pull request Feb 27, 2023
# Objective
This is a follow-up to #7745. An unbounded `async_channel`  occasionally allocates whenever it exceeds the capacity of the current buffer in it's internal linked list. This is avoidable.

This also used to be a bounded channel before stageless, which was introduced in #4919.

## Solution
Use a bounded channel to avoid allocations on system completion.

This shouldn't conflict with #7745, as it's impossible for the scheduler to exceed the channel capacity, even if somehow every system completed at the same time.
@bors
Copy link
Contributor

bors bot commented Feb 27, 2023

Timed out.

@james7132
Copy link
Member Author

bors retry

bors bot pushed a commit that referenced this pull request Feb 27, 2023
# Objective
This is a follow-up to #7745. An unbounded `async_channel`  occasionally allocates whenever it exceeds the capacity of the current buffer in it's internal linked list. This is avoidable.

This also used to be a bounded channel before stageless, which was introduced in #4919.

## Solution
Use a bounded channel to avoid allocations on system completion.

This shouldn't conflict with #7745, as it's impossible for the scheduler to exceed the channel capacity, even if somehow every system completed at the same time.
@bors bors bot changed the title Use a bounded channel in the multithreaded executor [Merged by Bors] - Use a bounded channel in the multithreaded executor Feb 28, 2023
@bors bors bot closed this Feb 28, 2023
@james7132 james7132 deleted the bounded-channel branch March 14, 2023 04:13
Shfty pushed a commit to shfty-rust/bevy that referenced this pull request Mar 19, 2023
# Objective
This is a follow-up to bevyengine#7745. An unbounded `async_channel`  occasionally allocates whenever it exceeds the capacity of the current buffer in it's internal linked list. This is avoidable.

This also used to be a bounded channel before stageless, which was introduced in bevyengine#4919.

## Solution
Use a bounded channel to avoid allocations on system completion.

This shouldn't conflict with bevyengine#7745, as it's impossible for the scheduler to exceed the channel capacity, even if somehow every system completed at the same time.
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-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.

4 participants