Skip to content

cpu-prof: match Node/Deno Chrome DevTools format #29241

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.

Sign up for GitHub

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

Merged
Jarred-Sumner merged 17 commits into from
Apr 14, 2026
Merged

cpu-prof: match Node/Deno Chrome DevTools format #29241

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
5cdb657
cpu-prof: match Node/Deno Chrome DevTools format
robobun Apr 13, 2026
0e9c177
[autofix.ci] apply automated fixes
autofix-ci[bot] Apr 13, 2026
57bf345
cpu-prof: carry the def-mapped URL through, tighten positionTicks test
robobun Apr 13, 2026
36ee468
cpu-prof test: assert sum(positionTicks) <= hitCount in sourcemap pat…
robobun Apr 13, 2026
97bfb9e
cpu-prof test: trim top-of-file prose, bump per-test timeout to 30s
robobun Apr 13, 2026
50db0ef
[autofix.ci] apply automated fixes
autofix-ci[bot] Apr 13, 2026
e840181
cpu-prof: delete dead stopCPUProfilerAndGetJSON
robobun Apr 13, 2026
a1756db
cpu-prof test: gate on isDebug so release CI lanes skip
robobun Apr 13, 2026
fab5f13
ci: retrigger after darwin-13-aarch64 runner flake
robobun Apr 13, 2026
a9c4af2
cpu-prof test: revert skipIf(!isDebug), let release lanes run
robobun Apr 13, 2026
a811a82
cpu-prof test: drop explicit timeout override
robobun Apr 13, 2026
09dfc0c
[autofix.ci] apply automated fixes
autofix-ci[bot] Apr 13, 2026
2bc31ad
cpu-prof: drop sample line when source maps to a different file
robobun Apr 13, 2026
538c046
cpu-prof: normalize callFrame.url to file:// after sourcemap calls
robobun Apr 13, 2026
68bac11
cpu-prof test: rename unused stderr → _stderr, fix stale comment
robobun Apr 13, 2026
a60bbbe
cpu-prof: fix misleading sampleURL comment + tighten fib merge key
robobun Apr 13, 2026
8633b6c
ci: retrigger gate after staging webkit caches
robobun Apr 13, 2026
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
193 changes: 169 additions & 24 deletions src/bun.js/bindings/BunCPUProfiler.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
#include <wtf/HashSet.h>
#include <wtf/URL.h>
#include <algorithm>
#include <limits>

extern "C" void Bun__startCPUProfiler(JSC::VM* vm);
extern "C" void Bun__stopCPUProfiler(JSC::VM* vm, BunString* outJSON, BunString* outText);
Expand Down Expand Up @@ -66,10 +67,19 @@
WTF::String functionName;
WTF::String url;
int scriptId;
// lineNumber/columnNumber are the location where the function is DEFINED
// (matching Node/Deno/Chrome DevTools), stored as 0-indexed values ready
// for JSON emission. -1 means "unknown".
int lineNumber;
int columnNumber;
int hitCount;
WTF::Vector<int> children;
// Per-line sample counts for this node, keyed by 1-indexed source line.
// Emitted as `positionTicks` in the JSON output when non-empty, matching
// the Chrome DevTools CPU profile format used by Node and Deno.
// Lines are guaranteed non-zero, so the default IntHashTraits (which reserve
// 0 and -1 as empty/deleted sentinels) are safe here.
WTF::HashMap<int, int, WTF::IntHash<int>> positionTicks;
};

Comment on lines +77 to 84
Copy link
Copy Markdown
Contributor

@claude claude bot Apr 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 The comment on positionTicks in the ProfileNode struct cites incorrect sentinel values: it says the default IntHashTraits reserves 0 and -1, but WTF::HashTraits<int> (the actual key traits) uses 0 as the empty sentinel and std::numeric_limits<int>::max() (INT_MAX) as the deleted sentinel — not -1. The code is functionally correct since source line numbers are always small positive integers that can never equal INT_MAX, but the comment misstates the reserved values and names a nonexistent WTF type.

Extended reasoning...

The comment at lines 80-81 reads: "the default IntHashTraits (which reserve 0 and -1 as empty/deleted sentinels) are safe here." This has two inaccuracies.

First, "IntHashTraits" is not a WTF type name. The HashMap declaration is WTF::HashMap<int, int, WTF::IntHash<int>>, where WTF::IntHash<int> is the hash function and the key traits default to WTF::HashTraits<int>. The comment conflates the hash function name with the traits type name.

