Skip to content

Implement Events::extend with Vec::extend #2149

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

Closed

Conversation

NathanSWard
Copy link
Contributor

Problem:

  • Events::extend manually called Vec::push for each element of the
    provided iterator.
  • This can potentially be an expensive.

Solution:

  • Use Vec::extend over a Vec::push for loop.

Fixes: #2146

Blocked by: #2145

@NathanSWard
Copy link
Contributor Author

This is based on #2145 so the only change this PR adds is the final commit 67afc54

@alice-i-cecile alice-i-cecile added C-Code-Quality A section of code that is hard to understand or change A-ECS Entities, components, systems, and events C-Performance A change motivated by improving speed, memory usage or compile times labels May 12, 2021
@alice-i-cecile
Copy link
Member

So, the main question that I have here is should we be benchmarking this, and if so, how?

Creating a proper events benchmark would be nice for #2073 as well, if we can design something that works for both.

@NathanSWard NathanSWard force-pushed the nward/events-vec-extend branch from 67afc54 to ffdd605 Compare May 12, 2021 15:25
Problem:
- Events::extend manually called Vec::push for each element of the
  provided iterator.
- This can potentially be an expensive.

Solution:
- Use Vec::extend over a Vec::push for loop.
@NathanSWard NathanSWard force-pushed the nward/events-vec-extend branch from ffdd605 to 2638ec5 Compare May 12, 2021 19:02
@NathanSWard NathanSWard mentioned this pull request May 17, 2021
@NathanSWard
Copy link
Contributor Author

Closing this in favor of a change that's coalescing this and some Event bug fixes.

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-Code-Quality A section of code that is hard to understand or change C-Performance A change motivated by improving speed, memory usage or compile times
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Events should use .extend internally
2 participants