mirror of
https://github.com/software-mansion/argent.git
synced 2026-09-14 19:27:14 +08:00
f31ea894b1
## Context This began as "add native iOS memory profiling," but the feature **already exists** — the iOS native profiler captures and reports memory **leaks** end-to-end (export → parse → aggregate → render → `leak_stacks`). An audit plus real-device verification then surfaced genuine defects, fixed here. > **Course-correction worth noting:** an early static read suspected the leak parser/xpath were broken. A real capture disproved that — xctrace's `Leaks` *track-detail* export uses self-closing attribute rows (a different shape from the CPU/hangs *schema tables*), and the parser matches it correctly. So this PR does **not** touch parsing/xpath; the real issues are below. ## Changes ### 1. Drop the fabricated `--hal` flag (`export.ts`) `xctrace export` has no `--hal` option — verified against xctrace 16 and every subcommand's help. On xctrace ≥ 15 the exporter passed it, so the **first** export attempt always failed and silently fell back to the plain export: a wasted failing subprocess per leak export, plus a false "required by newer xctrace versions" claim in code + docs. Leaks export exactly like hangs (one `xctrace export --xpath` of the `Leaks` track detail), so the special case and the now-unused `getXctraceVersion` are removed. ### 2. Stop flagging unattributed leaks as RED Argent records via `xctrace --attach`, which gives the Leaks/Allocations instruments **no malloc-stack history**, so most leaks come back unattributed: responsible frame `<Call stack limit reached>`, empty library, generic `Malloc N Bytes`/`xpc_*` types. (CPU/hang stacks symbolicate fine; leaks largely don't under attach.) The pipeline marked them **all** RED, so idle Safari reported 19 RED "leaks" (1646 system allocations) — benign noise as confirmed bugs. - `aggregateLeaks` tags each group `attributed` (real responsible frame) and sets severity RED (attributed) / **YELLOW** (unattributed); attributed leaks sort first. New `isLeakAttributed()` centralises the check. - The report shows attributed leaks in the full table; unattributed ones collapse into one low-confidence YELLOW summary with guidance. `leak_stacks` and the suggested-fix / next-step hints fire only for attributed leaks. ### 3. Tests + docs - New `ios-leak-pipeline.test.ts` over `parseLeaksXml` + `aggregateLeaks` (the leak path had **zero** coverage), built from a real xctrace 16 capture — pins the self-closing-attribute-row format and the attributed/unattributed severity split. - Reference doc + `argent-native-profiler` skill: corrected the `--hal` claim, documented the `--attach` attribution limitation, and updated the severity rules. ## Verification Ran the real pipeline + render over actual xctrace-16 captures (Argent template, `--attach`) on three apps: - **Bluesky** (social-app, complex app, ~85s onboarding/sign-in churn): **9 leak groups, exactly 1 genuinely attributed** — `hermes::vm::JSTypedArrayBase::createBuffer` (library `hermes`) → rendered RED in the table with a fix suggestion + `leak_stacks` next-step; the other 8 (345 objects, 642 KB) collapse into one YELLOW line. **This mixed case is the key proof:** a blanket filter would have *hidden a real Hermes leak*, so demote-don't-drop is the correct design. - **Idle Safari:** 1646 rows → 19 groups, all unattributed → one YELLOW summary (was 19 RED rows). - **Active pokemon RN app (~30s):** 1 unattributed group → one YELLOW summary (was 1 RED); CPU hotspots symbolicated normally, confirming the capture itself works. - `ios-leak-pipeline.test.ts`: 6/6 pass. Changed files type-clean + Prettier-clean. CI green.
217 lines
8.8 KiB
TypeScript
217 lines
8.8 KiB
TypeScript
import { describe, it, expect } from "vitest";
|
||
import { renderNativeProfilerReport } from "../../src/utils/ios-profiler/render";
|
||
import type {
|
||
CpuHotspot,
|
||
MemoryLeak,
|
||
ProfilerPayload,
|
||
UiHang,
|
||
} from "../../src/utils/ios-profiler/types";
|
||
|
||
// A mangled nested name that the conservative demangler understands:
|
||
// _ZN16GrDrawingManager5flushE... → GrDrawingManager::flush (argument list dropped).
|
||
const MANGLED = "_ZN16GrDrawingManager5flushE6SkSpanIP14GrSurfaceProxyE";
|
||
const DEMANGLED = "GrDrawingManager::flush";
|
||
|
||
// A second, distinct mangled nested name → MyClass::doWork. Used to prove the
|
||
// `responsibleFrame` / leak sites demangle independently of the hang sites.
|
||
const MANGLED_LEAK = "_ZN7MyClass6doWorkEv";
|
||
const DEMANGLED_LEAK = "MyClass::doWork";
|
||
|
||
function cpuHotspot(overrides: Partial<CpuHotspot> = {}): CpuHotspot {
|
||
return {
|
||
type: "cpu_hotspot",
|
||
platform: "android",
|
||
dominantFunction: MANGLED,
|
||
totalWeightMs: 120,
|
||
weightPercentage: 42,
|
||
sampleCount: 60,
|
||
thread: "Main Thread",
|
||
severity: "RED",
|
||
topCallChain: [],
|
||
topCallChains: [],
|
||
duringHang: false,
|
||
timeRangeMs: { first: 0, last: 1000 },
|
||
burstWindows: [],
|
||
...overrides,
|
||
};
|
||
}
|
||
|
||
function payloadWith(hotspot: CpuHotspot): ProfilerPayload {
|
||
return {
|
||
// traceFile null → renderNativeProfilerReport writes no file (hermetic).
|
||
metadata: { traceFile: null, platform: "android", timestamp: "2026-01-01T00:00:00Z" },
|
||
bottlenecks: [hotspot],
|
||
};
|
||
}
|
||
|
||
describe("renderNativeProfilerReport — CPU Hotspots leaf demangling", () => {
|
||
it("shows the DEMANGLED leaf in the table and heading, but keeps the RAW mangled name in the function_callers suggestion", async () => {
|
||
const { report } = await renderNativeProfilerReport({
|
||
payload: payloadWith(cpuHotspot()),
|
||
traceFile: null,
|
||
});
|
||
|
||
// (a) Human-facing display sites are demangled.
|
||
// Table cell: `| 1 | `GrDrawingManager::flush` | Main Thread | ...`
|
||
expect(report).toContain(`| 1 | \`${DEMANGLED}\` | Main Thread |`);
|
||
// Detail heading: `### `GrDrawingManager::flush` (Main Thread)`
|
||
expect(report).toContain(`### \`${DEMANGLED}\` (Main Thread)`);
|
||
// Suggested Improvements bullet is demangled too.
|
||
expect(report).toContain(`\`${DEMANGLED}\` on Main Thread`);
|
||
|
||
// (b) The copy-paste drill-down MUST keep the RAW mangled symbol — it is
|
||
// matched as a SQL substring of the mangled frame, and the demangled name
|
||
// (args dropped) is not a substring of it.
|
||
expect(report).toContain(`mode=\`function_callers\` function_name=\`${MANGLED}\``);
|
||
// The mangled name must NOT leak into the human-facing table/heading.
|
||
expect(report).not.toContain(`| 1 | \`${MANGLED}\``);
|
||
expect(report).not.toContain(`### \`${MANGLED}\``);
|
||
});
|
||
|
||
it("demangles the frames in the per-hotspot Call chains block", async () => {
|
||
// Android builds callChains as [{ chain: [leaf_function] }] — the chain frame
|
||
// is the SAME mangled string as dominantFunction. Without demangling here the
|
||
// heading would read `GrDrawingManager::flush` while the chain right under it
|
||
// shows the raw `_ZN16GrDrawingManager…` for the very same function.
|
||
const { report } = await renderNativeProfilerReport({
|
||
payload: payloadWith(cpuHotspot({ topCallChains: [{ chain: [MANGLED], count: 60 }] })),
|
||
traceFile: null,
|
||
});
|
||
expect(report).toContain(`- (60×) \`${DEMANGLED}\``);
|
||
expect(report).not.toContain(`- (60×) \`${MANGLED}\``);
|
||
// Only the function_callers drill-down keeps the raw mangled name (by design).
|
||
expect(report.split(MANGLED).length - 1).toBe(1);
|
||
expect(report).toContain(`function_name=\`${MANGLED}\``);
|
||
});
|
||
|
||
it("demangles the single-chain fallback (`Call chain:`) too", async () => {
|
||
// Reached when topCallChains is empty but a topCallChain is present.
|
||
const { report } = await renderNativeProfilerReport({
|
||
payload: payloadWith(cpuHotspot({ topCallChain: [MANGLED], topCallChains: [] })),
|
||
traceFile: null,
|
||
});
|
||
expect(report).toContain(`**Call chain:** \`${DEMANGLED}\``);
|
||
expect(report).not.toContain(`**Call chain:** \`${MANGLED}\``);
|
||
});
|
||
|
||
it("leaves an already-readable (iOS-style) leaf name untouched", async () => {
|
||
// iOS frames arrive pre-symbolicated; the demangler bails on anything that
|
||
// is not an Itanium `_Z…` name, so applying it is a safe no-op.
|
||
const ios = "-[MyViewController viewDidLoad]";
|
||
const { report } = await renderNativeProfilerReport({
|
||
payload: payloadWith(cpuHotspot({ platform: "ios", dominantFunction: ios })),
|
||
traceFile: null,
|
||
});
|
||
expect(report).toContain(`| 1 | \`${ios}\` |`);
|
||
expect(report).toContain(`### \`${ios}\` (Main Thread)`);
|
||
});
|
||
});
|
||
|
||
// ---------------------------------------------------------------------------
|
||
// General demangling: hang appCallChains / suspectedFunctions, and memory-leak
|
||
// responsibleFrame — in both the detail section AND Suggested Improvements.
|
||
// iOS frames are the realistic carrier of mangled C++ names here (Android hang
|
||
// appCallChains/suspectedFunctions are hardcoded empty; the SQL path already
|
||
// demangles its own callstack text), so these payloads are iOS-flavoured.
|
||
// ---------------------------------------------------------------------------
|
||
|
||
function uiHang(overrides: Partial<UiHang> = {}): UiHang {
|
||
return {
|
||
type: "ui_hang",
|
||
platform: "ios",
|
||
hangType: "hang",
|
||
durationMs: 850,
|
||
startTimeFormatted: "00:01.500",
|
||
startNs: 1_500_000_000,
|
||
endNs: 2_350_000_000,
|
||
suspectedFunctions: [],
|
||
appCallChains: [],
|
||
severity: "RED",
|
||
...overrides,
|
||
};
|
||
}
|
||
|
||
function memoryLeak(overrides: Partial<MemoryLeak> = {}): MemoryLeak {
|
||
return {
|
||
type: "memory_leak",
|
||
platform: "ios",
|
||
objectType: "MyLeakyObject",
|
||
totalSizeBytes: 4 * 1024 * 1024,
|
||
count: 128,
|
||
responsibleFrame: MANGLED_LEAK,
|
||
responsibleLibrary: "MyApp",
|
||
attributed: true,
|
||
severity: "RED",
|
||
...overrides,
|
||
};
|
||
}
|
||
|
||
function payloadOf(bottlenecks: ProfilerPayload["bottlenecks"]): ProfilerPayload {
|
||
return {
|
||
metadata: { traceFile: null, platform: "ios", timestamp: "2026-01-01T00:00:00Z" },
|
||
bottlenecks,
|
||
};
|
||
}
|
||
|
||
describe("renderNativeProfilerReport — general native-frame demangling", () => {
|
||
it("demangles a hang's appCallChains frames in the detail section", async () => {
|
||
const { report } = await renderNativeProfilerReport({
|
||
payload: payloadOf([
|
||
uiHang({ appCallChains: [{ chain: [MANGLED, MANGLED_LEAK], sampleCount: 40 }] }),
|
||
]),
|
||
traceFile: null,
|
||
});
|
||
// Detail: `1. `GrDrawingManager::flush > MyClass::doWork` (40 samples)`
|
||
expect(report).toContain(`1. \`${DEMANGLED} > ${DEMANGLED_LEAK}\` (40 samples)`);
|
||
expect(report).not.toContain(MANGLED);
|
||
expect(report).not.toContain(MANGLED_LEAK);
|
||
});
|
||
|
||
it("demangles a hang's suspectedFunctions in BOTH the detail list and Suggested Improvements", async () => {
|
||
const { report } = await renderNativeProfilerReport({
|
||
payload: payloadOf([uiHang({ suspectedFunctions: [MANGLED, MANGLED_LEAK] })]),
|
||
traceFile: null,
|
||
});
|
||
// Detail bullet (the `else if (suspectedFunctions)` branch, no appCallChains):
|
||
expect(report).toContain(`- \`${DEMANGLED}\``);
|
||
expect(report).toContain(`- \`${DEMANGLED_LEAK}\``);
|
||
// Suggested Improvements → UI Hangs "Likely caused by: `…`" uses [0].
|
||
expect(report).toContain(`Likely caused by: \`${DEMANGLED}\`.`);
|
||
// No mangled name anywhere in the report.
|
||
expect(report).not.toContain(MANGLED);
|
||
expect(report).not.toContain(MANGLED_LEAK);
|
||
});
|
||
|
||
it("demangles a memory leak's responsibleFrame in BOTH the table and Suggested Improvements", async () => {
|
||
const { report } = await renderNativeProfilerReport({
|
||
payload: payloadOf([memoryLeak({ responsibleFrame: MANGLED_LEAK })]),
|
||
traceFile: null,
|
||
});
|
||
// Memory-Leaks table cell.
|
||
expect(report).toContain(`\`${DEMANGLED_LEAK}\` | MyApp |`);
|
||
// Suggested Improvements → Memory Leaks "via `…`".
|
||
expect(report).toContain(`via \`${DEMANGLED_LEAK}\`:`);
|
||
expect(report).not.toContain(MANGLED_LEAK);
|
||
});
|
||
|
||
it("leaves already-readable (iOS-style) frames untouched at every general site (no-op)", async () => {
|
||
const hangFn = "-[DataStore loadAll]";
|
||
const leakFrame = "-[ImageCache store:]";
|
||
const { report } = await renderNativeProfilerReport({
|
||
payload: payloadOf([
|
||
uiHang({
|
||
appCallChains: [{ chain: [hangFn], sampleCount: 12 }],
|
||
suspectedFunctions: [hangFn],
|
||
}),
|
||
memoryLeak({ responsibleFrame: leakFrame }),
|
||
]),
|
||
traceFile: null,
|
||
});
|
||
// appCallChains detail, Suggested-Improvements hang note, leak table + via — all verbatim.
|
||
expect(report).toContain(`1. \`${hangFn}\` (12 samples)`);
|
||
expect(report).toContain(`Likely caused by: \`${hangFn}\`.`);
|
||
expect(report).toContain(`\`${leakFrame}\` | MyApp |`);
|
||
expect(report).toContain(`via \`${leakFrame}\`:`);
|
||
});
|
||
});
|