Second, the sentinel values cited (-1 as deleted) are likely wrong. Modern WebKit WTF defines HashTraits<int> via IntegralHashTraits<int>, which sets the empty sentinel to T() (0) via emptyValueIsZero = true, and the deleted sentinel to std::numeric_limits<T>::max() (INT_MAX for int). The comment's claim of -1 as the deleted sentinel does not match this type.

One refuting verifier argued that GenericHashTraits<T> for signed integer types uses T(-1) as the deleted sentinel, making the comment correct. However, multiple confirming verifiers independently traced the trait hierarchy to IntegralHashTraits<int> with INT_MAX as the deleted value. Regardless of which exact sentinel value is used, the type name "IntHashTraits" is not a real WTF type and will mislead any reader who tries to look it up.

The runtime impact is zero: sampleLine is always a 1-indexed source line number populated only when sourceMappedLineColumn.line > 0, making it a small positive integer that cannot collide with 0, -1, INT_MAX, or INT_MAX-1. The safety invariant holds regardless of which sentinel values WTF actually uses.

The risk of the misleading comment is that a future developer might: (a) add a spurious sampleLine != -1 guard believing -1 is a sentinel; (b) rely on the sampleLine > 0 guard as sufficient protection against the alleged "0 sentinel" while missing the actual INT_MAX sentinel; or (c) spend time searching for the nonexistent "IntHashTraits" type. The fix is to replace the comment with the correct type name (WTF::HashTraits<int>) and accurate sentinel values, along with the real safety justification: source line numbers are small positive integers, nowhere near INT_MAX.

WTF::String stopCPUProfilerAndGetJSON(JSC::VM& vm)
Expand Down Expand Up @@ -160,8 +170,13 @@
WTF::String functionName;
WTF::String url;
int scriptId = 0;
int lineNumber = -1;
int columnNumber = -1;
// Function-definition line/column (0-indexed for JSON emit, matching
// Node/Deno/Chrome DevTools). -1 means "unknown".
int functionDefLine = -1;
int functionDefColumn = -1;
// Current-sample line (1-indexed) — fed to positionTicks when this
// frame is the top of the stack. 0 means "unknown".
int sampleLine = 0;

// Get function name - displayName works for all frame types
functionName = frame.displayName(vm);
Expand Down Expand Up @@ -200,24 +215,67 @@
}
}

// Function definition location. JSC returns these 1-based;
// Node/Deno/Chrome DevTools emit them 0-based in the JSON.
// We remap this (not the sample position) through the
// sourcemap callback so callFrame.url/line/column all agree
// on the function's DEFINITION site — bundled/transpiled
// scripts then report the original source for the node while
// positionTicks reports original-source sample lines.
int rawFunctionStartLine = frame.functionStartLine();
unsigned rawFunctionStartColumn = frame.functionStartColumn();
if (rawFunctionStartLine > 0 && rawFunctionStartColumn != std::numeric_limits<unsigned>::max()) {
JSC::LineColumn functionStartLineColumn {
static_cast<unsigned>(rawFunctionStartLine),
rawFunctionStartColumn,
};
if (provider) {
#if USE(BUN_JSC_ADDITIONS)
auto& fn = vm.computeLineColumnWithSourcemap();
if (fn) {
// `url` is the out-param — on a successful remap
// it becomes the original-source URL, matching
// the line/column we're emitting.
fn(vm, provider, functionStartLineColumn, url);
}
#endif
}
// Emit 0-indexed, clamping at 0 so we never underflow.
functionDefLine = functionStartLineColumn.line > 0
? static_cast<int>(functionStartLineColumn.line) - 1
: 0;
functionDefColumn = functionStartLineColumn.column > 0
? static_cast<int>(functionStartLineColumn.column) - 1
: 0;
}

if (frame.hasExpressionInfo()) {
// Apply sourcemap if available
// Current sample position (what line inside the function
// was executing). Apply sourcemap if available, but keep
// the URL we already resolved for the function definition
// — a throwaway out-param ensures the sample remap can't
// clobber `url` with the sample's source file (which, for
// inlined helpers, may differ from the function's).
JSC::LineColumn sourceMappedLineColumn = frame.semanticLocation.lineColumn;
if (provider) {
#if USE(BUN_JSC_ADDITIONS)
auto& fn = vm.computeLineColumnWithSourcemap();
if (fn) {
fn(vm, provider, sourceMappedLineColumn, url);
WTF::String scratchURL = url;
fn(vm, provider, sourceMappedLineColumn, scratchURL);
}
#endif
}
lineNumber = static_cast<int>(sourceMappedLineColumn.line);
columnNumber = static_cast<int>(sourceMappedLineColumn.column);
// positionTicks lines are 1-indexed in the Chrome format;
// semanticLocation.line is already 1-based.
if (sourceMappedLineColumn.line > 0)
sampleLine = static_cast<int>(sourceMappedLineColumn.line);
}
}

