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

Reduce polling #812

Merged
merged 7 commits into from
Nov 7, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -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.

Expand Down
6 changes: 6 additions & 0 deletions schema/plugin.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
28 changes: 3 additions & 25 deletions src/components/GitPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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 });
Expand All @@ -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 () => {
Expand Down Expand Up @@ -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();
}
};

Expand Down Expand Up @@ -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 || ''}
/>
);
Expand Down Expand Up @@ -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.
*
Expand Down
13 changes: 3 additions & 10 deletions src/components/Toolbar.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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>;
}

/**
Expand Down Expand Up @@ -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,
Expand Down
111 changes: 79 additions & 32 deletions src/model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
};
}
}

/**
Expand Down Expand Up @@ -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.
*
Expand Down Expand Up @@ -401,7 +395,6 @@ export class GitExtension implements IGitExtension {

if (body.checkout_branch) {
await this.refreshBranch();
this._headChanged.emit();
} else {
await this.refreshStatus();
}
Expand Down Expand Up @@ -454,8 +447,7 @@ export class GitExtension implements IGitExtension {
top_repo_path: path
});
});
await this.refreshStatus();
this._headChanged.emit();
await this.refresh();
}

/**
Expand Down Expand Up @@ -655,7 +647,7 @@ export class GitExtension implements IGitExtension {
});
}
);
this._headChanged.emit();
this.refreshBranch(); // Will emit headChanged if required
return data;
}

Expand All @@ -680,7 +672,7 @@ export class GitExtension implements IGitExtension {
});
}
);
this._headChanged.emit();
this.refreshBranch();
return data;
}

Expand All @@ -690,15 +682,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> {
Expand All @@ -710,7 +702,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) {
Expand Down Expand Up @@ -738,6 +738,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> {
Expand Down Expand Up @@ -845,7 +847,6 @@ export class GitExtension implements IGitExtension {
});
});
await this.refreshBranch();
this._headChanged.emit();
}

/**
Expand Down Expand Up @@ -1123,12 +1124,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')
Expand All @@ -1139,6 +1153,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
Expand Down Expand Up @@ -1174,6 +1220,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);
Expand Down
5 changes: 5 additions & 0 deletions src/tokens.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
Loading