You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by yj...@apache.org on 2021/01/08 20:08:30 UTC

[superset] branch master updated: perf: Optimize performance of Results and Samples tables on Explore (#12257)

This is an automated email from the ASF dual-hosted git repository.

yjc pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new fd15dff  perf: Optimize performance of Results and Samples tables on Explore (#12257)
fd15dff is described below

commit fd15dff60eeb07c58efc9fa434809867cd41b336
Author: Kamil Gabryjelski <ka...@gmail.com>
AuthorDate: Fri Jan 8 21:07:51 2021 +0100

    perf: Optimize performance of Results and Samples tables on Explore (#12257)
---
 .../spec/javascripts/explore/utils_spec.jsx        | 15 -----------
 .../src/components/TableView/TableView.tsx         |  3 ++-
 .../components/dataViewCommon/TableCollection.tsx  | 29 ++++++++++------------
 .../src/explore/components/DataTableControl.tsx    | 12 +++++----
 .../src/explore/components/DataTablesPane.tsx      |  9 +++----
 superset-frontend/src/explore/exploreUtils.js      |  8 ------
 6 files changed, 25 insertions(+), 51 deletions(-)

diff --git a/superset-frontend/spec/javascripts/explore/utils_spec.jsx b/superset-frontend/spec/javascripts/explore/utils_spec.jsx
index e41db22..251475a 100644
--- a/superset-frontend/spec/javascripts/explore/utils_spec.jsx
+++ b/superset-frontend/spec/javascripts/explore/utils_spec.jsx
@@ -23,7 +23,6 @@ import {
   buildV1ChartDataPayload,
   getExploreUrl,
   getExploreLongUrl,
-  getDataTablePageSize,
   shouldUseLegacyApi,
   getSimpleSQLExpression,
 } from 'src/explore/exploreUtils';
@@ -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({
diff --git a/superset-frontend/src/components/TableView/TableView.tsx b/superset-frontend/src/components/TableView/TableView.tsx
index 761a341..58b02bd 100644
--- a/superset-frontend/src/components/TableView/TableView.tsx
+++ b/superset-frontend/src/components/TableView/TableView.tsx
@@ -56,6 +56,7 @@ const TableViewStyles = styled.div`
     display: flex;
     flex-direction: column;
     justify-content: center;
+    align-items: center;
   }
 
   .row-count-container {
@@ -165,4 +166,4 @@ const TableView = ({
   );
 };
 
-export default TableView;
+export default React.memo(TableView);
diff --git a/superset-frontend/src/components/dataViewCommon/TableCollection.tsx b/superset-frontend/src/components/dataViewCommon/TableCollection.tsx
index fe7f8a9..1c11f81 100644
--- a/superset-frontend/src/components/dataViewCommon/TableCollection.tsx
+++ b/superset-frontend/src/components/dataViewCommon/TableCollection.tsx
@@ -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;
     }
@@ -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"
@@ -314,5 +311,5 @@ export default function TableCollection({
           })}
       </tbody>
     </Table>
-  );
-}
+  ),
+);
diff --git a/superset-frontend/src/explore/components/DataTableControl.tsx b/superset-frontend/src/explore/components/DataTableControl.tsx
index cae88b0..30ffbd4 100644
--- a/superset-frontend/src/explore/components/DataTableControl.tsx
+++ b/superset-frontend/src/explore/components/DataTableControl.tsx
@@ -44,6 +44,12 @@ export const CopyButton = styled(Button)`
   }
 `;
 
+const CopyNode = (
+  <CopyButton buttonSize="xs">
+    <i className="fa fa-clipboard" />
+  </CopyButton>
+);
+
 export const CopyToClipboardButton = ({
   data,
 }: {
@@ -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}
   />
 );
 
diff --git a/superset-frontend/src/explore/components/DataTablesPane.tsx b/superset-frontend/src/explore/components/DataTablesPane.tsx
index a9403c3..79ed09b 100644
--- a/superset-frontend/src/explore/components/DataTablesPane.tsx
+++ b/superset-frontend/src/explore/components/DataTablesPane.tsx
@@ -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,
@@ -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;
@@ -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 />;
     }
@@ -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"
diff --git a/superset-frontend/src/explore/exploreUtils.js b/superset-frontend/src/explore/exploreUtils.js
index dd9d20b..68c5a6f 100644
--- a/superset-frontend/src/explore/exploreUtils.js
+++ b/superset-frontend/src/explore/exploreUtils.js
@@ -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',