Reduce polling (#812)

* Clarify refresh workflow
model refresh -> emit signals -> update view
This renders the history tab coherent

* Refresh when tab is shown

* Clarify link between headChanged and refreshBranch

Co-authored-by: Ian Hunt-Isaak <[email protected]>

Author: fcollonval

README.md

@@ -47,6 +47,7 @@ Once installed, extension behavior can be modified via the following settings wh
 - **displayStatus**: display Git extension status updates in the JupyterLab status bar. If `true`, the extension displays status updates in the JupyterLab status bar, such as when pulling and pushing changes, switching branches, and polling for changes. Depending on the level of extension activity, some users may find the status updates distracting. In which case, setting this to `false` should reduce visual noise.
 - **doubleClickDiff**: double click a file in the Git extension panel to open a diff of the file instead of opening the file for editing.
 - **historyCount**: number of commits shown in the history log, beginning with the most recent. Displaying a larger number of commits can lead to performance degradation, so use caution when modifying this setting.
+- **refreshIfHidden**: whether to refresh even if the Git tab is hidden; default to `false` (i.e. refresh is turned off if the Git tab is hidden).
 - **refreshInterval**: number of milliseconds between polling the file system for changes. In order to ensure that the UI correctly displays the current repository status, the extension must poll the file system for changes. Longer polling times increase the likelihood that the UI does not reflect the current status; however, longer polling times also incur less performance overhead.
 - **simpleStaging**: enable a simplified concept of staging. When this setting is `true`, all files with changes are automatically staged. When we develop in JupyterLab, we often only care about what files have changed (in the broadest sense) and don't need to distinguish between "tracked" and "untracked" files. Accordingly, this setting allows us to simplify the visual presentation of changes, which is especially useful for those less acquainted with Git.
 

schema/plugin.json

@@ -41,6 +41,12 @@
       "description": "Number of (most recent) commits shown in the history log",
       "default": 25
     },
+    "refreshIfHidden": {
+      "type": "boolean",
+      "title": "Refresh if the Git tab is hidden",
+      "description": "Whether to check Git status when the Git tab is not visible. Choose `false` for higher performance.",
+      "default": false
+    },
     "refreshInterval": {
       "type": "integer",
       "title": "Refresh interval",

src/components/GitPanel.tsx

@@ -124,7 +124,7 @@ export class GitPanel extends React.Component<IGitPanelProps, IGitPanelState> {
       this.setState({
         repository: args.newValue
       });
-      this.refresh();
+      this.refreshView();
     }, this);
     model.statusChanged.connect(() => {
       this.setState({ files: model.status });
@@ -133,13 +133,11 @@ export class GitPanel extends React.Component<IGitPanelProps, IGitPanelState> {
       await this.refreshBranch();
       if (this.state.tab === 1) {
         this.refreshHistory();
-      } else {
-        this.refreshStatus();
       }
     }, this);
     model.markChanged.connect(() => this.forceUpdate());
 
-    settings.changed.connect(this.refresh, this);
+    settings.changed.connect(this.refreshView, this);
   }
 
   refreshBranch = async () => {
@@ -168,18 +166,13 @@ export class GitPanel extends React.Component<IGitPanelProps, IGitPanelState> {
     }
   };
 
-  refreshStatus = async () => {
-    await this.props.model.refreshStatus();
-  };
-
   /**
    * Refresh widget, update all content
    */
-  refresh = async () => {
+  refreshView = async () => {
     if (this.props.model.pathRepository !== null) {
       await this.refreshBranch();
       await this.refreshHistory();
-      await this.refreshStatus();
     }
   };
 
@@ -279,7 +272,6 @@ export class GitPanel extends React.Component<IGitPanelProps, IGitPanelState> {
         commands={this.props.commands}
         logger={this.props.logger}
         model={this.props.model}
-        refresh={this._onRefresh}
         repository={this.state.repository || ''}
       />
     );
@@ -446,20 +438,6 @@ export class GitPanel extends React.Component<IGitPanelProps, IGitPanelState> {
     });
   };
 
