Skip to content

Commit

Permalink
perf: Optimize performance of Results and Samples tables on Explore (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
kgabryje authored Jan 8, 2021
1 parent 76b06b2 commit fd15dff
Show file tree
Hide file tree
Showing 6 changed files with 25 additions and 51 deletions.
15 changes: 0 additions & 15 deletions superset-frontend/spec/javascripts/explore/utils_spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ import {
buildV1ChartDataPayload,
getExploreUrl,
getExploreLongUrl,
getDataTablePageSize,
shouldUseLegacyApi,
getSimpleSQLExpression,
} from 'src/explore/exploreUtils';
Expand Down Expand Up @@ -202,20 +201,6 @@ describe('exploreUtils', () => {
});
});

describe('getDataTablePageSize', () => {
it('divides samples data into pages dynamically', () => {
let pageSize;
pageSize = getDataTablePageSize(500);
expect(pageSize).toEqual(20);
pageSize = getDataTablePageSize(0);
expect(pageSize).toEqual(50);
pageSize = getDataTablePageSize(1);
expect(pageSize).toEqual(10000);
pageSize = getDataTablePageSize(1000000);
expect(pageSize).toEqual(5);
});
});

describe('buildV1ChartDataPayload', () => {
it('generate valid request payload despite no registered buildQuery', () => {
const v1RequestPayload = buildV1ChartDataPayload({
Expand Down
3 changes: 2 additions & 1 deletion superset-frontend/src/components/TableView/TableView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ const TableViewStyles = styled.div`
display: flex;
flex-direction: column;
justify-content: center;
align-items: center;
}
.row-count-container {
Expand Down Expand Up @@ -165,4 +166,4 @@ const TableView = ({
);
};

export default TableView;
export default React.memo(TableView);
29 changes: 13 additions & 16 deletions superset-frontend/src/components/dataViewCommon/TableCollection.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,6 @@ export const Table = styled.table`
background: ${({ theme }) => theme.colors.grayscale.light5};
position: sticky;
top: 0;
white-space: nowrap;
&:first-of-type {
padding-left: ${({ theme }) => theme.gridUnit * 4}px;
}
Expand Down Expand Up @@ -205,17 +202,17 @@ export const Table = styled.table`

Table.displayName = 'table';

export default function TableCollection({
getTableProps,
getTableBodyProps,
prepareRow,
headerGroups,
columns,
rows,
loading,
highlightRowId,
}: TableCollectionProps) {
return (
export default React.memo(
({
getTableProps,
getTableBodyProps,
prepareRow,
headerGroups,
columns,
rows,
loading,
highlightRowId,
}: TableCollectionProps) => (
<Table
{...getTableProps()}
className="table table-hover"
Expand Down Expand Up @@ -314,5 +311,5 @@ export default function TableCollection({
})}
</tbody>
</Table>
);
}
),
);
12 changes: 7 additions & 5 deletions superset-frontend/src/explore/components/DataTableControl.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,12 @@ export const CopyButton = styled(Button)`
}
`;

const CopyNode = (
<CopyButton buttonSize="xs">
<i className="fa fa-clipboard" />
</CopyButton>
);

export const CopyToClipboardButton = ({
data,
}: {
Expand All @@ -52,11 +58,7 @@ export const CopyToClipboardButton = ({
<CopyToClipboard
text={data ? prepareCopyToClipboardTabularData(data) : ''}
wrapped={false}
copyNode={
<CopyButton buttonSize="xs">
<i className="fa fa-clipboard" />
</CopyButton>
}
copyNode={CopyNode}
/>
);

Expand Down
9 changes: 3 additions & 6 deletions superset-frontend/src/explore/components/DataTablesPane.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@ import Loading from 'src/components/Loading';
import TableView, { EmptyWrapperType } from 'src/components/TableView';
import { getChartDataRequest } from 'src/chart/chartAction';
import { getClientErrorObject } from 'src/utils/getClientErrorObject';
import { getDataTablePageSize } from 'src/explore/exploreUtils';
import {
CopyToClipboardButton,
FilterInput,
Expand All @@ -43,6 +42,8 @@ const NULLISH_RESULTS_STATE = {
[RESULT_TYPES.samples]: undefined,
};

const DATA_TABLE_PAGE_SIZE = 50;

const TableControlsWrapper = styled.div`
display: flex;
align-items: center;
Expand Down Expand Up @@ -189,9 +190,6 @@ export const DataTablesPane = ({
};

const renderDataTable = (type: string) => {
// restrict cell count to 10000 or min 5 rows to avoid crashing browser
const columnsLength = columns[type].length;
const pageSize = getDataTablePageSize(columnsLength);
if (isLoading[type]) {
return <Loading />;
}
Expand All @@ -206,8 +204,7 @@ export const DataTablesPane = ({
<TableView
columns={columns[type]}
data={filteredData[type]}
withPagination
pageSize={pageSize}
pageSize={DATA_TABLE_PAGE_SIZE}
noDataText={t('No data')}
emptyWrapperType={EmptyWrapperType.Small}
className="table-condensed"
Expand Down
8 changes: 0 additions & 8 deletions superset-frontend/src/explore/exploreUtils.js
Original file line number Diff line number Diff line change
Expand Up @@ -251,14 +251,6 @@ export function postForm(url, payload, target = '_blank') {
document.body.removeChild(hiddenForm);
}

export function getDataTablePageSize(columnsLength) {
let pageSize;
if (columnsLength) {
pageSize = Math.ceil(Math.max(5, 10000 / columnsLength));
}
return pageSize || 50;
}

export const exportChart = ({
formData,
resultFormat = 'json',
Expand Down

0 comments on commit fd15dff

Please sign in to comment.