// Create a unique key for this frame based on parent + callFrame
// This creates separate nodes for the same function in different call paths
// Create a unique key for this frame based on parent + callFrame.
// line/column here identify the function's DEFINITION, so all samples
// of the same function under the same parent collapse to one node.
WTF::StringBuilder keyBuilder;
keyBuilder.append(currentParentId);
keyBuilder.append(':');
Expand All @@ -227,9 +285,9 @@
keyBuilder.append(':');
keyBuilder.append(scriptId);
keyBuilder.append(':');
keyBuilder.append(lineNumber);
keyBuilder.append(functionDefLine);
keyBuilder.append(':');
keyBuilder.append(columnNumber);
keyBuilder.append(functionDefColumn);

WTF::String key = keyBuilder.toString();

Expand All @@ -245,8 +303,8 @@
node.functionName = functionName;
node.url = url;
node.scriptId = scriptId;
node.lineNumber = lineNumber;
node.columnNumber = columnNumber;
node.lineNumber = functionDefLine;
node.columnNumber = functionDefColumn;
node.hitCount = 0;

nodes.append(WTF::move(node));
Expand All @@ -262,9 +320,13 @@

currentParentId = nodeId;

// If this is the top frame, increment hit count
// If this is the top frame, increment hit count and record the
// sample line in positionTicks (matching Node/Deno/Chrome DevTools).
if (i == 0) {
nodes[nodeId - 1].hitCount++;
if (sampleLine > 0) {
nodes[nodeId - 1].positionTicks.add(sampleLine, 0).iterator->value++;
}
}
}

Expand Down Expand Up @@ -310,11 +372,31 @@
childrenArray->pushInteger(childId);
}
}
nodeObj->setValue("children"_s, childrenArray);
}

// Per-line sample counts (Chrome DevTools format). Emit sorted by line
// for deterministic output.
if (!node.positionTicks.isEmpty()) {
WTF::Vector<std::pair<int, int>> sortedTicks;
sortedTicks.reserveInitialCapacity(node.positionTicks.size());
for (auto& entry : node.positionTicks)
sortedTicks.append({ entry.key, entry.value });
std::sort(sortedTicks.begin(), sortedTicks.end(), [](const auto& a, const auto& b) {
return a.first < b.first;
});
auto positionTicksArray = JSON::Array::create();
for (auto& [line, ticks] : sortedTicks) {
auto tickObj = JSON::Object::create();
tickObj->setInteger("line"_s, line);
tickObj->setInteger("ticks"_s, ticks);
positionTicksArray->pushValue(tickObj);
}
nodeObj->setValue("positionTicks"_s, positionTicksArray);
}

nodesArray->pushValue(nodeObj);
}

Check notice on line 399 in src/bun.js/bindings/BunCPUProfiler.cpp

View check run for this annotation

Claude / Claude Code Review

stopCPUProfilerAndGetJSON is dead code: PR changes have no runtime effect 

This is a pre-existing issue: stopCPUProfilerAndGetJSON (BunCPUProfiler.cpp) has no callers anywhere in the codebase and is not declared in BunCPUProfiler.h — all production profiler stop paths go through Bun::stopCPUProfiler instead. The PR invested effort updating this dead function with the same positionTicks, functionDefLine/Column, and scratchURL isolation changes applied to the live implementation, but those changes have zero runtime effect. The dead function should be removed to avoid fut
Comment thread
robobun marked this conversation as resolved.
Outdated
json->setValue("nodes"_s, nodesArray);

// Add timing info in microseconds
Expand Down Expand Up @@ -638,8 +720,11 @@
WTF::String functionName = frame.displayName(vm);
WTF::String url;
int scriptId = 0;
int lineNumber = -1;
int columnNumber = -1;
// Function-definition line/column (0-indexed) for callFrame.
int functionDefLine = -1;
int functionDefColumn = -1;
// Current-sample line (1-indexed) for positionTicks.
int sampleLine = 0;

if (frame.frameType == JSC::SamplingProfiler::FrameType::Executable && frame.executable) {
auto sourceProviderAndID = frame.sourceProviderAndID();
Expand All @@ -664,20 +749,57 @@
url = WTF::URL::fileURLWithFileSystemPath(url).string();
}

