Skip to content
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
17 commits
Select commit Hold shift + click to select a range
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
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 @@ struct ProfileNode {
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

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 stopCPUProfilerAndGetJSON(JSC::VM& vm)
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 @@ WTF::String stopCPUProfilerAndGetJSON(JSC::VM& vm)
}
}

// 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 @@ WTF::String stopCPUProfilerAndGetJSON(JSC::VM& vm)
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 @@ WTF::String stopCPUProfilerAndGetJSON(JSC::VM& vm)
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 @@ WTF::String stopCPUProfilerAndGetJSON(JSC::VM& vm)

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 @@ -313,6 +375,26 @@ WTF::String stopCPUProfilerAndGetJSON(JSC::VM& vm)
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);
}
Comment thread
robobun marked this conversation as resolved.
Outdated
json->setValue("nodes"_s, nodesArray);
Expand Down Expand Up @@ -638,8 +720,11 @@ void stopCPUProfiler(JSC::VM& vm, WTF::String* outJSON, WTF::String* outText)
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 @@ void stopCPUProfiler(JSC::VM& vm, WTF::String* outJSON, WTF::String* outText)
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 @@ void stopCPUProfiler(JSC::VM& vm, WTF::String* outJSON, WTF::String* outText)
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 @@ void stopCPUProfiler(JSC::VM& vm, WTF::String* outJSON, WTF::String* outText)
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 @@ void stopCPUProfiler(JSC::VM& vm, WTF::String* outJSON, WTF::String* outText)

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 @@ void stopCPUProfiler(JSC::VM& vm, WTF::String* outJSON, WTF::String* outText)
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