Skip to content

Commit

Permalink
improve rerender of vz_line_chart (tensorflow#3524)
Browse files Browse the repository at this point in the history
We identified few several expensive operations going on underneath Plottable.
- When we modified dataset() attached to a plot, Plottable recalculated
  domain which in turn triggered redraw of the plot.
- Plot redraw is scheduled but repeated `clearTimeout` and `setTimeout`
  take considerable time which was the large part of the dataset update
- Changing a dataset caused us to updateSpecialDataset N times (N =
  number of runs)

We addressed the issue by:
- `commit` API after making all the changes
- remember which runs/dataset were changed
- update datasets only once per commit

We did not address:
- autoDomain recomputing the bounds on every data changes.
  - there is no programmatical way to disable auto domain unless we
    change the domain which causes, for instance,  zoom level to reset

Empirically, the render time:
- toggling run on the selector went down from 1740ms to 273ms
- triggering the log scale went down from 315ms to 264ms
  - from ~670ms to ~620ms when measured from mouse up

Do note that this change does not improve time for other charting
operation like zoom, smoothing changes, and etc...
  • Loading branch information
stephanwlee authored Apr 22, 2020
1 parent 0aee979 commit 27d0023
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -219,6 +219,10 @@
this.$.chart.setSeriesMetadata(name, metadata);
},

commitChanges() {
this.$.chart.commitChanges();
},

redraw() {
cancelAnimationFrame(this._redrawRaf);
this._redrawRaf = window.requestAnimationFrame(() => {
Expand Down
76 changes: 49 additions & 27 deletions tensorboard/components/vz_line_chart2/line-chart.ts
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ namespace vz_line_chart2 {
private lastPointsDataset: Plottable.Dataset;
private fillArea?: FillArea;
private datasets: Plottable.Dataset[];
private onDatasetChanged: (dataset: Plottable.Dataset) => void;
private nanDataset: Plottable.Dataset;
private smoothingWeight: number;
private smoothingEnabled: boolean;
Expand Down Expand Up @@ -139,10 +138,6 @@ namespace vz_line_chart2 {
// varies based on whether smoothing is enabled.
this.symbolFunction = symbolFunction;

// need to do a single bind, so we can deregister the callback from
// old Plottable.Datasets. (Deregistration is done by identity checks.)
this.onDatasetChanged = this._onDatasetChanged.bind(this);

this._defaultXRange = defaultXRange;
this._defaultYRange = defaultYRange;
this.tooltipColumns = tooltipColumns;
Expand Down Expand Up @@ -322,16 +317,6 @@ namespace vz_line_chart2 {
return new Plottable.Components.Group(groups);
}

/** Updates the chart when a dataset changes. Called every time the data of
* a dataset changes to update the charts.
*/
private _onDatasetChanged(dataset: Plottable.Dataset) {
if (this.smoothingEnabled) {
this.resmoothDataset(dataset);
}
this.updateSpecialDatasets();
}

public ignoreYOutliers(ignoreYOutliers: boolean) {
if (ignoreYOutliers !== this._ignoreYOutliers) {
this._ignoreYOutliers = ignoreYOutliers;
Expand Down Expand Up @@ -832,25 +817,53 @@ namespace vz_line_chart2 {
}

/**
* Update the selected series on the chart.
* Stages update of visible series on the chart.
*
* Please call `commitChanges` for the changes to be reflected on the chart
* after making all the changes.
*/
public setVisibleSeries(names: string[]) {
this.disableChanges();
names = names.sort();
names.reverse(); // draw first series on top
this.seriesNames = names;
}

names.reverse(); // draw first series on top
this.datasets.forEach((d) => d.offUpdate(this.onDatasetChanged));
this.datasets = names.map((r) => this.getDataset(r));
this.datasets.forEach((d) => d.onUpdate(this.onDatasetChanged));
this.linePlot.datasets(this.datasets);
private dirtyDatasets = new Set<string>();

private disableChanges() {
if (!this.dirtyDatasets.size) {
// Prevent plots from reacting to the dataset changes.
this.linePlot.datasets([]);
if (this.smoothLinePlot) {
this.smoothLinePlot.datasets([]);
}

if (this.marginAreaPlot) {
this.marginAreaPlot.datasets([]);
}
}
}

public commitChanges() {
this.datasets = this.seriesNames.map((r) => this.getDataset(r));
[...this.dirtyDatasets].forEach((d) => {
if (this.smoothingEnabled) {
this.resmoothDataset(this.getDataset(d));
}
});
this.updateSpecialDatasets();

this.linePlot.datasets(this.datasets);
if (this.smoothingEnabled) {
this.smoothLinePlot.datasets(this.datasets);
}
if (this.marginAreaPlot) {
this.marginAreaPlot.datasets(this.datasets);
}
this.updateSpecialDatasets();

this.measureBBoxAndMaybeInvalidateLayoutInRaf();
this.dirtyDatasets.clear();
}

/**
Expand All @@ -877,21 +890,30 @@ namespace vz_line_chart2 {
}

/**
* Sets the data of a series on the chart.
* Stages a data change of a series on the chart.
*
* Please call `commitChanges` for the changes to be reflected on the chart
* after making all the changes.
*/
public setSeriesData(name: string, data: vz_chart_helpers.ScalarDatum[]) {
this.disableChanges();
this.getDataset(name).data(data);
this.measureBBoxAndMaybeInvalidateLayoutInRaf();
this.dirtyDatasets.add(name);
}

/**
* Sets the metadata of a series on the chart.
* Sets a metadata change of a series on the chart.
*
* Please call `commitChanges` for the changes to be reflected on the chart
* after making all the changes.
*/
public setSeriesMetadata(name: string, meta: any) {
const newMeta = Object.assign({}, this.getDataset(name).metadata(), {
this.disableChanges();
this.getDataset(name).metadata({
...this.getDataset(name).metadata(),
meta,
});
this.getDataset(name).metadata(newMeta);
this.dirtyDatasets.add(name);
}

public smoothingUpdate(weight: number) {
Expand Down
6 changes: 6 additions & 0 deletions tensorboard/components/vz_line_chart2/vz-line-chart2.ts
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,11 @@ namespace vz_line_chart2 {
}
},

commitChanges() {
if (!this._chart) return;
this._chart.commitChanges();
},

/**
* Reset the chart domain. If the chart has not rendered yet, a call to this
* method no-ops.
Expand Down Expand Up @@ -426,6 +431,7 @@ namespace vz_line_chart2 {
this._chart.setSeriesMetadata(name, this._seriesMetadataCache[name]);
});
this._chart.setVisibleSeries(this._visibleSeriesCache);
this._chart.commitChanges();
},

_smoothingChanged: function() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,7 @@
const name = this._getSeriesNameFromDatum(datum);
scalarChart.setSeriesMetadata(name, datum);
scalarChart.setSeriesData(name, formattedData);
scalarChart.commitChanges();
};
},
readOnly: true,
Expand Down

0 comments on commit 27d0023

Please sign in to comment.