Bug 1215617 - Filtering interface for perfherder compare

Can now filter across test name and platform, as well as only show significant changes and/or results for which we have confidence.
2015-10-22 11:05:34 -04:00 · 2015-10-22 11:05:34 -04:00 · 7f7f54f91c
--- a/ui/js/controllers/perf/compare.js
+++ b/ui/js/controllers/perf/compare.js
@ -188,7 +188,6 @@ perf.controller('CompareResultsCtrl', [
                        cmap.detailsLink = detailsLink;
                    }
                    cmap.name = platform;
-                    cmap.hideMinorChanges = $scope.hideMinorChanges;
                    if (Object.keys($scope.compareResults).indexOf(testName) < 0)
                        $scope.compareResults[testName] = [];
                    $scope.compareResults[testName].push(cmap);
@ -220,6 +219,22 @@ perf.controller('CompareResultsCtrl', [
        $scope.dataLoading = true;
        $scope.getCompareClasses = PhCompare.getCompareClasses;

+        $scope.updateFilters = function() {
+
+            $state.transitionTo('compare', {
+                filterTest: $scope.filterOptions.testFilter,
+                filterPlatform: $scope.filterOptions.platformFilter,
+                showOnlyImportant: Boolean($scope.filterOptions.showOnlyImportant) ? undefined : 0,
+                showOnlyConfident: Boolean($scope.filterOptions.showOnlyConfident) ? undefined : 0,
+                showUnreliablePlatforms: Boolean($scope.filterOptions.showUnreliablePlatforms) ? 1 : undefined
+            }, {
+                location: true,
+                inherit: true,
+                relative: $state.$current,
+                notify: false
+            });
+        };
+
        var optionCollectionMap = {};
        var loadRepositories = ThRepositoryModel.load();
        var loadOptions = ThOptionCollectionModel.get_map().then(
@ -236,11 +251,16 @@ perf.controller('CompareResultsCtrl', [
                $scope.dataLoading = false;
                return;
            }
+            $scope.filterOptions = {
+                testFilter: $stateParams.filterTest || "",
+                platformFilter: $stateParams.filterPlatform || "",
+                showOnlyImportant: $stateParams.showOnlyImportant === undefined ||
+                    parseInt($stateParams.showOnlyImportant),
+                showOnlyConfident: $stateParams.showOnlyConfident === undefined ||
+                    parseInt($stateParams.showOnlyConfident),
+                showUnreliablePlatforms: Boolean(parseInt($stateParams.showUnreliablePlatforms))
+            };

-            // we are excluding macosx 10.10 by default for now, see: https://bugzilla.mozilla.org/show_bug.cgi?id=1201615
-            $scope.excludedPlatforms = Boolean($stateParams.showExcludedPlatforms) ?
-                [] : [ 'osx-10-10' ];
-            $scope.hideMinorChanges = Boolean($stateParams.hideMinorChanges);
            $scope.originalProject = ThRepositoryModel.getRepo(
                $stateParams.originalProject);
            $scope.newProject = ThRepositoryModel.getRepo(
@ -328,7 +348,6 @@ perf.controller('CompareSubtestResultsCtrl', [
                    }

                    cmap.name = page;
-                    cmap.hideMinorChanges = $scope.hideMinorChanges;
                    $scope.compareResults[testName].push(cmap);
                });
            });
@ -357,7 +376,6 @@ perf.controller('CompareSubtestResultsCtrl', [
                    return;
                }

-                $scope.hideMinorChanges = Boolean($stateParams.hideMinorChanges);
                $scope.originalProject = ThRepositoryModel.getRepo(
                    $stateParams.originalProject);
                $scope.newProject = ThRepositoryModel.getRepo(
--- a/ui/js/directives/perf/compare.js
+++ b/ui/js/directives/perf/compare.js
@ -2,16 +2,48 @@

 treeherder.directive(
    'phCompareTable',
-    ['PhCompare', function(PhCompare) {
+    ['PhCompare', 'phUnreliablePlatforms', function(PhCompare, phUnreliablePlatforms) {
        return {
            templateUrl: 'partials/perf/comparetable.html',
            scope: {
                titles: '=',
                compareResults: '=',
-                testList: '='
+                testList: '=',
+                testFilter: '=',
+                platformFilter: '=',
+                showOnlyImportant: '=',
+                showOnlyConfident: '=',
+                showUnreliablePlatforms: '='
            },
            link: function(scope, element, attrs) {
                scope.getCompareClasses = PhCompare.getCompareClasses;
+                function filter(item, matchText) {
+                    return !matchText || item.toLowerCase().indexOf(matchText.toLowerCase()) > (-1);
+                }
+                scope.filterTest = function(item) {
+                    return filter(item, scope.testFilter);
+                };
+                scope.filterPlatform = function(result) {
+                    return filter(result.name, scope.platformFilter) &&
+                        (!scope.showOnlyImportant || result.isMeaningful) &&
+                        (!scope.showOnlyConfident || result.isConfident) &&
+                        (scope.showUnreliablePlatforms || !_.contains(
+                            phUnreliablePlatforms, result.name));
+                };
+                function updateFilteredTestList() {
+                    scope.filteredTestList = _.filter(_.keys(scope.compareResults), function(testName) {
+                        return scope.filterTest(scope.titles[testName]) &&
+                            _.any(_.map(scope.compareResults[testName], function(result) {
+                                return scope.filterPlatform(result);
+                            }));
+                    }).sort();
+                }
+                scope.$watchGroup(['testFilter', 'platformFilter',
+                                   'showOnlyImportant', 'showOnlyConfident',
+                                   'showUnreliablePlatforms'], function() {
+                    updateFilteredTestList();
+                });
+                updateFilteredTestList();
            }
        };
    }]);
--- a/ui/js/perf.js
+++ b/ui/js/perf.js
@ -181,8 +181,6 @@ perf.factory('PhSeries', ['$http', 'thServiceDomain', function($http, thServiceD
                    // We don't generate number for tp5n, this is xperf and we collect counters
                    if (_.contains(series.name, "tp5n"))
                        return;
-                    if (_.contains(userOptions.excludedPlatforms, series.platform))
-                        return;

                    seriesList.push(series);

@ -268,7 +266,6 @@ perf.factory('PhCompare', [ '$q', '$http', 'thServiceDomain', 'PhSeries',

                                return {
                                    getCompareClasses: function(cr, type) {
-                                        if (cr.hideMinorChanges && !cr.isMeaningful) return 'subtest-empty';
                                        if (cr.isEmpty) return 'subtest-empty';
                                        if (type == 'row' && cr.highlightedTest) return 'active subtest-highlighted';
                                        if (type == 'row') return '';
@ -394,6 +391,13 @@ perf.factory('PhCompare', [ '$q', '$http', 'thServiceDomain', 'PhSeries',
                                        cmap.isRegression = (cmap.className == 'compare-regression');
                                        cmap.isImprovement = (cmap.className == 'compare-improvement');
                                        cmap.isMeaningful = (cmap.className != "");
+                                        cmap.isComplete = (cmap.originalRuns.length &&
+                                                           cmap.newRuns.length);
+                                        cmap.isConfident = ((cmap.originalRuns.length > 1 &&
+                                                             cmap.newRuns.length > 1 &&
+                                                             cmap.confidenceText === 'high') ||
+                                                            (cmap.originalRuns.length >= 6 &&
+                                                             cmap.newRuns.length >= 6));

                                        return cmap;
                                    },
--- a/ui/js/perfapp.js
+++ b/ui/js/perfapp.js
@ -12,11 +12,11 @@ perf.config(function($compileProvider, $stateProvider, $urlRouterProvider) {
        controller: 'GraphsCtrl'
    }).state('compare', {
        templateUrl: 'partials/perf/comparectrl.html',
-        url: '/compare?originalProject&originalRevision&newProject&newRevision&hideMinorChanges&showExcludedPlatforms',
+        url: '/compare?originalProject&originalRevision&newProject&newRevision&hideMinorChanges&showExcludedPlatforms&filterTest&filterPlatform&showOnlyImportant&showOnlyConfident&showUnreliablePlatforms',
        controller: 'CompareResultsCtrl'
    }).state('comparesubtest', {
        templateUrl: 'partials/perf/comparesubtestctrl.html',
-        url: '/comparesubtest?originalProject&originalRevision&newProject&newRevision&originalSignature&newSignature&hideMinorChanges',
+        url: '/comparesubtest?originalProject&originalRevision&newProject&newRevision&originalSignature&newSignature',
        controller: 'CompareSubtestResultsCtrl'
    }).state('comparechooser', {
        title: 'Perfherder Compare',
--- a/ui/js/values.js
+++ b/ui/js/values.js
@ -163,6 +163,11 @@ treeherder.value("phTimeRanges", [

 treeherder.value("phDefaultTimeRangeValue", 604800);

+// we are excluding macosx 10.10 by default for now, see: https://bugzilla.mozilla.org/show_bug.cgi?id=1201615
+treeherder.value("phUnreliablePlatforms", [
+    'osx-10-10'
+]);
+
 treeherder.value("thJobNavSelectors",
    {
        ALL_JOBS: {
--- a/ui/partials/perf/comparectrl.html
+++ b/ui/partials/perf/comparectrl.html
@ -14,10 +14,45 @@
        <strong>tests with no results:</strong>
        {{testNoResults}}
      </div>
+      <h4>Filters</h3>
+      <form class="form-inline">
+        <div class="form-group">
+          <label for="filter-test">Test</label>
+            <input id="filter-test" type="text" class="form-control" ng-model="filterOptions.testFilter" placeholder="e.g. tp5o opt" ng-change="updateFilters()"/>
+        </div>
+        <div class="form-group">
+          <label for="filter-platform">Platform</label>
+            <input id="filter-platform" type="text" class="form-control" ng-model="filterOptions.platformFilter" placeholder="e.g. linux64" ng-change="updateFilters()"/>
+        </div>
+        <div class="checkbox" tooltip="Non-trivial changes (1.5%+)">
+          <label>
+            <input type="checkbox" ng-model="filterOptions.showOnlyImportant" ng-change="updateFilters()"/>
+            Show only important changes
+          </label>
+        </div>
+        <div class="checkbox" tooltip="At least 6 datapoints OR 2+ datapoints and a large difference">
+          <label>
+            <input type="checkbox" ng-model="filterOptions.showOnlyConfident" ng-change="updateFilters()"/>
+            Hide uncertain results
+          </label>
+        </div>
+        <div class="checkbox" tooltip="Platforms which don't produce consistent / reliable data -- beware!">
+          <label>
+            <input type="checkbox" ng-model="filterOptions.showUnreliablePlatforms" ng-change="updateFilters()"/>
+            Show unreliable platforms
+          </label>
+        </div>
+      </form>
+      <hr/>
      <ph-compare-table
         titles="titles"
         test-list="testList"
-         compare-results="compareResults">
+         compare-results="compareResults"
+         test-filter="filterOptions.testFilter"
+         platform-filter="filterOptions.platformFilter"
+         show-only-important="filterOptions.showOnlyImportant"
+         show-only-confident="filterOptions.showOnlyConfident"
+         show-unreliable-platforms="filterOptions.showUnreliablePlatforms">
      </ph-compare-table>
    </div>
  </div>
--- a/ui/partials/perf/comparesubtestctrl.html
+++ b/ui/partials/perf/comparesubtestctrl.html
@ -14,7 +14,12 @@
    <ph-compare-table
       titles="titles"
       test-list="testList"
-       compare-results="compareResults">
+       compare-results="compareResults"
+       test-filter=""
+       platform-filter=""
+       show-only-important="0"
+       show-only-confident="0"
+       show-unreliable-platforms="1">
    </ph-compare-table>
  </div>
 </div>
--- a/ui/partials/perf/comparetable.html
+++ b/ui/partials/perf/comparetable.html
@ -1,4 +1,4 @@
-<table class="table compare-table" style="table-layout: fixed;"  ng-repeat="testName in testList">
+<table class="table compare-table" style="table-layout: fixed;" ng-repeat="testName in filteredTestList">
  <tbody>
    <tr class="subtest-header">
      <!-- Manually specify table widths because it's just easier this way -->
@ -12,7 +12,7 @@
      <td class="num-runs" style="width: 80px"># Runs</td>
      <td class="test-warning" style="width: 30px"><!-- warning if not enough --></td>
    </tr>
-    <tr ng-class="getCompareClasses(compareResult, 'row')" ng-repeat="compareResult in compareResults[testName] | orderBy: 'name'">
+    <tr ng-class="getCompareClasses(compareResult, 'row')" ng-repeat="compareResult in compareResults[testName] | orderBy: 'name'" ng-show="filterPlatform(compareResult)">
      <td class="test-title">{{compareResult.name}}
        <span ng-if="compareResult.detailsLink">
          (<a ng-href="{{compareResult.detailsLink}}" ng-show="compareResult.detailsLink">subtests</a>)
@ -85,16 +85,11 @@
               for confidence -- if we've done that many and we're still
               not confident in a change, more are unlikely to help)
          -->
-        <span ng-if="compareResult.originalRuns.length && compareResult.newRuns.length &&
-                     ((compareResult.originalRuns.length < 2 ||
-                       compareResult.newRuns.length < 2) ||
-                      (compareResult.originalRuns.length < 6 &&
-                       compareResult.newRuns.length < 6 &&
-                       compareResult.confidenceText !== 'high'))"
-                class="glyphicon glyphicon-warning-sign text-warning"
-                tooltip="More base / new runs recommended for increased confidence in comparison"
-                tooltip-placement="left"
-                style="cursor:default"></span>
+        <span ng-if="compareResult.isComplete && !compareResult.isConfident"
+              class="glyphicon glyphicon-warning-sign text-warning"
+              tooltip="More base / new runs recommended for increased confidence in comparison"
+              tooltip-placement="left"
+              style="cursor:default"></span>
      </td>
    </tr>
  </tbody>