-  /**
-   * Callback invoked upon refreshing a repository.
-   *
-   * @returns promise which refreshes a repository
-   */
-  private _onRefresh = async () => {
-    await this.refreshBranch();
-    if (this.state.tab === 1) {
-      this.refreshHistory();
-    } else {
-      this.refreshStatus();
-    }
-  };
-
   /**
    * Determines whether a user has a known Git identity.
    *

src/components/Toolbar.tsx

@@ -73,13 +73,6 @@ export interface IToolbarProps {
    * Current repository.
    */
   repository: string;
-
-  /**
-   * Callback to invoke in order to refresh a repository.
-   *
-   * @returns promise which refreshes a repository
-   */
-  refresh: () => Promise<void>;
 }
 
 /**
@@ -320,18 +313,18 @@ export class Toolbar extends React.Component<IToolbarProps, IToolbarState> {
   };
 
   /**
-   * Callback invoked upon clicking a button to refresh a repository.
+   * Callback invoked upon clicking a button to refresh the model.
    *
    * @param event - event object
-   * @returns a promise which resolves upon refreshing a repository
+   * @returns a promise which resolves upon refreshing the model
    */
   private _onRefreshClick = async (): Promise<void> => {
     this.props.logger.log({
       level: Level.RUNNING,
       message: 'Refreshing...'
     });
     try {
-      await this.props.refresh();
+      await this.props.model.refresh();
 
       this.props.logger.log({
         level: Level.SUCCESS,

src/model.ts

@@ -39,35 +39,19 @@ export class GitExtension implements IGitExtension {
     let interval: number;
     if (settings) {
       interval = settings.composite.refreshInterval as number;
-      settings.changed.connect(onSettingsChange, this);
+      settings.changed.connect(this._onSettingsChange, this);
     } else {
       interval = DEFAULT_REFRESH_INTERVAL;
     }
-    const poll = new Poll({
-      factory: () => this.refresh(),
+    this._poll = new Poll({
+      factory: this._refreshModel,
       frequency: {
         interval: interval,
         backoff: true,
         max: 300 * 1000
       },
-      standby: 'when-hidden'
+      standby: this._refreshStandby
     });
-    this._poll = poll;
-
-    /**
-     * Callback invoked upon a change to plugin settings.
-     *
-     * @private
-     * @param settings - settings registry
-     */
-    function onSettingsChange(settings: ISettingRegistry.ISettings) {
-      const freq = poll.frequency;
-      poll.frequency = {
-        interval: settings.composite.refreshInterval as number,
-        backoff: freq.backoff,
-        max: freq.max
-      };
-    }
   }
 
   /**
@@ -152,6 +136,16 @@ export class GitExtension implements IGitExtension {
     }
   }
 
+  /**
+   * Custom model refresh standby condition
+   */
+  get refreshStandbyCondition(): () => boolean {
+    return this._standbyCondition;
+  }
+  set refreshStandbyCondition(v: () => boolean) {
+    this._standbyCondition = v;
+  }
+
   /**
    * A list of modified files.
    *
@@ -401,7 +395,6 @@ export class GitExtension implements IGitExtension {
 
     if (body.checkout_branch) {
       await this.refreshBranch();
-      this._headChanged.emit();
     } else {
       await this.refreshStatus();
     }
@@ -454,8 +447,7 @@ export class GitExtension implements IGitExtension {
         top_repo_path: path
       });
     });
-    await this.refreshStatus();
-    this._headChanged.emit();
+    await this.refresh();
   }
 
   /**
@@ -675,7 +667,7 @@ export class GitExtension implements IGitExtension {
         });
       }
     );
-    this._headChanged.emit();
+    this.refreshBranch(); // Will emit headChanged if required
     return data;
   }
 
@@ -700,7 +692,7 @@ export class GitExtension implements IGitExtension {
         });
       }
     );
-    this._headChanged.emit();
+    this.refreshBranch();
     return data;
   }
 
@@ -710,15 +702,15 @@ export class GitExtension implements IGitExtension {
    * @returns promise which resolves upon refreshing the repository
    */
   async refresh(): Promise<void> {
-    await this._taskHandler.execute<void>('git:refresh', async () => {
-      await this.refreshBranch();
-      await this.refreshStatus();
-    });
+    await this._poll.refresh();
+    await this._poll.tick;
   }
 
   /**
    * Refresh the list of repository branches.
    *
+   * Emit headChanged if the branch or its top commit changes
+   *
    * @returns promise which resolves upon refreshing repository branches
    */
   async refreshBranch(): Promise<void> {
@@ -730,7 +722,15 @@ export class GitExtension implements IGitExtension {
         }
       );
 
-      const headChanged = this._currentBranch !== data.current_branch;
+      let headChanged = false;
+      if (!this._currentBranch || !data) {
+        headChanged = this._currentBranch !== data.current_branch; // Object comparison is not working
+      } else {
+        headChanged =
+          this._currentBranch.name !== data.current_branch.name ||
+          this._currentBranch.top_commit !== data.current_branch.top_commit;
+      }
+
       this._branches = data.branches;
       this._currentBranch = data.current_branch;
       if (this._currentBranch) {
@@ -758,6 +758,8 @@ export class GitExtension implements IGitExtension {
   /**
    * Refresh the repository status.
    *
+   * Emit statusChanged if required.
+   *
    * @returns promise which resolves upon refreshing the repository status
    */
   async refreshStatus(): Promise<void> {
@@ -865,7 +867,6 @@ export class GitExtension implements IGitExtension {
       });
     });
     await this.refreshBranch();
-    this._headChanged.emit();
   }
 
   /**
@@ -1143,12 +1144,25 @@ export class GitExtension implements IGitExtension {
     this._statusChanged.emit(this._status);
   }
 
+  /**
+   * Callback invoked upon a change to plugin settings.
+   *
+   * @private
+   * @param settings - plugin settings
+   */
+  private _onSettingsChange(settings: ISettingRegistry.ISettings) {
+    this._poll.frequency = {
+      ...this._poll.frequency,
+      interval: settings.composite.refreshInterval as number
+    };
+  }
+
   /**
    * open new editor or show an existing editor of the
    * .gitignore file. If the editor does not have unsaved changes
    * then ensure the editor's content matches the file on disk
    */
-  private _openGitignore() {
+  private _openGitignore(): void {
     if (this._docmanager) {
       const widget = this._docmanager.openOrReveal(
         this.getRelativeFilePath('.gitignore')
@@ -1159,6 +1173,38 @@ export class GitExtension implements IGitExtension {
     }
   }
 
+  /**
+   * Refresh model status through a Poll
+   */
+  private _refreshModel = async (): Promise<void> => {
+    await this._taskHandler.execute<void>('git:refresh', async () => {
+      try {
+        await this.refreshBranch();
+        await this.refreshStatus();
+      } catch (error) {
+        console.error('Failed to refresh git status', error);
+      }
+    });
+  };
+
+  /**
+   * Standby test function for the refresh Poll
+   *
+   * Standby refresh if
+   * - webpage is hidden
+   * - not in a git repository
+   * - standby condition is true
+   *
+   * @returns The test function
+   */
+  private _refreshStandby = (): boolean | Poll.Standby => {
+    if (this.pathRepository === null || this._standbyCondition()) {
+      return true;
+    }
+
+    return 'when-hidden';
+  };
+
   /**
    * if file is open in JupyterLab find the widget and ensure the JupyterLab
    * version matches the version on disk. Do nothing if the file has unsaved changes
@@ -1194,6 +1240,7 @@ export class GitExtension implements IGitExtension {
   private _pendingReadyPromise = 0;
   private _poll: Poll;
   private _settings: ISettingRegistry.ISettings | null;
+  private _standbyCondition: () => boolean = () => false;
   private _taskHandler: TaskHandler<IGitExtension>;
 
   private _headChanged = new Signal<IGitExtension, void>(this);

src/tokens.ts

@@ -48,6 +48,11 @@ export interface IGitExtension extends IDisposable {
    */
   ready: Promise<void>;
 
+  /**
+   * Custom model refresh standby condition
+   */
+  refreshStandbyCondition: () => boolean;
+
   /**
    * Files list resulting of a Git status call.
    */