Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(profiling): Parse Hermes Bytecode frames virtual address #3342

Merged
merged 4 commits into from
Oct 17, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Fixes

- Add actual `activeThreadId` to Profiles ([#3338](https://github.com/getsentry/sentry-react-native/pull/3338))
- Parse Hermes Profiling Bytecode Frames ([#3342](https://github.com/getsentry/sentry-react-native/pull/3342))

## 5.11.1

Expand Down
17 changes: 2 additions & 15 deletions src/js/profiling/convertHermesProfile.ts
Original file line number Diff line number Diff line change
@@ -1,22 +1,17 @@
import type { FrameId, StackId, ThreadCpuFrame, ThreadCpuSample, ThreadCpuStack, ThreadId } from '@sentry/types';
import { logger } from '@sentry/utils';
import { Platform } from 'react-native';

import { ANDROID_DEFAULT_BUNDLE_NAME, IOS_DEFAULT_BUNDLE_NAME } from '../integrations/rewriteframes';
import type * as Hermes from './hermes';
import { parseHermesStackFrameFunctionName } from './hermes';
import { parseHermesJSStackFrame } from './hermes';
import { MAX_PROFILE_DURATION_MS } from './integration';
import type { RawThreadCpuProfile } from './types';

const PLACEHOLDER_THREAD_ID_STRING = '0';
const MS_TO_NS = 1e6;
const MAX_PROFILE_DURATION_NS = MAX_PROFILE_DURATION_MS * MS_TO_NS;
const ANONYMOUS_FUNCTION_NAME = 'anonymous';
const UNKNOWN_STACK_ID = -1;
const JS_THREAD_NAME = 'JavaScriptThread';
const JS_THREAD_PRIORITY = 1;
const DEFAULT_BUNDLE_NAME =
Platform.OS === 'android' ? ANDROID_DEFAULT_BUNDLE_NAME : Platform.OS === 'ios' ? IOS_DEFAULT_BUNDLE_NAME : undefined;

/**
* Converts a Hermes profile to a Sentry profile.
Expand Down Expand Up @@ -136,15 +131,7 @@ function mapFrames(hermesStackFrames: Record<Hermes.StackFrameId, Hermes.StackFr
continue;
}
hermesStackFrameIdToSentryFrameIdMap.set(Number(key), frames.length);
const hermesFrame = hermesStackFrames[key];

const functionName = parseHermesStackFrameFunctionName(hermesFrame.name);
frames.push({
function: functionName || ANONYMOUS_FUNCTION_NAME,
file: hermesFrame.category == 'JavaScript' ? DEFAULT_BUNDLE_NAME : undefined,
lineno: hermesFrame.line !== undefined ? Number(hermesFrame.line) : undefined,
colno: hermesFrame.column !== undefined ? Number(hermesFrame.column) : undefined,
});
frames.push(parseHermesJSStackFrame(hermesStackFrames[key]));
}

return {
Expand Down
59 changes: 52 additions & 7 deletions src/js/profiling/hermes.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
import { Platform } from 'react-native';

import { ANDROID_DEFAULT_BUNDLE_NAME, IOS_DEFAULT_BUNDLE_NAME } from '../integrations/rewriteframes';
import { NATIVE } from '../wrapper';
import { convertToSentryProfile } from './convertHermesProfile';
import type { RawThreadCpuProfile } from './types';
Expand Down Expand Up @@ -28,10 +31,17 @@ export interface Sample {
}

export interface StackFrame {
// Hermes Bytecode
funcVirtAddr?: string;
offset?: string;

// JavaScript
line?: string;
column?: string;
funcLine?: string;
funcColumn?: string;

// Common
name: string;
category: string;
parent?: number;
Expand All @@ -43,15 +53,50 @@ export interface Profile {
stackFrames: Record<string, StackFrame>;
}

export interface ParsedHermesStackFrame {
function: string;
file?: string;
lineno?: number;
colno?: number;
}

const DEFAULT_BUNDLE_NAME =
Platform.OS === 'android' ? ANDROID_DEFAULT_BUNDLE_NAME : Platform.OS === 'ios' ? IOS_DEFAULT_BUNDLE_NAME : undefined;
const ANONYMOUS_FUNCTION_NAME = 'anonymous';

/**
* Hermes Profile Stack Frame Name contains function name and file path.
*
* `foo(/path/to/file.js:1:2)` -> `foo`
* Parses Hermes StackFrame to Sentry StackFrame.
* For native frames only function name is returned, for Hermes bytecode the line and column are calculated.
*/
export function parseHermesStackFrameFunctionName(hermesName: string): string {
const indexOfLeftParenthesis = hermesName.indexOf('(');
const name = indexOfLeftParenthesis !== -1 ? hermesName.substring(0, indexOfLeftParenthesis) : hermesName;
return name;
export function parseHermesJSStackFrame(frame: StackFrame): ParsedHermesStackFrame {
if (frame.category !== 'JavaScript') {
// Native
return { function: frame.name };
}

if (frame.funcVirtAddr !== undefined && frame.offset !== undefined) {
// Hermes Bytecode
return {
function: frame.name || ANONYMOUS_FUNCTION_NAME,
file: DEFAULT_BUNDLE_NAME,
// https://github.com/krystofwoldrich/metro/blob/417e6f276ff9422af6039fc4d1bce41fcf7d9f46/packages/metro-symbolicate/src/Symbolication.js#L298-L301
// Hermes lineno is hardcoded 1, currently only one bundle symbolication is supported by metro-symbolicate and thus by us.
lineno: 1,
// Hermes colno is 0-based, while Sentry is 1-based
colno: Number(frame.funcVirtAddr) + Number(frame.offset) + 1,
};
}

// JavaScript
const indexOfLeftParenthesis = frame.name.indexOf('(');
return {
function:
(indexOfLeftParenthesis !== -1 && (frame.name.substring(0, indexOfLeftParenthesis) || ANONYMOUS_FUNCTION_NAME)) ||
frame.name,
file: DEFAULT_BUNDLE_NAME,
lineno: frame.line !== undefined ? Number(frame.line) : undefined,
colno: frame.column !== undefined ? Number(frame.column) : undefined,
};
}

const MS_TO_NS: number = 1e6;
Expand Down
9 changes: 3 additions & 6 deletions test/profiling/convertHermesProfile.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ describe('convert hermes profile to sentry profile', () => {
column: '33',
funcLine: '1605',
funcColumn: '14',
name: 'fooA(/absolute/path/app:///main.jsbundle:1610:33)',
name: 'fooA(app:///main.jsbundle:1610:33)',
category: 'JavaScript',
parent: 1,
},
Expand All @@ -65,7 +65,7 @@ describe('convert hermes profile to sentry profile', () => {
column: '21',
funcLine: '1614',
funcColumn: '14',
name: 'fooB(/absolute/path/app:///main.jsbundle:1616:21)',
name: 'fooB(http://localhost:8081/index.bundle//&platform=ios&dev=true&minify=false&modulesOnly=false&runModule=true&app=org.reactjs.native.example.sampleNewArchitecture:193430:4)',
category: 'JavaScript',
parent: 1,
},
Expand All @@ -74,7 +74,7 @@ describe('convert hermes profile to sentry profile', () => {
column: '18',
funcLine: '1623',
funcColumn: '16',
name: '(/absolute/path/app:///main.jsbundle:1627:18)',
name: '(/Users/distiller/react-native/packages/react-native/sdks/hermes/build_iphonesimulator/lib/InternalBytecode/InternalBytecode.js:139:27)',
category: 'JavaScript',
parent: 2,
},
Expand All @@ -83,10 +83,7 @@ describe('convert hermes profile to sentry profile', () => {
const expectedSentryProfile: RawThreadCpuProfile = {
frames: [
{
colno: undefined,
file: undefined,
function: '[root]',
lineno: undefined,
},
{
colno: 33,
Expand Down
54 changes: 49 additions & 5 deletions test/profiling/hermes.test.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,62 @@
import { parseHermesStackFrameFunctionName } from '../../src/js/profiling/hermes';
import type { ParsedHermesStackFrame } from '../../src/js/profiling/hermes';
import { parseHermesJSStackFrame } from '../../src/js/profiling/hermes';

describe('hermes', () => {
describe('parseHermesStackFrameName', () => {
test('parses function name and file name', () => {
expect(parseHermesStackFrameFunctionName('fooA(/absolute/path/main.jsbundle:1610:33)')).toEqual('fooA');
expect(
parseHermesJSStackFrame({
name: 'fooA(/absolute/path/main.jsbundle:1610:33)',
line: '1610',
column: '33',
category: 'JavaScript',
}),
).toEqual(<ParsedHermesStackFrame>{
function: 'fooA',
file: 'app:///main.jsbundle',
lineno: 1610,
colno: 33,
});
});
test('parse hermes root stack frame', () => {
expect(parseHermesStackFrameFunctionName('[root]')).toEqual('[root]');
expect(
parseHermesJSStackFrame({
name: '[root]',
category: 'root',
}),
).toEqual(
expect.objectContaining(<ParsedHermesStackFrame>{
function: '[root]',
}),
);
});
test('parse only file name', () => {
expect(parseHermesStackFrameFunctionName('(/absolute/path/jsbundle:1610:33)')).toEqual('');
expect(
parseHermesJSStackFrame({
name: '(/absolute/path/main.jsbundle:1610:33)',
line: '1610',
column: '33',
category: 'JavaScript',
}),
).toEqual(<ParsedHermesStackFrame>{
function: 'anonymous',
file: 'app:///main.jsbundle',
lineno: 1610,
colno: 33,
});
});
test('parse only function name', () => {
expect(parseHermesStackFrameFunctionName('fooA')).toEqual('fooA');
expect(
parseHermesJSStackFrame({
name: 'fooA',
category: 'JavaScript',
}),
).toEqual(
expect.objectContaining(<ParsedHermesStackFrame>{
function: 'fooA',
file: 'app:///main.jsbundle',
}),
);
});
});
});
Loading