Make sample indexes compatible between the unfiltered and (preview) f…

…iltered call tree summary strategy samples when using an allocation strategy (#5332)

Fixes #5331.

Let's say we allocate 100 objects, and then free the first 50 of them.
If you look at the entire thread, the `retained-native-allocations` view
will show the allocation samples for the second 50 objects (the ones
that weren't freed).
If you look at just the part of the thread where you allocated, you will
see allocation samples for all 100 objects (because none were freed in
that part of the thread).
How do you get the stack category for each of the 100 allocation
samples, especially if you've applied transforms or a JS-only filter?

You need to look at the stack of the raw nativeAllocations table.

You cannot look at the `retained-native-allocations` of the entire
thread, because that table doesn't have the stacks for the first 50
objects - it removes those samples because it knows that those objects
have been deallocated.

This commit makes it so that the unfilteredCtssSamples are just the raw
allocation tables, when an allocation-related call tree summary strategy
is selected.

And it makes it so that `extractSamplesLikeTable` always returns a table
that has compatible indexes to the `extractUnfilteredCtssSamples`, just
with some samples having a null stack.

Now the unfilteredCtssSamples can be used to look up the stack, and
therefore the original category of a sample, based on the sample index
in the filtered (or preview-filtered) CTSS samples table. We should not
create a call tree just based on the unfilteredCtssSamples because these
samples have not been filtered according to the strategy.

src/profile-logic/call-tree.js

@@ -601,8 +601,17 @@ export function getCallTree(
 }
 
 /**
- * This function takes the call tree summary strategy, and finds the appropriate data
- * structure. This can then be used by the call tree and other UI to report on the data.
+ * Returns a table with the appropriate data for the call tree summary strategy,
+ * for use by the call tree or flame graph.
+ *
+ * If the strategy is one of the native allocation strategies, the returned table
+ * will be based on thread.nativeAllocations, but with a modified
+ * stack column: Some samples will have a null stack, so that they are ignored.
+ * For example, the returned table for `native-allocations` will have null stacks
+ * for samples which are deallocations.
+ *
+ * The returned table has compatible indexes with the unfiltered table returned
+ * by extractUnfilteredSamplesLikeTable.
  */
 export function extractSamplesLikeTable(
   thread: Thread,
@@ -670,6 +679,39 @@ export function extractSamplesLikeTable(
   }
 }
 
+/**
+ * Returns the samples, jsAllocations or nativeAllocations table, without
+ * nulling out the stack for any of the samples.
+ *
+ * The stack column of the returned table can be used to look up sample
+ * categories.
+ */
+export function extractUnfilteredSamplesLikeTable(
+  thread: Thread,
+  strategy: CallTreeSummaryStrategy
+): SamplesLikeTable {
+  switch (strategy) {
+    case 'timing':
+      return thread.samples;
+    case 'js-allocations':
+      return ensureExists(
+        thread.jsAllocations,
+        'Expected the NativeAllocationTable to exist when using a "js-allocation" strategy'
+      );
+    case 'native-retained-allocations':
+    case 'native-allocations':
+    case 'native-deallocations-sites':
+    case 'native-deallocations-memory':
+      return ensureExists(
+        thread.nativeAllocations,
+        'Expected the NativeAllocationTable to exist when using a "native-allocation" strategy'
+      );
+    /* istanbul ignore next */
+    default:
+      throw assertExhaustiveCheck(strategy);
+  }
+}
+
 /**
  * This function is extremely similar to computeCallNodeLeafAndSummary,
  * but is specialized for converting sample counts into traced timing. Samples

src/profile-logic/profile-data.js

@@ -9,8 +9,6 @@ import MixedTupleMap from 'mixedtuplemap';
 import { oneLine } from 'common-tags';
 import {
   resourceTypes,
-  getEmptyUnbalancedNativeAllocationsTable,
-  getEmptyBalancedNativeAllocationsTable,
   getEmptyStackTable,
   getEmptyCallNodeTable,
   shallowCloneFrameTable,
@@ -2737,35 +2735,19 @@ export function getCategoryPairLabel(
 export function filterToAllocations(
   nativeAllocations: NativeAllocationsTable
 ): NativeAllocationsTable {
-  let newNativeAllocations;
-  if (nativeAllocations.memoryAddress) {
-    newNativeAllocations = getEmptyBalancedNativeAllocationsTable();
-    for (let i = 0; i < nativeAllocations.length; i++) {
-      const weight = nativeAllocations.weight[i];
-      if (weight > 0) {
-        newNativeAllocations.time.push(nativeAllocations.time[i]);
-        newNativeAllocations.stack.push(nativeAllocations.stack[i]);
-        newNativeAllocations.weight.push(weight);
-        newNativeAllocations.memoryAddress.push(
-          nativeAllocations.memoryAddress[i]
-        );
-        newNativeAllocations.threadId.push(nativeAllocations.threadId[i]);
-        newNativeAllocations.length++;
-      }
-    }
-  } else {
-    newNativeAllocations = getEmptyUnbalancedNativeAllocationsTable();
-    for (let i = 0; i < nativeAllocations.length; i++) {
-      const weight = nativeAllocations.weight[i];
-      if (weight > 0) {
-        newNativeAllocations.time.push(nativeAllocations.time[i]);
-        newNativeAllocations.stack.push(nativeAllocations.stack[i]);
-        newNativeAllocations.weight.push(weight);
-        newNativeAllocations.length++;
-      }
+  const filteredStackCol = nativeAllocations.stack.slice();
+  for (let i = 0; i < nativeAllocations.length; i++) {
+    const weight = nativeAllocations.weight[i];
+    if (weight <= 0) {
+      // Not an allocation, null out the sample's stack.
+      filteredStackCol[i] = null;
     }
   }
-  return newNativeAllocations;
+
+  return {
+    ...nativeAllocations,
+    stack: filteredStackCol,
+  };
 }
 
 /**
@@ -2775,35 +2757,19 @@ export function filterToAllocations(
 export function filterToDeallocationsSites(
   nativeAllocations: NativeAllocationsTable
 ): NativeAllocationsTable {
-  let newNativeAllocations;
-  if (nativeAllocations.memoryAddress) {
-    newNativeAllocations = getEmptyBalancedNativeAllocationsTable();
-    for (let i = 0; i < nativeAllocations.length; i++) {
-      const weight = nativeAllocations.weight[i];
-      if (weight < 0) {
-        newNativeAllocations.time.push(nativeAllocations.time[i]);
-        newNativeAllocations.stack.push(nativeAllocations.stack[i]);
-        newNativeAllocations.weight.push(weight);
-        newNativeAllocations.memoryAddress.push(
-          nativeAllocations.memoryAddress[i]
-        );
-        newNativeAllocations.threadId.push(nativeAllocations.threadId[i]);
-        newNativeAllocations.length++;
-      }
-    }
-  } else {
-    newNativeAllocations = getEmptyUnbalancedNativeAllocationsTable();
-    for (let i = 0; i < nativeAllocations.length; i++) {
-      const weight = nativeAllocations.weight[i];
-      if (weight < 0) {
-        newNativeAllocations.time.push(nativeAllocations.time[i]);
-        newNativeAllocations.stack.push(nativeAllocations.stack[i]);
-        newNativeAllocations.weight.push(weight);
-        newNativeAllocations.length++;
-      }
+  const filteredStackCol = nativeAllocations.stack.slice();
+  for (let i = 0; i < nativeAllocations.length; i++) {
+    const weight = nativeAllocations.weight[i];
+    if (weight >= 0) {
+      // Not a deallocation, null out the sample's stack.
+      filteredStackCol[i] = null;
     }
   }
-  return newNativeAllocations;
+
+  return {
+    ...nativeAllocations,
+    stack: filteredStackCol,
+  };
 }
 
 /**
@@ -2818,7 +2784,7 @@ export function filterToDeallocationsMemory(
 
   // This is like a Map<MemoryAddress, IndexIntoStackTable | null>;
   const memoryAddressToAllocationSite: Array<IndexIntoStackTable | null> = [];
-  const newDeallocations = getEmptyBalancedNativeAllocationsTable();
+  const filteredStackCol = nativeAllocations.stack.slice();
 
   for (
     let allocationIndex = 0;
@@ -2840,40 +2806,40 @@ export function filterToDeallocationsMemory(
       }
       memoryAddressToAllocationSite[memoryAddress] =
         nativeAllocations.stack[allocationIndex];
-      continue;
-    }
+      filteredStackCol[allocationIndex] = null;
+    } else {
+      // This is a deallocation.
+      // Lookup the previous allocation.
+      const allocationStackIndex = memoryAddressToAllocationSite[memoryAddress];
+      if (allocationStackIndex === undefined) {
+        // This deallocation doesn't match an allocation. Let's bail out.
+        filteredStackCol[allocationIndex] = null;
+      } else {
+        // This deallocation matches a previous allocation. Keep the sample and
+        // change the stack to the allocation stack.
+        filteredStackCol[allocationIndex] = allocationStackIndex;
 
-    // This is a deallocation.
-    // Lookup the previous allocation.
-    const allocationStackIndex = memoryAddressToAllocationSite[memoryAddress];
-    if (allocationStackIndex === undefined) {
-      // This deallocation doesn't match an allocation. Let's bail out.
-      continue;
+        // Remove the saved allocation
+        delete memoryAddressToAllocationSite[memoryAddress];
+      }
     }
-
-    // This deallocation matches a previous allocation.
-    newDeallocations.time.push(nativeAllocations.time[allocationIndex]);
-    newDeallocations.stack.push(allocationStackIndex);
-    newDeallocations.weight.push(bytes);
-    newDeallocations.memoryAddress.push(memoryAddress);
-    newDeallocations.threadId.push(nativeAllocations.threadId[allocationIndex]);
-    newDeallocations.length++;
-
-    // Remove the saved allocation
-    delete memoryAddressToAllocationSite[memoryAddress];
   }
 
-  return newDeallocations;
+  return {
+    ...nativeAllocations,
+    stack: filteredStackCol,
+  };
 }
 
 /**
- * Currently the native allocations naively collect allocations and deallocations.
- * There is no attempt to match up the sampled allocations with the deallocations.
- * Because of this, if a calltree were to combine both allocations and deallocations,
- * then the summary would most likely lie and not misreport leaked or retained memory.
- * For now, filter to only showing allocations or deallocations.
+ * Keeps the samples for any allocations of memory addresses for which we don't
+ * have a deallocation sample. Does not keep any deallocation samples.
+ *
+ * This is used when you want to know how much memory is still around at the
+ * end of the selected range, and where this memory was allocated.
  *
- * This function filters to only positive values.
+ * The returned table has the same length and indexes as the `nativeAllocations`
+ * argument.
  */
 export function filterToRetainedAllocations(
   nativeAllocations: BalancedNativeAllocationsTable
@@ -2883,7 +2849,7 @@ export function filterToRetainedAllocations(
   type IndexIntoAllocations = number;
   const memoryAddressToAllocation: Map<Address, IndexIntoAllocations> =
     new Map();
-  const retainedAllocation = [];
+  const filteredStackCol = nativeAllocations.stack.slice();
   for (
     let allocationIndex = 0;
     allocationIndex < nativeAllocations.length;
@@ -2896,39 +2862,27 @@ export function filterToRetainedAllocations(
 
       // Provide a map back to this index.
       memoryAddressToAllocation.set(memoryAddress, allocationIndex);
-      retainedAllocation[allocationIndex] = true;
     } else {
-      // Do not retain deallocations.
-      retainedAllocation[allocationIndex] = false;
+      // Null out the stack for deallocation samples.
+      filteredStackCol[allocationIndex] = null;
 
       // Lookup the previous allocation.
       const previousAllocationIndex =
         memoryAddressToAllocation.get(memoryAddress);
       if (previousAllocationIndex !== undefined) {
-        // This deallocation matches a previous allocation. Remove the allocation.
-        retainedAllocation[previousAllocationIndex] = false;
+        // This deallocation matches a previous allocation. Null out the
+        // corresponding allocation sample.
+        filteredStackCol[previousAllocationIndex] = null;
         // There is a match, so delete this old association.
         memoryAddressToAllocation.delete(memoryAddress);
       }
     }
   }
 
-  const newNativeAllocations = getEmptyBalancedNativeAllocationsTable();
-  for (let i = 0; i < nativeAllocations.length; i++) {
-    const weight = nativeAllocations.weight[i];
-    if (retainedAllocation[i]) {
-      newNativeAllocations.time.push(nativeAllocations.time[i]);
-      newNativeAllocations.stack.push(nativeAllocations.stack[i]);
-      newNativeAllocations.weight.push(weight);
-      newNativeAllocations.memoryAddress.push(
-        nativeAllocations.memoryAddress[i]
-      );
-      newNativeAllocations.threadId.push(nativeAllocations.threadId[i]);
-      newNativeAllocations.length++;
-    }
-  }
-
-  return newNativeAllocations;
+  return {
+    ...nativeAllocations,
+    stack: filteredStackCol,
+  };
 }
 
 /**

src/selectors/per-thread/thread.js

@@ -250,7 +250,7 @@ export function getBasicThreadSelectorsPerThread(
   const getUnfilteredCtssSamples: Selector<SamplesLikeTable> = createSelector(
     getThread,
     getCallTreeSummaryStrategy,
-    CallTree.extractSamplesLikeTable
+    CallTree.extractUnfilteredSamplesLikeTable
   );
 
   /**