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

Implement queries that add/remove components #337

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Ralith
Copy link
Owner

@Ralith Ralith commented Sep 17, 2023

Fixes #334.

TODO:

  • Work out why this machinery is 10x slower in the iterate_mut_100k benchmark than query_mut in master. Nothing's jumping out at me in the disassembly; input would be welcome.
  • Move side effects into a guard object exposed via a new API (query_with_effect?) to fix unsoundness
  • Update macros

@adamreichold
Copy link
Collaborator

Work out why this machinery is 10x slower in the iterate_mut_100k benchmark than query_mut in master. Nothing's jumping out at me in the disassembly; input would be welcome.

This is pure speculation, but could this be cache effects of the increased working set due to the effect table?

@Ralith
Copy link
Owner Author

Ralith commented Sep 19, 2023

This is pure speculation, but could this be cache effects of the increased working set due to the effect table?

Surprisingly, that slowdown is for in-place/effect-free queries, for which effects are ZSTs and the effect table should allocate no memory.

@a1phyr
Copy link
Contributor

a1phyr commented Sep 21, 2023

These benchmarks do very few things within the iteration, which sometimes allow LLVM to optimize the loop far better... or not !
This seems to be very fragile. Adding a continue in the iteration loop or a black_box with the query values can make or break it.

For example, on #341, adding black_box around vel.0 in the benchmark code canceled the optimization.

@Ralith
Copy link
Owner Author

Ralith commented Sep 22, 2023

A 10-fold difference is a lot to hand-wave, even so. Is that not sustained with black_box involved? Maybe a more computationally intensive benchmark would be interesting?

@Ralith
Copy link
Owner Author

Ralith commented Feb 4, 2024

This would be useful for #366, which presently must make two passes and perform a lot of redundant lookups to add/remove components to/from entities satisfying a query.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Queries that add/remove components
3 participants