data_store: reduce the cost of creating prerequisite objects #5523
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
oliver-sanders
commented
Jun 14, 2023
Comment on lines
-247
to
-248
| prereq_buf.conditions.extend(conds) | ||
| prereq_buf.cycle_points.extend( |
There was a problem hiding this comment.
@dwsutherland Is there a special why these PbPrerequisite fields were initialised, then extended rather than being declared directly? It seems to work this way but I know PB ain't Python so can appear to have strange interactions so want to make sure I'm not breaking anything deliberate.
There was a problem hiding this comment.
It may have been a work around to an issue that has been solved:
protocolbuffers/protobuf#10063
Or just a quirk of how I did things (perhaps thinking sub message fields couldn't be in the init args)..
If it works, and results are as expected .. Then should be fine.
hjoliver
approved these changes
Jun 19, 2023
dwsutherland
approved these changes
Jun 19, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Attempt to reduce the overheads of creating prerequisite objects in the data store.
At Cylc7 we only created prerequisite objects for tasks in the pool. At Cylc 8 we create pbPrerequsite objects for each task in the n=1 window by default, expandable out to large n numbers by request.
These prereq objects are costly to generate. The bulk of that is due to a high number of
detokenisecalls. Since there is no real function to having the tokens objects around for data store prerequsites which don't technically exist yet, we may as well go back to string formatting here.Also make some minor optimisations on route and fix what looks like a bug in the expression for non-conditional prerequisites.
Check List
CONTRIBUTING.mdand added my name as a Code Contributor.setup.cfg(andconda-environment.ymlif present).?.?.xbranch.