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

rose edit: speed up adding/deleting sections #1758

Merged

Conversation

benfitzpatrick
Copy link
Contributor

This speeds up rose edit, particularly for adding and deleting sections.

When deleting or adding sections in one of the summary panel windows
(including the custom STASH widget) the speed up can be around 2x -
larger or smaller depending on the total number of sections in a configuration.

The change works by caching the slower function calls in the post-change updating,
and by avoiding expensive treeview re-drawing when there's no change.

@benfitzpatrick benfitzpatrick added this to the next-release milestone Nov 18, 2015
@benfitzpatrick benfitzpatrick force-pushed the rose-edit.speedup-summary-sections branch from fb4488e to fd6180c Compare November 18, 2015 16:25
@matthewrmshin matthewrmshin modified the milestones: next-release, 2015.12.0 Dec 10, 2015
@benfitzpatrick benfitzpatrick modified the milestones: soon, next-release Jan 11, 2016
@benfitzpatrick benfitzpatrick modified the milestones: next-release, soon Jan 26, 2016
@benfitzpatrick
Copy link
Contributor Author

@matthewrmshin, please review.

@matthewrmshin
Copy link
Member

Change looks sane to me, and does not appear to break the new syntax test. I'd prefer if this branch can be re-based merge, because it is very old.

@stevewardle as discussed, please review and test.

@@ -434,16 +440,23 @@ def get_ns_ignored_status(self, namespace):
status_counts = object_statuses.items()
status_counts.sort(lambda x, y: cmp(x[1], y[1]))
if not status_counts:
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems as though cache[namespace] is being set on a few more lines than it really needs to be here (actually above this point as well but it's clearest in this section) - for instance it could appear once after L445 instead of on L447,L450 and L452

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, good point

@benfitzpatrick benfitzpatrick force-pushed the rose-edit.speedup-summary-sections branch from fd6180c to 525315c Compare January 27, 2016 10:34
@benfitzpatrick
Copy link
Contributor Author

Rebased

@stevewardle
Copy link
Contributor

Based on testing this on a fairly typical UM app - it's definitely faster at what it's doing. However I notice that the blue * indicators on the STASH request panel don't appear to update when ignoring the items (I guess maybe the early-exit via the cached settings is happening before it updates the indicators?)

@benfitzpatrick
Copy link
Contributor Author

OK, I've removed the change that led to the indicators not showing. The other changes are independent and should still lead to good speedup. We'll have to address the STASH panel behaviour in another pull request, because I think it can be sped up much more.

@stevewardle
Copy link
Contributor

I've had a look again and it seems like the issue above is resolved

stevewardle pushed a commit that referenced this pull request Feb 8, 2016
…y-sections

rose edit: speed up adding/deleting sections
@stevewardle stevewardle merged commit ad794ad into metomi:master Feb 8, 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