aqa-test-tools icon indicating copy to clipboard operation
aqa-test-tools copied to clipboard

Optimize Tabular View Code

Open piyush286 opened this issue 6 years ago • 2 comments

In the Tabular view (https://github.com/AdoptOpenJDK/openjdk-test-tools/issues/37), there are few places where the code seems to pretty costly to run. In interest of time and considering the first commit for Tabular View, we'll revisit the code for it to see how we can optimize it in order to reduce CPU usage.

For example, we are using distinct to get unique values for platforms and benchmarks. distinct is expensive to run.

Snippet from getTabularData.js

const platforms = await db.distinct("buildName", query);
const benchmarks = await db.distinct("aggregateInfo.benchmarkName", query);

For more details, please refer to https://github.com/AdoptOpenJDK/openjdk-test-tools/pull/131#issuecomment-524310606.

piyush286 avatar Aug 23 '19 14:08 piyush286

@llxia After some initial review for Tabular View, we didn't end up using the snippet with distinct mentioned above. This issue was created a few days before the final changes were merged, so we have cleaned up and optimized most things already.

Mentioning some relevant snippets below. If you have any specific suggestions on how to optimize the queries further, please mention it. Thanks!

https://github.com/AdoptOpenJDK/openjdk-test-tools/blob/69e586c031d71e49d694edd944af9e591cd10ba2/TestResultSummaryService/routes/getTabularDropdown.js#L18-L20

https://github.com/AdoptOpenJDK/openjdk-test-tools/blob/69e586c031d71e49d694edd944af9e591cd10ba2/TestResultSummaryService/routes/getTabularData.js#L28-L29

https://github.com/AdoptOpenJDK/openjdk-test-tools/blob/69e586c031d71e49d694edd944af9e591cd10ba2/TestResultSummaryService/routes/getTabularData.js#L69

piyush286 avatar Mar 03 '20 19:03 piyush286

I feel this is not only a specific function is used or not. There are lots of problems. Below are some examples:

  • Instead of writing code to filter and find the latest run, can we query on date? https://github.com/AdoptOpenJDK/openjdk-test-tools/blob/69e586c031d71e49d694edd944af9e591cd10ba2/TestResultSummaryService/routes/getTabularData.js#L47-L67

  • why loop through all info and string concat into benchmarkNVM , then split benchmarkNVM get the values back?

benchmarkNVM = testResultsObject.aggregateInfo[aggregateIndex].benchmarkName + ',' + testResultsObject.aggregateInfo[aggregateIndex].benchmarkVariant + "," + testResultsObject.aggregateInfo[aggregateIndex].metrics[metric].name;
  • lots of unused variables: https://github.com/AdoptOpenJDK/openjdk-test-tools/blob/69e586c031d71e49d694edd944af9e591cd10ba2/TestResultSummaryService/routes/getTabularDropdown.js#L8

...

Before we dive into fixing the code, I would like to take a step back and go over the design of the Tabular View and understand the use cases. To me, both client TabularView.jsx and server logic for Tabular View seem convoluted.

llxia avatar Mar 12 '20 17:03 llxia