// Function definition location. JSC returns these 1-based;
// Node/Deno/Chrome DevTools emit them 0-based in the JSON.
// The definition (not the sample position) is remapped
// through the sourcemap callback so callFrame.url and
// callFrame.line/column agree on the function's source.
int rawFunctionStartLine = frame.functionStartLine();
unsigned rawFunctionStartColumn = frame.functionStartColumn();
if (rawFunctionStartLine > 0 && rawFunctionStartColumn != std::numeric_limits<unsigned>::max()) {
JSC::LineColumn functionStartLineColumn {
static_cast<unsigned>(rawFunctionStartLine),
rawFunctionStartColumn,
};
if (provider) {
#if USE(BUN_JSC_ADDITIONS)
auto& fn = vm.computeLineColumnWithSourcemap();
if (fn) {
// `url` is the out-param — on a successful
// remap it becomes the original-source URL.
fn(vm, provider, functionStartLineColumn, url);
}
#endif
}
functionDefLine = functionStartLineColumn.line > 0
? static_cast<int>(functionStartLineColumn.line) - 1
: 0;
functionDefColumn = functionStartLineColumn.column > 0
? static_cast<int>(functionStartLineColumn.column) - 1
: 0;
}
Comment thread
robobun marked this conversation as resolved.

if (frame.hasExpressionInfo()) {
// Sample position for positionTicks — a throwaway
// out-param ensures the sample remap can't clobber
// `url` with a different file than the definition.
JSC::LineColumn sourceMappedLineColumn = frame.semanticLocation.lineColumn;
if (provider) {
#if USE(BUN_JSC_ADDITIONS)
auto& fn = vm.computeLineColumnWithSourcemap();
if (fn)
fn(vm, provider, sourceMappedLineColumn, url);
if (fn) {
WTF::String scratchURL = url;
fn(vm, provider, sourceMappedLineColumn, scratchURL);
}
#endif
}
lineNumber = static_cast<int>(sourceMappedLineColumn.line);
columnNumber = static_cast<int>(sourceMappedLineColumn.column);
if (sourceMappedLineColumn.line > 0)
sampleLine = static_cast<int>(sourceMappedLineColumn.line);
Comment thread
robobun marked this conversation as resolved.
}
}

// line/column here identify the function's DEFINITION, so all
// samples of the same function under the same parent collapse.
WTF::StringBuilder keyBuilder;
keyBuilder.append(currentParentId);
keyBuilder.append(':');
Expand All @@ -687,9 +809,9 @@
keyBuilder.append(':');
keyBuilder.append(scriptId);
keyBuilder.append(':');
keyBuilder.append(lineNumber);
keyBuilder.append(functionDefLine);
keyBuilder.append(':');
keyBuilder.append(columnNumber);
keyBuilder.append(functionDefColumn);

WTF::String key = keyBuilder.toString();

Expand All @@ -704,8 +826,8 @@
node.functionName = functionName;
node.url = url;
node.scriptId = scriptId;
node.lineNumber = lineNumber;
node.columnNumber = columnNumber;
node.lineNumber = functionDefLine;
node.columnNumber = functionDefColumn;
node.hitCount = 0;

nodes.append(WTF::move(node));
Expand All @@ -718,8 +840,11 @@

currentParentId = nodeId;

if (i == 0)
if (i == 0) {
nodes[nodeId - 1].hitCount++;
if (sampleLine > 0)
nodes[nodeId - 1].positionTicks.add(sampleLine, 0).iterator->value++;
}
}

samples.append(currentParentId);
Expand Down Expand Up @@ -761,6 +886,26 @@
nodeObj->setValue("children"_s, childrenArray);
}

// Per-line sample counts (Chrome DevTools format). Emit sorted by
// line for deterministic output.
if (!node.positionTicks.isEmpty()) {
WTF::Vector<std::pair<int, int>> sortedTicks;
sortedTicks.reserveInitialCapacity(node.positionTicks.size());
for (auto& entry : node.positionTicks)
sortedTicks.append({ entry.key, entry.value });
std::sort(sortedTicks.begin(), sortedTicks.end(), [](const auto& a, const auto& b) {
return a.first < b.first;
});
auto positionTicksArray = JSON::Array::create();
for (auto& [line, ticks] : sortedTicks) {
auto tickObj = JSON::Object::create();
tickObj->setInteger("line"_s, line);
tickObj->setInteger("ticks"_s, ticks);
positionTicksArray->pushValue(tickObj);
}
nodeObj->setValue("positionTicks"_s, positionTicksArray);
}

nodesArray->pushValue(nodeObj);
}
json->setValue("nodes"_s, nodesArray);
Expand Down
Loading
Loading