Change the order of merging on about:profiler.

chrome/browser/resources/profiler.js

 // Copyright (c) 2011 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 var g_browserBridge;
 var g_mainView;
 
 // TODO(eroman): The handling of "max" across snapshots is not correct.
 // For starters the browser needs to be aware to generate new maximums.
 // Secondly, we need to take into account the "max" of intermediary snapshots,
@@ skipping 848 matching lines @@
   }
 
   /**
    * Merges the rows in |origRows|, by collapsing the columns listed in
    * |mergeKeys|. Returns an array with the merged rows (in no particular
    * order).
    *
    * If |mergeSimilarThreads| is true, then threads with a similar name will be
    * considered equivalent. For instance, "WorkerThread-1" and "WorkerThread-2"
    * will be remapped to "WorkerThread-*".
+   *
+   * If |outputAsDictionary| is false then the merged rows will be returned as a
+   * flat list. Otherwise the result will be a dictionary, where each row
+   * has a unique key.
    */
-  function mergeRows(origRows, mergeKeys, mergeSimilarThreads) {
+  function mergeRows(origRows, mergeKeys, mergeSimilarThreads,
+                     outputAsDictionary) {
     // Define a translation function for each property. Normally we copy over
     // properties as-is, but if we have been asked to "merge similar threads" we
     // we will remap the thread names that end in a numeric suffix.
     var propertyGetterFunc;
 
     if (mergeSimilarThreads) {
       propertyGetterFunc = function(row, key) {
         var value = row[key];
         // If the property is a thread name, try to remap it.
         if (key == KEY_BIRTH_THREAD || key == KEY_DEATH_THREAD) {
           var m = /^(.*[^\d])(\d+)$/.exec(value);
           if (m)
             value = m[1] + '*';
         }
         return value;
       }
     } else {
       propertyGetterFunc = function(row, key) { return row[key]; };
     }
 
     // Determine which sets of properties a row needs to match on to be
     // considered identical to another row.
     var identityKeys = IDENTITY_KEYS.slice(0);
     deleteValuesFromArray(identityKeys, mergeKeys);
 
     // Set |aggregateKeys| to everything else, since we will be aggregating
     // their value as part of the merge.
     var aggregateKeys = ALL_KEYS.slice(0);
     deleteValuesFromArray(aggregateKeys, IDENTITY_KEYS);
+    deleteValuesFromArray(aggregateKeys, mergeKeys);
 
     // Group all the identical rows together, bucketed into |identicalRows|.
     var identicalRows =
         bucketIdenticalRows(origRows, identityKeys, propertyGetterFunc);
 
-    var mergedRows = [];
+    var mergedRows = outputAsDictionary ? {} : [];
 
     // Merge the rows and save the results to |mergedRows|.
     for (var k in identicalRows) {
       // We need to smash the list |l| down to a single row...
       var l = identicalRows[k];
 
       var newRow = [];
-      mergedRows.push(newRow);
+
+      if (outputAsDictionary) {
+        mergedRows[k] = newRow;
+      } else {
+        mergedRows.push(newRow);
+      }
 
       // Copy over all the identity columns to the new row (since they
       // were the same for each row matched).
       for (var i = 0; i < identityKeys.length; ++i)
         newRow[identityKeys[i]] = propertyGetterFunc(l[0], identityKeys[i]);
 
       // Compute aggregates for the other columns.
       var aggregates = initializeAggregates(aggregateKeys);
 
       // Feed the rows to the aggregators.
       for (var i = 0; i < l.length; ++i)
         consumeAggregates(aggregates, l[i]);
 
       // Suck out the data generated by the aggregators.
       for (var aggregateKey in aggregates)
         newRow[aggregateKey] = aggregates[aggregateKey].getValue();
     }
 
     return mergedRows;
   }
 
   /**
-   * Takes two flat lists data1 and data2, and returns a new flat list which
+   * Takes two dictionaries data1 and data2, and returns a new flat list which
    * represents the difference between them. The exact meaning of "difference"
    * is column specific, but for most numeric fields (like the count, or total
    * time), it is found by subtracting.
    *
-   * TODO(eroman): Some of this code is duplicated from mergeRows().
+   * Rows in data1 and data2 are expected to use the same scheme for the keys.
+   * In other words, data1[k] is considered the analagous row to data2[k].
    */
-  function subtractSnapshots(data1, data2) {
+  function subtractSnapshots(data1, data2, columnsToExclude) {

[jar (doing other things), 2011/12/06 01:54:36]

nit: rather than data1, and data2, perhaps data_new and data_old would make it
clearer which was being subtracted from the which.  For instance, we are
guaranteed than all keys found in data_old also exist in data_new (even if they
have value zero in many fields).  

[eroman, 2011/12/06 19:21:25]

I'll leave this to a follow-up change (since it touches quite a few lines).

     // These columns are computed from the other columns. We won't bother
     // diffing/aggregating these, but rather will derive them again from the
     // final row.
     var COMPUTED_AGGREGATE_KEYS = [KEY_AVG_QUEUE_TIME, KEY_AVG_RUN_TIME];
 
     // These are the keys which determine row equality. Since we are not doing
     // any merging yet at this point, it is simply the list of all identity
     // columns.
-    var identityKeys = IDENTITY_KEYS;
+    var identityKeys = IDENTITY_KEYS.slice(0);
+    deleteValuesFromArray(identityKeys, columnsToExclude);
 
     // The columns to compute via aggregation is everything else.
     var aggregateKeys = ALL_KEYS.slice(0);
     deleteValuesFromArray(aggregateKeys, IDENTITY_KEYS);
     deleteValuesFromArray(aggregateKeys, COMPUTED_AGGREGATE_KEYS);
-
-    // Group all the identical rows for each list together.
-    var propertyGetterFunc = function(row, key) { return row[key]; };
-    var identicalRows1 =
-        bucketIdenticalRows(data1, identityKeys, propertyGetterFunc);
-    var identicalRows2 =
-        bucketIdenticalRows(data2, identityKeys, propertyGetterFunc);
+    deleteValuesFromArray(aggregateKeys, columnsToExclude);
 
     var diffedRows = [];
 
-    for (var k in identicalRows2) {
-      var rows2 = identicalRows2[k];
-      var rows1 = identicalRows1[k];
-      if (rows1 == undefined)
-        rows1 = [];
+    for (var rowId in data2) {
+      var row1 = data1[rowId];
+      var row2 = data2[rowId];
 
       var newRow = [];
 
       // Copy over all the identity columns to the new row (since they
       // were the same for each row matched).
       for (var i = 0; i < identityKeys.length; ++i)
-        newRow[identityKeys[i]] = propertyGetterFunc(rows2[0], identityKeys[i]);
+        newRow[identityKeys[i]] = row2[identityKeys[i]];
 
-      // The raw data for each snapshot *may* have contained duplicate rows, so
-      // smash them down into a single row using our aggregation functions.
-      var aggregates1 = initializeAggregates(aggregateKeys);
-      var aggregates2 = initializeAggregates(aggregateKeys);
-      for (var i = 0; i < rows1.length; ++i)
-        consumeAggregates(aggregates1, rows1[i]);
-      for (var i = 0; i < rows2.length; ++i)
-        consumeAggregates(aggregates2, rows2[i]);
+      // Diff the two rows.
+      if (row1) {
+        for (var i = 0; i < aggregateKeys.length; ++i) {
+          var aggregateKey = aggregateKeys[i];
+          var a = row1[aggregateKey];
+          var b = row2[aggregateKey];
 
-      // Finally, diff the two merged rows.
-      for (var aggregateKey in aggregates2) {
-        var a = aggregates1[aggregateKey].getValue();
-        var b = aggregates2[aggregateKey].getValue();
-
-        var diffFunc =  KEY_PROPERTIES[aggregateKey].diff;
-        newRow[aggregateKey] = diffFunc(a, b);
+          var diffFunc =  KEY_PROPERTIES[aggregateKey].diff;
+          newRow[aggregateKey] = diffFunc(a, b);
+        }
+      } else {
+        // If the the row doesn't appear in snapshot1, then there is nothing to
+        // diff, so just copy row2 as is.
+        for (var i = 0; i < aggregateKeys.length; ++i) {
+          var aggregateKey = aggregateKeys[i];
+          newRow[aggregateKey] = row2[aggregateKey];
+        }
       }
 
       if (newRow[KEY_COUNT] == 0) {
         // If a row's count has gone to zero, it means there were no new
         // occurrences of it in the second snapshot, so remove it.
         continue;
       }
 
-      // Since we excluded the averages during diffing phase, re-compute them
-      // using the diffed totals.
+      // Since we excluded the averages during the diffing phase, re-compute
+      // them using the diffed totals.
       computeDataRowAverages(newRow);
       diffedRows.push(newRow);
     }
 
     return diffedRows;
   }
 
   // --------------------------------------------------------------------------
   // HTML drawing code
   // --------------------------------------------------------------------------
@@ skipping 209 matching lines @@
 
         // Add our computed properties.
         augmentDataRow(newRow);
 
         snapshot.flatData.push(newRow);
       }
 
       if (!arrayToSet(this.getSelectedSnapshotIndexes_())[snapshotIndex]) {
         // Optimization: If this snapshot is not a data dependency for the
         // current display, then don't bother updating anything.
         return;

[jar (doing other things), 2011/12/06 01:54:36]

This may be a dangerous optimization. I think it is ok... but I doubt it is
worth it.  I'm not clear if it is correct for example if a belated snapshot
arrives, and is an intermediare of the selected pair of snapshots.

I was especially worried given the outstanding bug on the Mac (why I'm
commenting).

[eroman, 2011/12/06 19:21:25]

I'll leave this to a follow-up change (since I want to keep this change focused
on the merge refactoring for simplicity).

It is safe with the current code, but I agree once we are doing min/max for
snapshots we will need to re-consider every snapshot between a range.

       }
 
       // We may end up calling addDataToSnapshot_() repeatedly (once for each
       // process). To avoid this from slowing us down we do bulk updates on a
       // timer.
-      this.updateFlatDataSoon_();
+      this.updateMergedDataSoon_();
     },
 
-    updateFlatDataSoon_: function() {
-      if (this.updateFlatDataPending_) {
+    updateMergedDataSoon_: function() {

[eroman, 2011/11/23 22:33:28]

this is just a rename "flatData" -> "mergedData"

+      if (this.updateMergedDataPending_) {
         // If a delayed task has already been posted to re-merge the data,
         // then we don't need to do anything extra.
         return;
       }
 
-      // Otherwise schedule updateFlatData_() to be called later. We want it to
-      // be called no more than once every PROCESS_DATA_DELAY_MS milliseconds.
+      // Otherwise schedule updateMergedData_() to be called later. We want it
+      // to be called no more than once every PROCESS_DATA_DELAY_MS
+      // milliseconds.
 
-      if (this.lastUpdateFlatDataTime_ == undefined)
-        this.lastUpdateFlatDataTime_ = 0;
+      if (this.lastUpdateMergedDataTime_ == undefined)
+        this.lastUpdateMergedDataTime_ = 0;
 
-      var timeSinceLastMerge = getTimeMillis() - this.lastUpdateFlatDataTime_;
+      var timeSinceLastMerge = getTimeMillis() - this.lastUpdateMergedDataTime_;
       var timeToWait = Math.max(0, PROCESS_DATA_DELAY_MS - timeSinceLastMerge);
 
       var functionToRun = function() {
         // Do the actual update.
-        this.updateFlatData_();
+        this.updateMergedData_();
         // Keep track of when we last ran.
-        this.lastUpdateFlatDataTime_ = getTimeMillis();
-        this.updateFlatDataPending_ = false;
+        this.lastUpdateMergedDataTime_ = getTimeMillis();
+        this.updateMergedDataPending_ = false;
       }.bind(this);
 
-      this.updateFlatDataPending_ = true;
+      this.updateMergedDataPending_ = true;
       window.setTimeout(functionToRun, timeToWait);
     },
 
     /**
      * Returns a list of the currently selected snapshots. This list is
      * guaranteed to be of length 1 or 2.
      */
     getSelectedSnapshotIndexes_: function() {
       var indexes = this.getSelectedSnapshotBoxes_();
       for (var i = 0; i < indexes.length; ++i)
         indexes[i] = indexes[i].__index;
       return indexes;
     },
 
     /**
      * Same as getSelectedSnapshotIndexes_(), only it returns the actual
      * checkbox input DOM nodes rather than the snapshot ID.
      */
     getSelectedSnapshotBoxes_: function() {
       // Figure out which snaphots to use for our data.
       var boxes = [];
       for (var i = 0; i < this.snapshots_.length; ++i) {
         var box = this.getSnapshotCheckbox_(i);
         if (box.checked)
           boxes.push(box);
       }
       return boxes;
     },
 
-    /**
-     * This function should be called any time a snapshot dependency for what is
-     * being displayed on the screen has changed. It will re-calculate the
-     * difference between the two snapshots and update flatData_.

[jar (doing other things), 2011/12/06 01:54:36]

nit: It would be good to update the comment, rather than discarding it.

[eroman, 2011/12/06 19:21:25]

Done.

(I initially omitted the comment since I felt the new function name adequately
explained things.)

-     */
-    updateFlatData_: function() {
+    updateSnapshotSelectionSummaryDiv_: function() {
       var summaryDiv = $(SNAPSHOT_SELECTION_SUMMARY_ID);
 
       var selectedSnapshots = this.getSelectedSnapshotIndexes_();
       if (selectedSnapshots.length == 1) {
-        // If only one snapshot is chosen then we will display that snapshot's
-        // data in its entirety.
-        this.flatData_ = this.snapshots_[selectedSnapshots[0]].flatData;
-
         // Don't bother displaying any text when just 1 snapshot is selected,
         // since it is obvious what this should do.
         summaryDiv.innerText = '';
       } else if (selectedSnapshots.length == 2) {
         // Otherwise if two snapshots were chosen, show the difference between
         // them.
         var snapshot1 = this.snapshots_[selectedSnapshots[0]];
         var snapshot2 = this.snapshots_[selectedSnapshots[1]];
 
-        this.flatData_ =
-            subtractSnapshots(snapshot1.flatData, snapshot2.flatData);
-
         var timeDeltaInSeconds =
             ((snapshot2.time - snapshot1.time) / 1000).toFixed(0);
 
         // Explain that what is being shown is the difference between two
         // snapshots.
         summaryDiv.innerText =
             'Showing the difference between snapshots #' +
             selectedSnapshots[0] + ' and #' +
             selectedSnapshots[1] + ' (' + timeDeltaInSeconds +
             ' seconds worth of data)';
       } else {
         // This shouldn't be possible...
         throw 'Unexpected number of selected snapshots';
       }
-
-      // Recompute mergedData_ (since it is derived from flatData_)
-      this.updateMergedData_();
     },
 
     updateMergedData_: function() {
-      // Recompute mergedData_.
-      this.mergedData_ = mergeRows(this.flatData_,
-                                   this.getMergeColumns_(),
-                                   this.shouldMergeSimilarThreads_());
+      // Retrieve the merge options.
+      var mergeColumns = this.getMergeColumns_();
+      var shouldMergeSimilarThreads = this.shouldMergeSimilarThreads_();
+
+      var selectedSnapshots = this.getSelectedSnapshotIndexes_();
+
+      // We do merges a bit differently depending if we are displaying the diffs
+      // between two snapshots, or just displaying a single snapshot.
+      if (selectedSnapshots.length == 1) {
+        var snapshot = this.snapshots_[selectedSnapshots[0]];
+        this.mergedData_ = mergeRows(snapshot.flatData,
+                                     mergeColumns,
+                                     shouldMergeSimilarThreads,
+                                     false);
+
+      } else if (selectedSnapshots.length == 2) {
+        var snapshot1 = this.snapshots_[selectedSnapshots[0]];
+        var snapshot2 = this.snapshots_[selectedSnapshots[1]];
+
+        // Merge the data for snapshot1.
+        var mergedRows1 = mergeRows(snapshot1.flatData,
+                                    mergeColumns,
+                                    shouldMergeSimilarThreads,
+                                    true);
+
+        // Merge the data for snapshot2.
+        var mergedRows2 = mergeRows(snapshot2.flatData,
+                                    mergeColumns,
+                                    shouldMergeSimilarThreads,
+                                    true);
+
+        // Do a diff between the two snapshots.
+        this.mergedData_ = subtractSnapshots(mergedRows1,
+                                             mergedRows2,
+                                             mergeColumns);
+      } else {
+        throw 'Unexpected number of selected snapshots';
+      }
 
       // Recompute filteredData_ (since it is derived from mergedData_)
       this.updateFilteredData_();
     },
 
     updateFilteredData_: function() {
       // Recompute filteredData_.
       this.filteredData_ = [];
       var filterFunc = this.getFilterFunction_();
       for (var i = 0; i < this.mergedData_.length; ++i) {
@@ skipping 311 matching lines @@
       //     constants.)
       // (2) We "augment" each row by adding some extra computed columns
       //     (like averages).
       // (3) The rows are merged using current merge settings.
       // (4) The rows that don't match current search expression are
       //     tossed out.
       // (5) The rows are organized into "groups" based on current settings,
       //     and aggregate values are computed for each resulting group.
       // (6) The rows within each group are sorted using current settings.
       // (7) The grouped rows are drawn to the screen.
-      this.flatData_ = [];
       this.mergedData_ = [];
       this.filteredData_ = [];
       this.groupedData_ = {};
       this.sortedGroupKeys_ = [];
 
       this.groupDisplaySettings_ = {};
 
       this.fillSelectionCheckboxes_($(COLUMN_TOGGLES_CONTAINER_ID));
       this.fillMergeCheckboxes_($(COLUMN_MERGE_TOGGLES_CONTAINER_ID));
 
@@ skipping 95 matching lines @@
           checked[i].checked = false;
         checked.length = 2;
       }
 
       // We should always have at least 1 selection. Prevent the user from
       // unselecting the final box.
       if (checked.length == 0)
         event.target.checked = true;
 
       this.updateSnapshotCheckboxStyling_();
+      this.updateSnapshotSelectionSummaryDiv_();
 
-      // Recompute flatData_ (since it is derived from selected snapshots).
-      this.updateFlatData_();
+      // Recompute mergedData_ (since it is derived from selected snapshots).
+      this.updateMergedData_();
     },
 
     fillSelectionCheckboxes_: function(parent) {
       this.selectionCheckboxes_ = {};
 
       var onChangeFunc = this.onSelectCheckboxChanged_.bind(this);
 
       for (var i = 0; i < ALL_TABLE_COLUMNS.length; ++i) {
         var key = ALL_TABLE_COLUMNS[i];
         var checkbox = addLabeledCheckbox(parent, getNameForKey(key));
@@ skipping 244 matching lines @@
           groupKey.push(entry);
         }
 
         return JSON.stringify(groupKey);
       };
     },
   };
 
   return MainView;
 })();