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

cylc.task_pool: speed up queued db ops #1812

Merged
merged 2 commits into from
Apr 27, 2016

Conversation

benfitzpatrick
Copy link
Contributor

This speeds up the process_queued_db_ops method by around 40% (for the modified busy suite on a fast filesystem). The for loop and the calls to sorted and items cost a fair bit (according to the profiler) in the null dictionary case.

@arjclark, @hjoliver please review.

@benfitzpatrick benfitzpatrick added this to the next-release milestone Apr 26, 2016
@arjclark
Copy link
Contributor

Looks ok to me, no problems from the test battery in my environment. Over to you @hjoliver

@arjclark arjclark assigned hjoliver and unassigned arjclark Apr 26, 2016
@@ -1224,6 +1224,9 @@ def process_queued_db_ops(self):
"""Handle queued db operations for each task proxy."""
for itask in self.get_all_tasks():
# (runahead pool tasks too, to get new state recorders).
if not (any(itask.db_inserts_map.values()) or
any(itask.db_updates_map.values())):
continue
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Won't this still result in some unnecessary work if one but not both of the maps is empty? You could do:

if any(itask.db_inserts_map.values()):
    for table_name, db_inserts in sorted(itask.db_inserts_map.items()):
         #...
if any(itask.db_updates_map.values()):
    for table_name, db_updates in sorted(itask.db_updates_map.items()):
         #...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yep, that's better and faster!

@hjoliver hjoliver assigned benfitzpatrick and unassigned hjoliver Apr 27, 2016
@arjclark
Copy link
Contributor

Suggested speed up looks good to me.

@hjoliver
Copy link
Member

Sweet.

@hjoliver hjoliver merged commit 5df8d40 into cylc:master Apr 27, 2016
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.

3 participants