Skip to content

Commit 5a675be

Browse files
hoonjiAnuarTB
authored andcommitted
Memoizes the functions for getting header both runs and scalar data tables (tensorflow#6836)
## Motivation for features / changes Using functions in Angular templates leads to unnecessary recomputation, e.g. getHeaders() is called 60 times from data tables on init and add). More seriously, they also interfere with other component usage like CDK Overlay: namely, when an Overlay is set to appear next to a header, the header element is destroyed and recreated (because of the function in the template) before the Overlay can position itself, and ends up always resetting its position to (0,0). Simply memoizing the function will prevent the recomputation ([Angular ref](https://angular.io/guide/change-detection-slow-computations#optimizing-slow-computations)). Profiling the memoized change (using Chrome devtools profiler, which is admittedly a bit inconsistent) seems to show a several 100ms improvement in scripting performance. ## Technical description of changes Replaces the `getHeaders` functions used in scalar/runs data tables with a new memoized `extendHeaders(headers)` function. No change in UI behavior. ## Screenshots of UI changes (or N/A) N/A
1 parent 3e1dcdb commit 5a675be

File tree

7 files changed

+52
-42
lines changed

7 files changed

+52
-42
lines changed

tensorboard/webapp/metrics/views/card_renderer/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,6 +286,7 @@ tf_ng_module(
286286
":utils",
287287
"//tensorboard/webapp/metrics:types",
288288
"//tensorboard/webapp/runs:types",
289+
"//tensorboard/webapp/util:memoize",
289290
"//tensorboard/webapp/widgets/card_fob:types",
290291
"//tensorboard/webapp/widgets/data_table",
291292
"//tensorboard/webapp/widgets/data_table:types",

tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ng.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,7 @@
3030
(loadAllColumns)="loadAllColumns.emit()"
3131
>
3232
<ng-container header>
33-
<ng-container *ngFor="let header of getHeaders()">
33+
<ng-container *ngFor="let header of extendHeaders(columnHeaders)">
3434
<tb-data-table-header-cell
3535
*ngIf="header.enabled && (header.type !== ColumnHeaderType.SMOOTHED || smoothingEnabled)"
3636
[header]="header"
@@ -42,7 +42,7 @@
4242
<ng-container content>
4343
<ng-container *ngFor="let dataRow of getTimeSelectionTableData()">
4444
<tb-data-table-content-row>
45-
<ng-container *ngFor="let header of getHeaders()">
45+
<ng-container *ngFor="let header of extendHeaders(columnHeaders)">
4646
<tb-data-table-content-cell
4747
*ngIf="header.enabled && (header.type !== ColumnHeaderType.SMOOTHED || smoothingEnabled)"
4848
[header]="header"

tensorboard/webapp/metrics/views/card_renderer/scalar_card_data_table.ts

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import {
4343
AddColumnEvent,
4444
} from '../../../widgets/data_table/types';
4545
import {isDatumVisible} from './utils';
46+
import {memoize} from '../../../util/memoize';
4647

4748
@Component({
4849
selector: 'scalar-card-data-table',
@@ -74,15 +75,23 @@ export class ScalarCardDataTable {
7475

7576
ColumnHeaderType = ColumnHeaderType;
7677

77-
getHeaders(): ColumnHeader[] {
78-
return [
79-
{
80-
name: 'color',
81-
displayName: '',
82-
type: ColumnHeaderType.COLOR,
83-
enabled: true,
84-
},
85-
].concat(this.columnHeaders);
78+
// Columns must be memoized to stop needless re-rendering of the content and headers in these
79+
// columns. This has been known to cause problems with the controls in these columns,
80+
// specifically the add button.
81+
extendHeaders = memoize(this.internalExtendHeaders);
82+
83+
private internalExtendHeaders(headers: ColumnHeader[]) {
84+
return ([] as Array<ColumnHeader>).concat(
85+
[
86+
{
87+
name: 'color',
88+
displayName: '',
89+
type: ColumnHeaderType.COLOR,
90+
enabled: true,
91+
},
92+
],
93+
headers
94+
);
8695
}
8796

8897
getMinPointInRange(

tensorboard/webapp/runs/views/runs_table/BUILD

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,7 @@ tf_ng_module(
110110
"//tensorboard/webapp/types:ui",
111111
"//tensorboard/webapp/util:colors",
112112
"//tensorboard/webapp/util:matcher",
113+
"//tensorboard/webapp/util:memoize",
113114
"//tensorboard/webapp/widgets/custom_modal",
114115
"//tensorboard/webapp/widgets/data_table",
115116
"//tensorboard/webapp/widgets/data_table:filter_dialog",

tensorboard/webapp/runs/views/runs_table/runs_data_table.ng.html

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,7 @@
4141
(loadAllColumns)="loadAllColumns.emit()"
4242
>
4343
<ng-container header>
44-
<ng-container *ngFor="let header of getHeaders()">
44+
<ng-container *ngFor="let header of extendHeaders(headers)">
4545
<tb-data-table-header-cell
4646
[header]="header"
4747
[sortingInfo]="sortingInfo"
@@ -67,7 +67,7 @@
6767
<ng-container content>
6868
<ng-container *ngFor="let dataRow of data; trackBy: trackByRuns">
6969
<tb-data-table-content-row [attr.data-id]="dataRow.id">
70-
<ng-container *ngFor="let header of getHeaders()">
70+
<ng-container *ngFor="let header of extendHeaders(headers)">
7171
<tb-data-table-content-cell
7272
[header]="header"
7373
[datum]="dataRow[header.name]"

tensorboard/webapp/runs/views/runs_table/runs_data_table.ts

Lines changed: 23 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
ReorderColumnEvent,
3131
AddColumnEvent,
3232
} from '../../../widgets/data_table/types';
33+
import {memoize} from '../../../util/memoize';
3334
@Component({
3435
selector: 'runs-data-table',
3536
templateUrl: 'runs_data_table.ng.html',
@@ -67,32 +68,30 @@ export class RunsDataTable {
6768
@Output() addFilter = new EventEmitter<FilterAddedEvent>();
6869
@Output() loadAllColumns = new EventEmitter<null>();
6970

70-
// These columns must be stored and reused to stop needless re-rendering of
71-
// the content and headers in these columns. This has been known to cause
72-
// problems with the controls in these columns, specifically the color picker.
73-
beforeColumns = [
74-
{
75-
name: 'selected',
76-
displayName: '',
77-
type: ColumnHeaderType.CUSTOM,
78-
enabled: true,
79-
},
80-
];
71+
// Columns must be memoized to stop needless re-rendering of the content and headers in these
72+
// columns. This has been known to cause problems with the controls in these columns,
73+
// specifically the add button.
74+
extendHeaders = memoize(this.internalExtendHeaders);
8175

82-
afterColumns = [
83-
{
84-
name: 'color',
85-
displayName: '',
86-
type: ColumnHeaderType.COLOR,
87-
enabled: true,
88-
},
89-
];
90-
91-
getHeaders() {
76+
private internalExtendHeaders(headers: ColumnHeader[]) {
9277
return ([] as Array<ColumnHeader>).concat(
93-
this.beforeColumns,
94-
this.headers,
95-
this.afterColumns
78+
[
79+
{
80+
name: 'selected',
81+
displayName: '',
82+
type: ColumnHeaderType.CUSTOM,
83+
enabled: true,
84+
},
85+
],
86+
headers,
87+
[
88+
{
89+
name: 'color',
90+
displayName: '',
91+
type: ColumnHeaderType.COLOR,
92+
enabled: true,
93+
},
94+
]
9695
);
9796
}
9897

tensorboard/webapp/runs/views/runs_table/runs_data_table_test.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -220,25 +220,25 @@ describe('runs_data_table', () => {
220220
});
221221
});
222222

223-
// If the call to getHeaders produces a new color header on each call, then
223+
// If the call to extendHeaders produces a new color header on each call, then
224224
// the content gets re-rendered when the color picker opens. This causes the
225225
// color picker modal to close immediately. This test ensures that we keep
226226
// the same reference to the color header and continue to use that reference
227227
// instead of creating a new one on each call.
228-
it('getHeaders does not produce new color header on each call', () => {
228+
it('extendHeaders does not produce new color header on each call', () => {
229229
const fixture = createComponent({});
230-
231230
const runsDataTable = fixture.debugElement.query(
232231
By.directive(RunsDataTable)
233232
);
233+
const {headers} = runsDataTable.componentInstance;
234234

235235
expect(
236236
runsDataTable.componentInstance
237-
.getHeaders()
237+
.extendHeaders(headers)
238238
.find((header: ColumnHeader) => header.name === 'color')
239239
).toBe(
240240
runsDataTable.componentInstance
241-
.getHeaders()
241+
.extendHeaders(headers)
242242
.find((header: ColumnHeader) => header.name === 'color')
243243
);
244244
});

0 commit comments

Comments
 (0)