Add a "Merge unaccounted native frames" transform.

This makes it easier to analyze profiles with JIT frames.

This transform gets rid of 0x12345 frames in the call tree - but only
those that haven't been accounted to a shared library, i.e. only frames
which we consider to be "probably JIT" frames. This transform does not
merge unsymbolicated frames for which we know what library they belong to.

Ideally, we wouldn't have those stray hex addresses in the profiles;
either Firefox would filter them out during stack merging, or it should
give us the correct memory ranges for JIT functions so that we can account
these addresses to the right JS function / JIT trampoline etc.
But it's not doing that correctly today.

https://bugzilla.mozilla.org/show_bug.cgi?id=1463559 is one of the bugs
about this.

For now it seems easier to just offer a transform which gets rid of these
stray JIT address frames. This new transform doesn't really clutter
anything up; users will only know about it if they right-click such a
frame / call node.

Even if Firefox starts giving us better information for JIT frames, this
transform can still be useful. For example, you could see this type of
stack frame in profiles that are gathered with samply, on applications
that use other JITs or applications which don't have Jitdump
instrumentation enabled.
Another use case would be if Firefox uses a system library that makes
use of jitting, for example a software OpenGL implementation. Those
libraries can't be expected to expose their JIT details to our profiler.

Author: mstange

locales/en-US/app.ftl

@@ -65,6 +65,12 @@ CallNodeContextMenu--transform-merge-call-node = Merge node only
         function’s node that called it. It only removes the function from that
         specific part of the tree. Any other places the function was called from
         will remain in the profile.
+CallNodeContextMenu--transform-merge-unaccounted-native-functions = Merge unaccounted native frames
+    .title =
+        Removes stack frames such as unsymbolicated JIT frames from the call tree,
+        and make their callers absorb their cost.
+        Specifically, this transform merges any native stack frames which were
+        not accounted to a shared library.
 
 # This is used as the context menu item title for "Focus on function" and "Focus
 # on function (inverted)" transforms.
@@ -1081,6 +1087,10 @@ TransformNavigator--merge-call-node = Merge Node: { $item }
 #   $item (String) - Name of the function that transform applied to.
 TransformNavigator--merge-function = Merge: { $item }
 
+# "Merge unaccounted native frames" transform.
+# See: https://profiler.firefox.com/docs/#/./guide-filtering-call-trees?id=merge
+TransformNavigator--merge-unaccounted-native-functions = Merge unaccounted native
+
 # "Drop function" transform.
 # See: https://profiler.firefox.com/docs/#/./guide-filtering-call-trees?id=drop
 # Variables:

src/components/shared/CallNodeContextMenu.js

@@ -13,6 +13,7 @@ import { parseFileNameFromSymbolication } from 'firefox-profiler/utils/special-p
 import {
   funcHasDirectRecursiveCall,
   funcHasRecursiveCall,
+  isUnaccountedNativeFunction,
 } from 'firefox-profiler/profile-logic/transforms';
 import { getFunctionName } from 'firefox-profiler/profile-logic/function-info';
 import {
@@ -333,6 +334,11 @@ class CallNodeContextMenuImpl extends React.PureComponent<Props> {
           funcIndex: selectedFunc,
         });
         break;
