mirror of
https://github.com/oven-sh/bun
synced 2026-02-09 10:28:47 +00:00
Address PR feedback
- Use Bun:: namespace functions instead of Bun__ extern "C" wrappers - Use createNotEnoughArgumentsError() for missing arguments - Use Bun::V::validateInteger() for proper integer validation - Remove unused extern "C" wrappers (setSamplingInterval, isCPUProfilerRunning) - Reset sampling interval to default in profile() when not specified - Delete flaky timestamp test (low value, wrong assumptions)
This commit is contained in:
@@ -18,8 +18,6 @@
|
||||
|
||||
extern "C" void Bun__startCPUProfiler(JSC::VM* vm);
|
||||
extern "C" BunString Bun__stopCPUProfilerAndGetJSON(JSC::VM* vm);
|
||||
extern "C" void Bun__setCPUSamplingInterval(int intervalMicroseconds);
|
||||
extern "C" bool Bun__isCPUProfilerRunning();
|
||||
|
||||
namespace Bun {
|
||||
|
||||
@@ -348,13 +346,3 @@ extern "C" BunString Bun__stopCPUProfilerAndGetJSON(JSC::VM* vm)
|
||||
WTF::String result = Bun::stopCPUProfilerAndGetJSON(*vm);
|
||||
return Bun::toStringRef(result);
|
||||
}
|
||||
|
||||
extern "C" void Bun__setCPUSamplingInterval(int intervalMicroseconds)
|
||||
{
|
||||
Bun::setSamplingInterval(intervalMicroseconds);
|
||||
}
|
||||
|
||||
extern "C" bool Bun__isCPUProfilerRunning()
|
||||
{
|
||||
return Bun::isCPUProfilerRunning();
|
||||
}
|
||||
|
||||
@@ -1,36 +1,26 @@
|
||||
#include "root.h"
|
||||
#include "BunString.h"
|
||||
#include "helpers.h"
|
||||
#include "BunCPUProfiler.h"
|
||||
#include "NodeValidator.h"
|
||||
#include <JavaScriptCore/JSGlobalObject.h>
|
||||
#include <JavaScriptCore/VM.h>
|
||||
#include <JavaScriptCore/Error.h>
|
||||
|
||||
using namespace JSC;
|
||||
|
||||
extern "C" void Bun__startCPUProfiler(JSC::VM* vm);
|
||||
extern "C" BunString Bun__stopCPUProfilerAndGetJSON(JSC::VM* vm);
|
||||
extern "C" void Bun__setCPUSamplingInterval(int intervalMicroseconds);
|
||||
extern "C" bool Bun__isCPUProfilerRunning();
|
||||
|
||||
JSC_DECLARE_HOST_FUNCTION(jsFunction_startCPUProfiler);
|
||||
JSC_DEFINE_HOST_FUNCTION(jsFunction_startCPUProfiler, (JSGlobalObject * globalObject, CallFrame*))
|
||||
{
|
||||
Bun__startCPUProfiler(&globalObject->vm());
|
||||
Bun::startCPUProfiler(globalObject->vm());
|
||||
return JSValue::encode(jsUndefined());
|
||||
}
|
||||
|
||||
JSC_DECLARE_HOST_FUNCTION(jsFunction_stopCPUProfiler);
|
||||
JSC_DEFINE_HOST_FUNCTION(jsFunction_stopCPUProfiler, (JSGlobalObject * globalObject, CallFrame*))
|
||||
{
|
||||
BunString result = Bun__stopCPUProfilerAndGetJSON(&globalObject->vm());
|
||||
|
||||
if (result.tag == BunStringTag::Empty || result.tag == BunStringTag::Dead) {
|
||||
return JSValue::encode(jsEmptyString(globalObject->vm()));
|
||||
}
|
||||
|
||||
WTF::String wtfString = result.toWTFString();
|
||||
result.deref();
|
||||
|
||||
return JSValue::encode(jsString(globalObject->vm(), wtfString));
|
||||
auto& vm = globalObject->vm();
|
||||
auto result = Bun::stopCPUProfilerAndGetJSON(vm);
|
||||
return JSValue::encode(jsString(vm, result));
|
||||
}
|
||||
|
||||
JSC_DECLARE_HOST_FUNCTION(jsFunction_setCPUSamplingInterval);
|
||||
@@ -40,28 +30,20 @@ JSC_DEFINE_HOST_FUNCTION(jsFunction_setCPUSamplingInterval, (JSGlobalObject * gl
|
||||
auto scope = DECLARE_THROW_SCOPE(vm);
|
||||
|
||||
if (callFrame->argumentCount() < 1) {
|
||||
throwTypeError(globalObject, scope, "setSamplingInterval requires an interval argument"_s);
|
||||
throwVMError(globalObject, scope, createNotEnoughArgumentsError(globalObject));
|
||||
return {};
|
||||
}
|
||||
|
||||
JSValue intervalArg = callFrame->uncheckedArgument(0);
|
||||
if (!intervalArg.isNumber()) {
|
||||
throwTypeError(globalObject, scope, "interval must be a number"_s);
|
||||
return {};
|
||||
}
|
||||
int interval;
|
||||
Bun::V::validateInteger(scope, globalObject, callFrame->uncheckedArgument(0), "interval"_s, jsNumber(1), jsUndefined(), &interval);
|
||||
RETURN_IF_EXCEPTION(scope, {});
|
||||
|
||||
int interval = static_cast<int>(intervalArg.asNumber());
|
||||
if (interval <= 0) {
|
||||
throwRangeError(globalObject, scope, "interval must be a positive number"_s);
|
||||
return {};
|
||||
}
|
||||
|
||||
Bun__setCPUSamplingInterval(interval);
|
||||
Bun::setSamplingInterval(interval);
|
||||
return JSValue::encode(jsUndefined());
|
||||
}
|
||||
|
||||
JSC_DECLARE_HOST_FUNCTION(jsFunction_isCPUProfilerRunning);
|
||||
JSC_DEFINE_HOST_FUNCTION(jsFunction_isCPUProfilerRunning, (JSGlobalObject*, CallFrame*))
|
||||
{
|
||||
return JSValue::encode(jsBoolean(Bun__isCPUProfilerRunning()));
|
||||
return JSValue::encode(jsBoolean(Bun::isCPUProfilerRunning()));
|
||||
}
|
||||
|
||||
@@ -655,6 +655,10 @@ JSC_DEFINE_HOST_FUNCTION(functionRunProfiler, (JSGlobalObject * globalObject, Ca
|
||||
if (sampleValue.isNumber()) {
|
||||
unsigned sampleInterval = sampleValue.toUInt32(globalObject);
|
||||
samplingProfiler.setTimingInterval(Seconds::fromMicroseconds(sampleInterval));
|
||||
} else {
|
||||
// Reset to default interval (1000 microseconds) to ensure each profile()
|
||||
// call is independent of previous calls
|
||||
samplingProfiler.setTimingInterval(Seconds::fromMicroseconds(1000));
|
||||
}
|
||||
|
||||
const auto report = [](JSC::VM& vm,
|
||||
|
||||
@@ -169,33 +169,6 @@ describe("node:inspector", () => {
|
||||
expect(disableResult).toEqual({});
|
||||
});
|
||||
|
||||
test("profile timestamps are in microseconds", () => {
|
||||
session.post("Profiler.enable");
|
||||
session.post("Profiler.start");
|
||||
|
||||
// Do enough work to ensure we get samples
|
||||
function fibonacci(n: number): number {
|
||||
if (n <= 1) return n;
|
||||
return fibonacci(n - 1) + fibonacci(n - 2);
|
||||
}
|
||||
fibonacci(25); // This should take enough time to get samples
|
||||
|
||||
const result = session.post("Profiler.stop");
|
||||
const profile = result.profile;
|
||||
|
||||
// If we got samples, timestamps should be in microseconds
|
||||
if (profile.samples.length > 0) {
|
||||
// Timestamps should be in microseconds (large numbers)
|
||||
// Microseconds since Unix epoch for 2020 would be around 1577836800000000
|
||||
expect(profile.startTime).toBeGreaterThan(1000000000000000);
|
||||
expect(profile.endTime).toBeGreaterThanOrEqual(profile.startTime);
|
||||
} else {
|
||||
// If no samples, startTime and endTime will be 0 (valid empty profile)
|
||||
expect(profile.startTime).toBe(0);
|
||||
expect(profile.endTime).toBe(0);
|
||||
}
|
||||
});
|
||||
|
||||
test("samples and timeDeltas have same length", () => {
|
||||
session.post("Profiler.enable");
|
||||
session.post("Profiler.start");
|
||||
|
||||
Reference in New Issue
Block a user