From 4aec66f5bb3126359c15bd37eeced60536540856 Mon Sep 17 00:00:00 2001 From: "James E. Blair" Date: Fri, 31 Oct 2014 11:31:19 -0700 Subject: [PATCH] Selectively refresh screen When sync events produce results that modify the local db state, record those events and have the screen widgets use that info to decide whether to refresh (if relevant data are updated). In practice this should mean far fewer screen refreshes (which require costly db queries and manifest as UI pauses). Change-Id: Iefca75ef3d727236b8e4d1199fd4301d25822fef --- gertty/app.py | 8 +++++- gertty/sync.py | 53 ++++++++++++++++++++++++++++++++++--- gertty/view/change.py | 12 ++++++++- gertty/view/change_list.py | 23 +++++++++++++--- gertty/view/diff.py | 13 +++++++-- gertty/view/project_list.py | 16 +++++++++-- 6 files changed, 113 insertions(+), 12 deletions(-) diff --git a/gertty/app.py b/gertty/app.py index 350bd14..1662442 100644 --- a/gertty/app.py +++ b/gertty/app.py @@ -16,6 +16,7 @@ import argparse import logging import os +import Queue import sys import threading import webbrowser @@ -227,7 +228,12 @@ class App(object): widget = self.loop.widget while isinstance(widget, urwid.Overlay): widget = widget.contents[0][0] - widget.refresh() + try: + while True: + event = self.sync.result_queue.get(0) + widget.refresh(event) + except Queue.Empty: + pass def popup(self, widget, relative_width=50, relative_height=25, diff --git a/gertty/sync.py b/gertty/sync.py index e758a7a..fdbb8b9 100644 --- a/gertty/sync.py +++ b/gertty/sync.py @@ -78,6 +78,38 @@ class MultiQueue(object): finally: self.condition.release() +class UpdateEvent(object): + def updateRelatedChanges(self, session, change): + related_change_keys = set() + related_change_keys.add(change.key) + for revision in change.revisions: + parent = session.getRevisionByCommit(revision.parent) + if parent: + related_change_keys.add(parent.change.key) + for child in session.getRevisionsByParent(revision.commit): + related_change_keys.add(child.change.key) + self.related_change_keys = related_change_keys + +class ProjectAddedEvent(UpdateEvent): + def __init__(self, project): + self.project_key = project.key + +class ChangeAddedEvent(UpdateEvent): + def __init__(self, change): + self.project_key = change.project.key + self.change_key = change.key + self.related_change_keys = set() + self.review_flag_changed = True + self.status_changed = True + +class ChangeUpdatedEvent(UpdateEvent): + def __init__(self, change): + self.project_key = change.project.key + self.change_key = change.key + self.related_change_keys = set() + self.review_flag_changed = False + self.status_changed = False + class Task(object): def __init__(self, priority=NORMAL_PRIORITY): self.log = logging.getLogger('gertty.sync') @@ -85,6 +117,7 @@ class Task(object): self.succeeded = None self.event = threading.Event() self.tasks = [] + self.results = [] def complete(self, success): self.succeeded = success @@ -127,7 +160,9 @@ class SyncProjectListTask(Task): for name in remote_keys-local_keys: p = remote[name] - session.createProject(name, description=p.get('description', '')) + project = session.createProject(name, + description=p.get('description', '')) + self.results.append(ProjectAddedEvent(project)) class SyncSubscribedProjectBranchesTask(Task): def __repr__(self): @@ -327,8 +362,14 @@ class SyncChangeTask(Task): remote_change['subject'], created, updated, remote_change['status'], topic=remote_change.get('topic')) + result = ChangeAddedEvent(change) + else: + result = ChangeUpdatedEvent(change) + self.results.append(result) change.owner = account - change.status = remote_change['status'] + if change.status != remote_change['status']: + change.status = remote_change['status'] + result.status_changed = True change.subject = remote_change['subject'] change.updated = dateutil.parser.parse(remote_change['updated']) change.topic = remote_change.get('topic') @@ -367,6 +408,7 @@ class SyncChangeTask(Task): sync.submitTask(SyncChangeByCommitTask(revision.parent, self.priority)) self.log.debug("Change %s revision %s needs parent commit %s synced" % (change.id, remote_revision['_number'], revision.parent)) + result.updateRelatedChanges(session, change) remote_comments_data = remote_revision['_gertty_remote_comments_data'] for remote_file, remote_comments in remote_comments_data.items(): for remote_comment in remote_comments: @@ -499,7 +541,9 @@ class SyncChangeTask(Task): if not user_voted: # Only consider changing the reviewed state if we don't have a vote if new_revision or new_message: - change.reviewed = False + if not change.reviewed: + change.reviewed = False + result.review_flag_changed = True for url, refs in fetches.items(): self.log.debug("Fetching from %s with refs %s", url, refs) try: @@ -781,6 +825,7 @@ class Sync(object): self.app = app self.log = logging.getLogger('gertty.sync') self.queue = MultiQueue([HIGH_PRIORITY, NORMAL_PRIORITY, LOW_PRIORITY]) + self.result_queue = Queue.Queue() self.session = requests.Session() if self.app.config.auth_type == 'basic': authclass = requests.auth.HTTPBasicAuth @@ -838,6 +883,8 @@ class Sync(object): self.app.status.update(error=True, refresh=False) self.offline = False self.app.status.update(offline=False, refresh=False) + for r in task.results: + self.result_queue.put(r) os.write(pipe, 'refresh\n') return None diff --git a/gertty/view/change.py b/gertty/view/change.py index 096e15f..028d466 100644 --- a/gertty/view/change.py +++ b/gertty/view/change.py @@ -14,6 +14,7 @@ # under the License. import datetime +import logging import urwid @@ -415,6 +416,7 @@ class ChangeView(urwid.WidgetWrap): def __init__(self, app, change_key): super(ChangeView, self).__init__(urwid.Pile([])) + self.log = logging.getLogger('gertty.view.change') self.app = app self.change_key = change_key self.revision_rows = {} @@ -497,7 +499,15 @@ class ChangeView(urwid.WidgetWrap): if not succeeded: raise gertty.view.DisplayError("Git commits not present in local repository") - def refresh(self): + def refresh(self, event=None): + if event and not ((isinstance(event, sync.ChangeAddedEvent) and + self.change_key in event.related_change_keys) + or + (isinstance(event, sync.ChangeUpdatedEvent) and + self.change_key in event.related_change_keys)): + self.log.debug("Ignoring refresh change due to event %s" % (event,)) + return + self.log.debug("Refreshing change due to event %s" % (event,)) change_info = [] with self.app.db.getSession() as session: change = session.getChange(self.change_key) diff --git a/gertty/view/change_list.py b/gertty/view/change_list.py index fb39f74..396ffd9 100644 --- a/gertty/view/change_list.py +++ b/gertty/view/change_list.py @@ -14,6 +14,7 @@ # under the License. import datetime +import logging import urwid from gertty import keymap @@ -115,8 +116,10 @@ class ChangeListView(urwid.WidgetWrap): "Reverse the sort") ] - def __init__(self, app, query, query_desc=None, unreviewed=False): + def __init__(self, app, query, query_desc=None, project_key=None, + unreviewed=False): super(ChangeListView, self).__init__(urwid.Pile([])) + self.log = logging.getLogger('gertty.view.change_list') self.app = app self.query = query self.query_desc = query_desc or query @@ -124,7 +127,8 @@ class ChangeListView(urwid.WidgetWrap): self.change_rows = {} self.listbox = urwid.ListBox(urwid.SimpleFocusListWalker([])) self.display_owner = self.display_project = self.display_updated = True - if '_project_key' in query: + self.project_key = project_key + if project_key is not None: self.display_project = False self.sort_by = 'number' self.reverse = False @@ -138,7 +142,20 @@ class ChangeListView(urwid.WidgetWrap): self._w.contents.append((self.listbox, ('weight', 1))) self._w.set_focus(3) - def refresh(self): + def refresh(self, event=None): + if event and not (('_project_key' in self.query and + isinstance(event, sync.ChangeAddedEvent) and + self.project_key == event.project_key) + or + ('_project_key' not in self.query and + isinstance(event, sync.ChangeAddedEvent)) + or + (isinstance(event, sync.ChangeUpdatedEvent) and + event.change_key in self.change_rows.keys())): + self.log.debug("Ignoring refresh change list due to event %s" % (event,)) + return + self.log.debug("Refreshing change list due to event %s" % (event,)) + unseen_keys = set(self.change_rows.keys()) with self.app.db.getSession() as session: lst = session.getChanges(self.query, self.unreviewed, diff --git a/gertty/view/diff.py b/gertty/view/diff.py index 2d9f9cb..9630a7d 100644 --- a/gertty/view/diff.py +++ b/gertty/view/diff.py @@ -20,6 +20,7 @@ import urwid from gertty import keymap from gertty import mywid from gertty import gitrepo +from gertty import sync class PatchsetDialog(urwid.WidgetWrap): signals = ['ok', 'cancel'] @@ -340,9 +341,17 @@ class BaseDiffView(urwid.WidgetWrap): def makeFileHeader(self, diff, comment_lists): raise NotImplementedError - def refresh(self): + def refresh(self, event=None): + if event and not ((isinstance(event, sync.ChangeAddedEvent) and + self.change_key in event.related_change_keys) + or + (isinstance(event, sync.ChangeUpdatedEvent) and + self.change_key in event.related_change_keys)): + #self.log.debug("Ignoring refresh diff due to event %s" % (event,)) + return + #self.log.debug("Refreshing diff due to event %s" % (event,)) #TODO - pass + return def keypress(self, size, key): old_focus = self.listbox.focus diff --git a/gertty/view/project_list.py b/gertty/view/project_list.py index 221bed7..5fa1eab 100644 --- a/gertty/view/project_list.py +++ b/gertty/view/project_list.py @@ -13,6 +13,7 @@ # License for the specific language governing permissions and limitations # under the License. +import logging import urwid from gertty import keymap @@ -82,6 +83,7 @@ class ProjectListView(urwid.WidgetWrap): def __init__(self, app): super(ProjectListView, self).__init__(urwid.Pile([])) + self.log = logging.getLogger('gertty.view.project_list') self.app = app self.unreviewed = True self.subscribed = True @@ -95,7 +97,17 @@ class ProjectListView(urwid.WidgetWrap): self._w.contents.append((self.listbox, ('weight', 1))) self._w.set_focus(3) - def refresh(self): + def refresh(self, event=None): + if event and not (isinstance(event, sync.ProjectAddedEvent) + or + isinstance(event, sync.ChangeAddedEvent) + or + (isinstance(event, sync.ChangeUpdatedEvent) and + (event.status_changed or event.review_flag_changed))): + self.log.debug("Ignoring refresh project list due to event %s" % (event,)) + return + self.log.debug("Refreshing project list due to event %s" % (event,)) + if self.subscribed: self.title = u'Subscribed projects' if self.unreviewed: @@ -134,7 +146,7 @@ class ProjectListView(urwid.WidgetWrap): self.app.changeScreen(view_change_list.ChangeListView( self.app, "_project_key:%s %s" % (project_key, self.app.config.project_change_list_query), - project_name, unreviewed=True)) + project_name, project_key=None, unreviewed=True)) def keypress(self, size, key): r = super(ProjectListView, self).keypress(size, key)