-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
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
Compile multiple run conditions into one #7580
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't love adding more complexity here, but the aim is valuable. Great comments!
Requests:
- Add a matching
Either
combinator (to avoid manglingOr
from queries). - Add doc tests to both
Either
andAnd
. - Add links to both
Either
andAnd
from theCondition
docs.not
from [Merged by Bors] - Add condition negation #7559 would be nice to mention there too.
I'm assuming you want this in order to satisfy #7584 -- however the I am working on another PR that would satify #7584, though. |
Okay great, I'm fine to cut 1 from this PR :) |
I'm taking this off of the milestone if that's okay, since 0.10 is so close and this is just a nice-to-have optimization. |
Closing this since it's out of date, and I don't consider this to be high enough priority to get it working again. |
Objective
When multiple run conditions are attached to a system, they should be compiled together as one function.
Solution
Add a variant of
System{Set}Config
that preserves the types of run conditions and combines them with any newly-added ones. This allows the compiler to inline them into one function.This optimization is performed opportunistically -- run conditions that have already been boxed and type erased are not combined, as there is no benefit to doing so. This means that structs that perform type erasure such as
SystemConfig
present a barrier for this optimization. Resolving this should be possible in the future, but it would require a much higher-impact change that involves preserving the type data for allSystemConfig
instances.