+      case 'merge-unaccounted-native-functions':
+        addTransformToStack(threadsKey, {
+          type: 'merge-unaccounted-native-functions',
+        });
+        break;
       case 'drop-function':
         addTransformToStack(threadsKey, {
           type: 'drop-function',
@@ -484,7 +490,7 @@ class CallNodeContextMenuImpl extends React.PureComponent<Props> {
 
     const {
       callNodeIndex,
-      thread: { funcTable },
+      thread: { funcTable, stringTable },
       callNodeInfo,
     } = rightClickedCallNodeInfo;
 
@@ -504,6 +510,11 @@ class CallNodeContextMenuImpl extends React.PureComponent<Props> {
     const fileName =
       filePath &&
       parseFileNameFromSymbolication(filePath).path.match(/[^\\/]+$/)?.[0];
+    const isProbablyJIT = isUnaccountedNativeFunction(
+      funcIndex,
+      funcTable,
+      stringTable
+    );
     return (
       <>
         {fileName ? (
@@ -545,6 +556,19 @@ class CallNodeContextMenuImpl extends React.PureComponent<Props> {
               content: 'Merge node only',
             })}
 
+        {isProbablyJIT
+          ? this.renderTransformMenuItem({
+              l10nId:
+                'CallNodeContextMenu--transform-merge-unaccounted-native-functions',
+              shortcut: 'J',
+              icon: 'Merge',
+              onClick: this._handleClick,
+              transform: 'merge-unaccounted-native-functions',
+              title: '',
+              content: 'Merge unaccounted native frames',
+            })
+          : null}
+
         {this.renderTransformMenuItem({
           l10nId: inverted
             ? 'CallNodeContextMenu--transform-focus-function-inverted'

src/profile-logic/transforms.js

@@ -65,6 +65,7 @@ const SHORT_KEY_TO_TRANSFORM: { [string]: TransformType } = {};
   'focus-category',
   'merge-call-node',
   'merge-function',
+  'merge-unaccounted-native-functions',
   'drop-function',
   'collapse-resource',
   'collapse-direct-recursion',
@@ -91,6 +92,9 @@ const SHORT_KEY_TO_TRANSFORM: { [string]: TransformType } = {};
     case 'merge-function':
       shortKey = 'mf';
       break;
+    case 'merge-unaccounted-native-functions':
+      shortKey = 'munfs';
+      break;
     case 'drop-function':
       shortKey = 'df';
       break;
@@ -235,6 +239,12 @@ export function parseTransforms(transformString: string): TransformStack {
         }
         break;
       }
+      case 'merge-unaccounted-native-functions': {
+        transforms.push({
+          type: 'merge-unaccounted-native-functions',
+        });
+        break;
+      }
       case 'focus-category': {
         // e.g. "fg-3"
         const [, categoryRaw] = tuple;
@@ -348,6 +358,8 @@ export function stringifyTransforms(transformStack: TransformStack): string {
         case 'collapse-function-subtree':
         case 'focus-function':
           return `${shortKey}-${transform.funcIndex}`;
+        case 'merge-unaccounted-native-functions':
+          return shortKey;
         case 'focus-category':
           return `${shortKey}-${transform.category}`;
         case 'collapse-resource':
@@ -432,6 +444,13 @@ export function getTransformLabelL10nIds(
       }
     }
 
+    if (transform.type === 'merge-unaccounted-native-functions') {
+      return {
+        l10nId: 'TransformNavigator--merge-unaccounted-native-functions',
+        item: '',
+      };
+    }
+
     // Lookup function name.
     let funcIndex;
     switch (transform.type) {
@@ -517,6 +536,12 @@ export function applyTransformToCallNodePath(
       return _mergeNodeInCallNodePath(transform.callNodePath, callNodePath);
     case 'merge-function':
       return _mergeFunctionInCallNodePath(transform.funcIndex, callNodePath);
+    case 'merge-unaccounted-native-functions':
+      return _mergeUnaccountedNativeFunctionsInCallNodePath(
+        callNodePath,
+        transformedThread.funcTable,
+        transformedThread.stringTable
+      );
     case 'drop-function':
       return _dropFunctionInCallNodePath(transform.funcIndex, callNodePath);
     case 'collapse-resource':
@@ -588,6 +613,38 @@ function _mergeFunctionInCallNodePath(
   return callNodePath.filter((nodeFunc) => nodeFunc !== funcIndex);
 }
 
+function _mergeUnaccountedNativeFunctionsInCallNodePath(
+  callNodePath: CallNodePath,
+  funcTable: FuncTable,
+  stringTable: UniqueStringArray
+): CallNodePath {
+  return callNodePath.filter((nodeFunc) =>
+    isUnaccountedNativeFunction(nodeFunc, funcTable, stringTable)
+  );
+}
+
+// Returns true if funcIndex is probably a JIT frame outside of any known JIT
+// address mappings.
+export function isUnaccountedNativeFunction(
+  funcIndex: IndexIntoFuncTable,
+  funcTable: FuncTable,
+  stringTable: UniqueStringArray
+): boolean {
+  if (funcTable.resource[funcIndex] !== -1) {
+    // This function has a resource. That means it's not "unaccounted".
+    return false;
+  }
+  if (funcTable.isJS[funcIndex]) {
+    // This function is a JS function. That means it's not a "native" function.
+    return false;
+  }
+  // Ok, so now we either have a native function without a library (otherwise it
+  // would have a "lib" resource), or we have a label frame. Assume that label
+  // frames don't start with "0x".
+  const locationString = stringTable.getString(funcTable.name[funcIndex]);
+  return locationString.startsWith('0x');
+}
+
 function _dropFunctionInCallNodePath(
   funcIndex: IndexIntoFuncTable,
   callNodePath: CallNodePath
@@ -859,13 +916,96 @@ export function mergeFunction(
   });
 }
 
+/**
+ * Returns a Uint8Array filled with zeros and ones.
+ * result[funcIndex] === 1 iff isUnaccountedNativeFunction(funcIndex)
+ */
+function getUnaccountedNativeFunctions(thread: Thread): Uint8Array {
+  const { funcTable, stringTable } = thread;
+  const funcCount = funcTable.length;
+  const { isJS: funcIsJS, resource: funcResource, name: funcName } = funcTable;
+  const isUnaccountedNativeFunctionArr = new Uint8Array(funcTable.length);
+  for (let i = 0; i < funcCount; i++) {
+    if (funcIsJS[i] || funcResource[i] !== -1) {
+      continue;
+    }
+    const locationString = stringTable.getString(funcName[i]);
+    if (locationString.startsWith('0x')) {
+      isUnaccountedNativeFunctionArr[i] = 1;
+    }
+  }
+  return isUnaccountedNativeFunctionArr;
+}
+
+export function mergeUnaccountedNativeFunctions(thread: Thread): Thread {
+  const isUnaccountedNativeFunctionArr = getUnaccountedNativeFunctions(thread);
+  return mergeFunctions(thread, isUnaccountedNativeFunctionArr);
+}
+
+export function mergeFunctions(
+  thread: Thread,
+  shouldMergeFunction: Uint8Array
+): Thread {
+  const { stackTable, frameTable } = thread;
+
+  // A map oldStack -> newStack+1, implemented as a Uint32Array for performance.
+  // If newStack+1 is zero it means "null", i.e. this stack was filtered out.
+  // Typed arrays are initialized to zero, which we interpret as null.
+  //
+  // For each old stack, the new stack is computed as follows:
+  //  - If the old stack's function is not funcIndexToMerge, then the new stack
+  //    is the same as the old stack.
+  //  - If the old stack's function is funcIndexToMerge, then the new stack is
+  //    the closest ancestor whose func is not funcIndexToMerge, or null if no
+  //    such ancestor exists.
+  //
+  // We only compute a new prefix column; the other columns are copied from the
+  // old stack table. The skipped stacks are "orphaned"; they'll still be present
+  // in the new stack table but not referenced by samples or other stacks.
+  const oldStackToNewStackPlusOne = new Uint32Array(stackTable.length);
+
+  const stackTableFrameCol = stackTable.frame;
+  const frameTableFuncCol = frameTable.func;
+  const oldPrefixCol = stackTable.prefix;
+  const newPrefixCol = new Array(stackTable.length);
+
+  for (let stackIndex = 0; stackIndex < stackTable.length; stackIndex++) {
+    const oldPrefix = oldPrefixCol[stackIndex];
+    const newPrefixPlusOne =
+      oldPrefix === null ? 0 : oldStackToNewStackPlusOne[oldPrefix];
+
+    const frameIndex = stackTableFrameCol[stackIndex];
+    const funcIndex = frameTableFuncCol[frameIndex];
+    if (shouldMergeFunction[funcIndex] === 1) {
+      oldStackToNewStackPlusOne[stackIndex] = newPrefixPlusOne;
+    } else {
+      oldStackToNewStackPlusOne[stackIndex] = stackIndex + 1;
+    }
+    const newPrefix = newPrefixPlusOne === 0 ? null : newPrefixPlusOne - 1;
+    newPrefixCol[stackIndex] = newPrefix;
+  }
+
+  const newStackTable = {
+    ...stackTable,
+    prefix: newPrefixCol,
+  };
+
+  return updateThreadStacks(thread, newStackTable, (oldStack) => {
+    if (oldStack === null) {
+      return null;
+    }
+    const newStackPlusOne = oldStackToNewStackPlusOne[oldStack];
+    return newStackPlusOne === 0 ? null : newStackPlusOne - 1;
+  });
+}
+
 /**
  * Drop any samples that contain the given function.
  */
 export function dropFunction(
   thread: Thread,
   funcIndexToDrop: IndexIntoFuncTable
-) {
+): Thread {
   const { stackTable, frameTable } = thread;
 
   // Go through each stack, and label it as containing the function or not.
@@ -1774,6 +1914,8 @@ export function applyTransform(
       );
     case 'merge-function':
       return mergeFunction(thread, transform.funcIndex);
+    case 'merge-unaccounted-native-functions':
+      return mergeUnaccountedNativeFunctions(thread);
     case 'drop-function':
       return dropFunction(thread, transform.funcIndex);
     case 'focus-function':

src/test/store/transforms.test.js

@@ -497,6 +497,70 @@ describe('"merge-function" transform', function () {
   });
 });
 
+describe('"merge-unaccounted-native-functions" transform', function () {
+  describe('on a call tree', function () {
+    /**
+     * Assert this transformation:
+     *
+     *                     A:3,0                              A:3,0
+     *                       |                                  |
+     *                       v        merge 0xC,0xE             v
+     *                     B:3,0           -->                B:3,0
+     *                     /    \                           /   |   \
+     *                    v      v                         v    v    v
+     *                C:2,0     H:1,0                 D:1,0   F:1,0   H:1,1
+     *               /      \         \                         |
+     *              v        v         v                        v
+     *            D:1,0     F:1,0     C:1,1                   G:1,1
+     *            |           |
+     *            v           v
+     *          0xE:1,1       G:1,1
+     */
+    const { profile } = getProfileFromTextSamples(`
+      A[lib:one][address:20][sym:Asym:20:]  A[lib:one][address:21][sym:Asym:20:]  A[lib:one][address:20][sym:Asym:20:]
+      B                                     B                                     B
+      0xC                                   0xC                                   H.js
+      D.js                                  F[lib:two][address:11][sym:Fsym:10:]  0xC
+      0xE                                   G[lib:one][address:51][sym:Dsym:40:]
+    `);
+    const threadIndex = 0;
+
+    const { dispatch, getState } = storeWithProfile(profile);
+    const originalCallTree = selectedThreadSelectors.getCallTree(getState());
+
+    it('starts as an unfiltered call tree', function () {
+      expect(formatTree(originalCallTree)).toEqual([
+        '- A (total: 3, self: —)',
+        '  - B (total: 3, self: —)',
+        '    - 0xC (total: 2, self: —)',
+        '      - D.js (total: 1, self: —)',
+        '        - 0xE (total: 1, self: 1)',
+        '      - F (total: 1, self: —)',
+        '        - G (total: 1, self: 1)',
+        '    - H.js (total: 1, self: —)',
+        '      - 0xC (total: 1, self: 1)',
+      ]);
+    });
+
+    it('functions 0xC and 0xE are merged into callers', function () {
+      dispatch(
+        addTransformToStack(threadIndex, {
+          type: 'merge-unaccounted-native-functions',
+        })
+      );
+      const callTree = selectedThreadSelectors.getCallTree(getState());
+      expect(formatTree(callTree)).toEqual([
+        '- A (total: 3, self: —)',
+        '  - B (total: 3, self: —)',
+        '    - D.js (total: 1, self: 1)',
+        '    - F (total: 1, self: —)',
+        '      - G (total: 1, self: 1)',
+        '    - H.js (total: 1, self: 1)',
+      ]);
+    });
+  });
+});
+
 describe('"drop-function" transform', function () {
   describe('on a call tree', function () {
     const {