-
-
Notifications
You must be signed in to change notification settings - Fork 159
fix(perf_hooks): implement class prototype machinery #8254
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| ### fix(perf_hooks): give public classes Node-compatible prototypes and constructors | ||
|
|
||
| The public `node:perf_hooks` classes now expose their real prototype chains, | ||
| property descriptors, accessors, methods, and `Symbol.toStringTag` values. | ||
| Performance entries, observers, the global `performance` object, and observer | ||
| entry lists are linked to the same canonical prototypes, so reflection, | ||
| `instanceof`, extracted prototype calls, and receiver validation agree with | ||
| Node. | ||
|
|
||
| `new PerformanceMark(name, options)` now creates a detached mark with cloned | ||
| detail data without adding it to the performance timeline. The other | ||
| non-publicly-constructible perf classes throw `ERR_ILLEGAL_CONSTRUCTOR`, while | ||
| `PerformanceObserver` retains its supported constructor. Entry-list filters | ||
| also report Node-compatible missing-argument, Symbol-coercion, and invalid-this | ||
| errors across both generic and typed dispatch paths. Fixes #8231. | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -453,6 +453,20 @@ pub fn closure_get_dynamic_prop(ptr: usize, prop: &str) -> f64 { | |
| return f64::from_bits(crate::value::TAG_UNDEFINED); | ||
| } | ||
|
|
||
| // `PerformanceObserver.supportedEntryTypes` is a built-in static accessor: | ||
| // reflection reads its descriptor below, while ordinary property reads | ||
| // must invoke it and receive a fresh frozen array. Native-module class | ||
| // exports otherwise store only ordinary dynamic data properties, so keep | ||
| // this constructor-specific accessor at the common closure read seam. | ||
| if prop == "supportedEntryTypes" { | ||
| let value = crate::value::js_nanbox_pointer(ptr as i64); | ||
| if unsafe { crate::object::bound_native_callable_module_and_method(value) }.is_some_and( | ||
| |(module, method)| module == "perf_hooks" && method == "PerformanceObserver", | ||
| ) { | ||
| return crate::perf_hooks::perf_supported_entry_types_value(); | ||
| } | ||
| } | ||
|
Comment on lines
+456
to
+468
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Description: Check where the PerformanceObserver constructor closure is created and whether the supportedEntryTypes accessor is always installed.
set -euo pipefail
rg -n -C 8 'supportedEntryTypes' --type=rust
# Constructor closure creation paths for PerformanceObserver.
rg -n -C 8 '"PerformanceObserver"' --type=rustRepository: PerryTS/perry Length of output: 151 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- candidate files ---'
git ls-files | rg '(^|/)(dynamic_props\.rs|perf_hooks|performance|native_module|closure).*' | head -200
printf '%s\n' '--- exact symbol locations ---'
rg -n -S 'supportedEntryTypes|PerformanceObserver|attach_perf_hooks_constructor|bound_native_callable_module_and_method|get_accessor_descriptor|js_object_define_property|delete' \
crates/perry-runtime crates/perry-codegen crates/perry-hir --glob '*.rs' || true
printf '%s\n' '--- dynamic_props outline ---'
ast-grep outline crates/perry-runtime/src/closure/dynamic_props.rsRepository: PerryTS/perry Length of output: 50370 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- dynamic property read seam ---'
rg -n -C 18 'supportedEntryTypes|bound_native_callable_module_and_method|get_accessor_descriptor' \
crates/perry-runtime/src/closure/dynamic_props.rs
printf '%s\n' '--- perf_hooks installation paths ---'
rg -n -C 20 'supportedEntryTypes|PerformanceObserver|attach_perf_hooks_constructor|define.*property|accessor' \
crates/perry-runtime/src/perf_hooks.rs \
crates/perry-runtime/src/perf_hooks/prototypes.rs \
crates/perry-codegen/src/lower_call/native/perf_hooks.rs \
crates/perry-codegen/src/lower_call/native_module_dispatch.rs
printf '%s\n' '--- all exact matches in runtime ---'
rg -n -S 'supportedEntryTypes|attach_perf_hooks_constructor|PerformanceObserver' \
crates/perry-runtime/src --glob '*.rs'Repository: PerryTS/perry Length of output: 50369 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- attachment call sites ---'
rg -n -C 12 'attach_perf_hooks_constructor' crates --glob '*.rs'
printf '%s\n' '--- native callable metadata ---'
rg -n -C 18 'bound_native_callable_module_and_method|set_bound_native|native_callable_module|native_callable.*method' \
crates/perry-runtime/src/object crates/perry-runtime/src/closure --glob '*.rs'
printf '%s\n' '--- closure define/delete implementation symbols ---'
rg -n -C 20 'closure_.*define|define.*closure|closure_.*delete|delete.*closure|set_builtin_accessor_descriptor|get_accessor_descriptor' \
crates/perry-runtime/src/closure crates/perry-runtime/src/object --glob '*.rs' \
| head -800
printf '%s\n' '--- focused source ranges ---'
sed -n '231,425p' crates/perry-runtime/src/perf_hooks/prototypes.rs
sed -n '1,240p' crates/perry-runtime/src/object/delete_rest.rsRepository: PerryTS/perry Length of output: 50370 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- exact attachment references ---'
rg -n -F 'attach_perf_hooks_constructor' crates/perry-runtime crates/perry-codegen --glob '*.rs'
printf '%s\n' '--- native export perf range ---'
sed -n '35,180p' crates/perry-runtime/src/object/native_module/callable_exports.rs
printf '%s\n' '--- exact metadata references ---'
rg -n -F 'bound_native_callable_module_and_method' crates/perry-runtime/src --glob '*.rs'
rg -n -F 'BOUND_METHOD_FUNC_PTR' crates/perry-runtime/src/object/native_module --glob '*.rs'
printf '%s\n' '--- exact closure mutation references ---'
rg -n -F 'closure_set_dynamic_prop' crates/perry-runtime/src --glob '*.rs'
rg -n -F 'closure_delete_own_dynamic_prop' crates/perry-runtime/src --glob '*.rs'
rg -n -F 'set_builtin_accessor_descriptor' crates/perry-runtime/src --glob '*.rs'
rg -n -F 'get_accessor_descriptor' crates/perry-runtime/src/closure crates/perry-runtime/src/object --glob '*.rs'Repository: PerryTS/perry Length of output: 33682 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- native metadata implementation ---'
sed -n '1270,1345p' crates/perry-runtime/src/object/native_module/callable_exports.rs
printf '%s\n' '--- closure dynamic mutation implementation ---'
sed -n '700,785p' crates/perry-runtime/src/closure/dynamic_props.rs
printf '%s\n' '--- closure defineProperty path ---'
sed -n '700,815p' crates/perry-runtime/src/object/object_ops/define_property.rs
printf '%s\n' '--- closure delete path ---'
sed -n '160,220p' crates/perry-runtime/src/object/delete_rest.rs
printf '%s\n' '--- descriptor access and builtin descriptor mutation ---'
sed -n '700,815p' crates/perry-runtime/src/object/descriptor_state.rs
sed -n '900,985p' crates/perry-runtime/src/object/descriptor_state.rs
printf '%s\n' '--- alternate PerformanceObserver construction references ---'
rg -n -F 'PerformanceObserver' crates/perry-runtime/src --glob '*.rs' | rg -v 'perf_hooks/prototypes.rs|perf_hooks.rs' || trueRepository: PerryTS/perry Length of output: 27678 🏁 Script executed: #!/bin/bash
set -euo pipefail
python3 - <<'PY'
from pathlib import Path
read = Path("crates/perry-runtime/src/closure/dynamic_props.rs").read_text()
attach = Path("crates/perry-runtime/src/perf_hooks/prototypes.rs").read_text()
export = Path("crates/perry-runtime/src/object/native_module/callable_exports.rs").read_text()
branch = read.index('if prop == "supportedEntryTypes"')
descriptor = read.index('if let Some(acc) = crate::object::get_accessor_descriptor(ptr, prop)')
assert branch < descriptor, "special-case branch is not before descriptor lookup"
install = attach.index('"supportedEntryTypes"')
assert 'PropertyAttrs::new(true, false, true)' in attach[attach.index('if class_name == "PerformanceObserver"'):], \
"accessor is not configurable"
call = export.index('attach_perf_hooks_constructor')
assert call >= 0
print("branch_before_descriptor_lookup: yes")
print("configurable_accessor_installation: yes")
print("common_perf_export_attachment_callsite: yes")
PYRepository: PerryTS/perry Length of output: 272 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- deleted-key helpers ---'
rg -n -C 18 'pub fn closure_mark_key_deleted|fn closure_mark_key_deleted|closure_own_key_present|get_closure_deleted_keys' \
crates/perry-runtime/src/closure crates/perry-runtime/src/object --glob '*.rs'
printf '%s\n' '--- exact closure delete branch through accessor lookup ---'
sed -n '180,225p' crates/perry-runtime/src/object/delete_rest.rs
sed -n '700,765p' crates/perry-runtime/src/object/descriptor_state.rs
printf '%s\n' '--- focused source verifier ---'
python3 - <<'PY'
from pathlib import Path
delete = Path("crates/perry-runtime/src/object/delete_rest.rs").read_text()
descriptor = Path("crates/perry-runtime/src/object/descriptor_state.rs").read_text()
dynamic = Path("crates/perry-runtime/src/closure/dynamic_props.rs").read_text()
define = Path("crates/perry-runtime/src/object/object_ops/define_property.rs").read_text()
closure_delete = delete[delete.index('if crate::closure::is_closure_ptr(obj as usize)'):]
closure_delete = closure_delete[:closure_delete.index('return 1;', closure_delete.index('if let Some(name)')) + len('return 1;')]
assert 'closure_delete_own_dynamic_prop' in closure_delete
assert 'closure_mark_key_deleted' in closure_delete
assert 'clear_accessor_descriptor' not in closure_delete
get_acc = descriptor[descriptor.index('pub(crate) fn get_accessor_descriptor'):descriptor.index('pub(crate) fn accessor_descriptor_keys_for_obj')]
assert 'deleted' not in get_acc
branch = dynamic.index('if prop == "supportedEntryTypes"')
lookup = dynamic.index('get_accessor_descriptor(ptr, prop)', branch)
assert branch < lookup
define_start = define.index('if has_accessor {')
define_end = define.index('let read_bool', define_start)
define_section = define[define_start:define_end]
assert 'accessor_descriptors' in define_section and 'remove' in define_section
print("define_data_descriptor_removes_accessor: yes")
print("closure_delete_does_not_clear_accessor: yes")
print("accessor_lookup_ignores_deleted_keys: yes")
print("special_case_precedes_accessor_lookup: yes")
PYRepository: PerryTS/perry Length of output: 50369 Remove the The interception runs before 🤖 Prompt for AI Agents |
||
|
|
||
| if let Some(acc) = crate::object::get_accessor_descriptor(ptr, prop) { | ||
| if acc.get == 0 { | ||
| return f64::from_bits(crate::value::TAG_UNDEFINED); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1051,6 +1051,26 @@ pub unsafe extern "C-unwind" fn js_native_call_method( | |
| args_ptr: *const f64, | ||
| args_len: usize, | ||
| ) -> f64 { | ||
| // PerformanceObserverEntryList is a native namespace receiver, and typed | ||
| // feedback can dispatch its methods before the generic prototype/native- | ||
| // module tower below. Validate the WebIDL-required filter argument at this | ||
| // common entry so direct calls and extracted prototype calls agree. | ||
| if method_name_len == 16 | ||
| && !method_name_ptr.is_null() | ||
| && crate::perf_hooks::is_perf_observer_list_value(object) | ||
| { | ||
| let name = std::slice::from_raw_parts(method_name_ptr as *const u8, method_name_len); | ||
| let arg0 = if args_len > 0 && !args_ptr.is_null() { | ||
| *args_ptr | ||
| } else { | ||
| f64::from_bits(crate::value::TAG_UNDEFINED) | ||
| }; | ||
| if name == b"getEntriesByName" { | ||
| crate::perf_hooks::validate_perf_list_filter_arg(arg0, "name", args_len == 0); | ||
| } else if name == b"getEntriesByType" { | ||
| crate::perf_hooks::validate_perf_list_filter_arg(arg0, "type", args_len == 0); | ||
| } | ||
| } | ||
|
Comment on lines
+1054
to
+1073
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Description: Show is_perf_observer_list_value and check whether it guards the handle band before reading field 0.
set -euo pipefail
ast-grep run --pattern 'fn is_perf_observer_list_value($$$) { $$$ }' --lang rust crates/perry-runtime/src/perf_hooks.rs
rg -n -C 10 'is_perf_observer_list_value' --type=rustRepository: PerryTS/perry Length of output: 151 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- candidate definitions/usages ---'
rg -n -C 12 'is_perf_observer(_list)?_value|fn is_perf_observer' crates/perry-runtime/src/perf_hooks.rs crates/perry-runtime/src/object/native_call_method.rs || true
printf '%s\n' '--- relevant address helpers and native-call context ---'
rg -n -C 8 'is_plausible_heap_addr|js_nanbox_get_pointer|handle.band|handle band|is_valid_obj_ptr' crates/perry-runtime/src/object/native_call_method.rs crates/perry-runtime/src/perf_hooks.rs crates/perry-runtime/src/value || true
printf '%s\n' '--- file layout ---'
wc -l crates/perry-runtime/src/perf_hooks.rs crates/perry-runtime/src/object/native_call_method.rsRepository: PerryTS/perry Length of output: 50370 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '--- exact predicates ---'
sed -n '78,155p' crates/perry-runtime/src/value/addr_class.rs
printf '%s\n' '--- exact observer predicates ---'
sed -n '235,275p' crates/perry-runtime/src/perf_hooks.rs
printf '%s\n' '--- call-site block ---'
sed -n '1048,1075p' crates/perry-runtime/src/object/native_call_method.rs
printf '%s\n' '--- read-only structural check ---'
python3 - <<'PY'
from pathlib import Path
import re
perf = Path("crates/perry-runtime/src/perf_hooks.rs").read_text()
call = Path("crates/perry-runtime/src/object/native_call_method.rs").read_text()
addr = Path("crates/perry-runtime/src/value/addr_class.rs").read_text()
m = re.search(
r"pub\(crate\) fn is_perf_observer_list_value\(value: f64\) -> bool \{(.*?)\n\}",
perf,
re.S,
)
assert m, "observer-list predicate not found"
body = m.group(1)
field = body.find("js_object_get_field(obj, 0)")
guard = min(
[i for i in (body.find("is_plausible_heap_addr"), body.find("is_above_handle_band"))
if i >= 0],
default=-1,
)
print("observer_list_field_read_offset=", field)
print("observer_list_address_guard_offset=", guard)
print("guard_precedes_field_read=", 0 <= guard < field)
cm = re.search(
r"if method_name_len == 16.*?is_perf_observer_list_value\(object\).*?\n \}",
call,
re.S,
)
assert cm, "native-call probe not found"
probe = cm.group(0)
print("call_site_has_plausible_heap_guard=",
"is_plausible_heap_addr" in probe)
print("call_site_has_above_handle_guard=",
"is_above_handle_band" in probe)
print("canonical_plausible_predicate_defined=",
"fn is_plausible_heap_addr" in addr)
PYRepository: PerryTS/perry Length of output: 7135 🏁 Script executed: #!/bin/bash
set -euo pipefail
rg -n -A 18 -B 4 'pub fn is_plausible_heap_addr|fn is_plausible_heap_addr' crates/perry-runtime/src/value/addr_class.rsRepository: PerryTS/perry Length of output: 1369 Guard
🤖 Prompt for AI AgentsSource: Learnings |
||
| // #7769: the tower's own previously-computed answer for this | ||
| // (class_id, method_name) pair, when the receiver still satisfies every | ||
| // per-object precondition. See `try_class_vtable_fast_dispatch`. | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -33,6 +33,11 @@ use std::cell::{Cell, RefCell}; | |
| use std::sync::{Once, OnceLock}; | ||
| use std::time::{Instant, SystemTime, UNIX_EPOCH}; | ||
|
|
||
| mod prototypes; | ||
|
|
||
| pub(crate) use prototypes::{attach_perf_hooks_constructor, perf_supported_entry_types_value}; | ||
| use prototypes::{is_perf_constructor_name, link_perf_prototype}; | ||
|
|
||
| const ENTRY_TYPE_MARK: u8 = 0; | ||
| const ENTRY_TYPE_MEASURE: u8 = 1; | ||
| const ENTRY_TYPE_RESOURCE: u8 = 2; | ||
|
|
@@ -327,6 +332,7 @@ pub fn performance_namespace() -> f64 { | |
| } | ||
| let module = b"perf_hooks"; | ||
| let ns = crate::object::js_create_native_module_namespace(module.as_ptr(), module.len()); | ||
| let ns = link_perf_prototype(ns, "Performance"); | ||
| PERFORMANCE_NS.with(|c| c.set(ns.to_bits())); | ||
| ns | ||
| } | ||
|
|
@@ -549,7 +555,13 @@ unsafe fn entry_to_object(e: &PerfEntry) -> f64 { | |
| if let Some(initiator_type) = &e.initiator_type { | ||
| set_named_field(obj, "initiatorType", str_value(initiator_type)); | ||
| } | ||
| crate::value::js_nanbox_pointer(obj as i64) | ||
| let class_name = match e.entry_type { | ||
| ENTRY_TYPE_MARK => "PerformanceMark", | ||
| ENTRY_TYPE_MEASURE => "PerformanceMeasure", | ||
| ENTRY_TYPE_RESOURCE => "PerformanceResourceTiming", | ||
| _ => "PerformanceEntry", | ||
| }; | ||
| link_perf_prototype(crate::value::js_nanbox_pointer(obj as i64), class_name) | ||
| } | ||
|
|
||
| /// `performance.now()` reading used for default mark startTimes / measure | ||
|
|
@@ -746,6 +758,75 @@ pub extern "C" fn js_perf_mark(name_val: f64, options_val: f64) -> f64 { | |
| } | ||
| } | ||
|
|
||
| /// `new PerformanceMark(name, options?)` creates a detached mark. Node clones | ||
| /// `detail` exactly like `performance.mark`, but does not append the result to | ||
| /// the global performance timeline or notify observers. | ||
| #[no_mangle] | ||
| pub extern "C" fn js_perf_mark_constructor(name_val: f64, options_val: f64) -> f64 { | ||
| unsafe { | ||
| if crate::symbol::js_is_symbol(name_val) != 0 { | ||
| throw_type_error("Cannot convert a Symbol value to a string"); | ||
| } | ||
| let name = coerce_to_string(name_val); | ||
| let mut start_time = perf_now(); | ||
| let mut detail_bits = JSValue::null().bits(); | ||
| if let Some(opts) = as_object_ptr(options_val) { | ||
| if option_present(opts, "startTime") { | ||
| match option_number(opts, "startTime") { | ||
| Some(st) => { | ||
| validate_user_timing_timestamp(st); | ||
| start_time = st; | ||
| } | ||
| None => throw_type_error_with_code( | ||
| "The \"startTime\" option must be of type number", | ||
| "ERR_INVALID_ARG_TYPE", | ||
| ), | ||
| } | ||
| } | ||
| detail_bits = option_detail_bits(opts); | ||
| } | ||
| let entry = PerfEntry { | ||
| name, | ||
| entry_type: ENTRY_TYPE_MARK, | ||
| start_time, | ||
| duration: 0.0, | ||
| detail_bits, | ||
| object_bits: 0, | ||
| initiator_type: None, | ||
| }; | ||
| entry_to_object(&entry) | ||
| } | ||
| } | ||
|
Comment on lines
+761
to
+799
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🌐 Web query:
💡 Result: Yes, in recent versions of Node.js, calling new PerformanceMark("nodeStart") will throw an error [1]. Node.js internally reserves certain names for performance milestones—such as "nodeStart", "environment", "v8Start", "loopStart", "loopExit", and "bootstrapComplete"—to track internal process timing [2][3]. To prevent user-defined marks from conflicting with these reserved internal milestones, the PerformanceMark constructor explicitly checks the provided name against a list of these reserved attributes [1]. If a match is found, it throws an ERR_INVALID_ARG_VALUE error [1]. Citations:
🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- relevant symbols ---'
rg -n -C 6 'js_perf_mark_constructor|js_perf_mark|reject_reserved_milestone_name|reserved.*milestone|nodeStart' crates/perry-runtime/src/perf_hooks.rs crates/perry-runtime/src || true
printf '%s\n' '--- nearby implementation ---'
sed -n '680,815p' crates/perry-runtime/src/perf_hooks.rsRepository: PerryTS/perry Length of output: 35736 🏁 Script executed: #!/bin/bash
set -eu
if command -v node >/dev/null 2>&1; then
node - <<'JS'
const { PerformanceMark, performance } = require('node:perf_hooks');
for (const name of ['nodeStart', 'v8Start', 'environment', 'loopStart',
'loopExit', 'bootstrapComplete', 'userMark']) {
for (const [label, fn] of [
['PerformanceMark', () => new PerformanceMark(name)],
['performance.mark', () => performance.mark(name)],
]) {
try {
fn();
console.log(`${label}(${name}): accepted`);
} catch (error) {
console.log(`${label}(${name}): ${error.code ?? error.name}`);
}
}
}
JS
else
echo "node is unavailable"
fiRepository: PerryTS/perry Length of output: 844 Reject reserved milestone names in
🤖 Prompt for AI Agents |
||
|
|
||
| #[no_mangle] | ||
| pub extern "C" fn js_perf_illegal_constructor() -> f64 { | ||
| throw_type_error_with_code("Illegal constructor", "ERR_ILLEGAL_CONSTRUCTOR") | ||
| } | ||
|
|
||
| pub(crate) unsafe fn construct_perf_hooks_class( | ||
| class_name: &str, | ||
| args_ptr: *const f64, | ||
| args_len: usize, | ||
| ) -> Option<f64> { | ||
| if !is_perf_constructor_name(class_name) { | ||
| return None; | ||
| } | ||
| let args = if args_ptr.is_null() { | ||
| &[][..] | ||
| } else { | ||
| std::slice::from_raw_parts(args_ptr, args_len) | ||
| }; | ||
| let undefined = f64::from_bits(crate::value::TAG_UNDEFINED); | ||
| Some(match class_name { | ||
| "PerformanceMark" => js_perf_mark_constructor( | ||
| args.first().copied().unwrap_or(undefined), | ||
| args.get(1).copied().unwrap_or(undefined), | ||
| ), | ||
| "PerformanceObserver" => js_perf_observer_new(args.first().copied().unwrap_or(undefined)), | ||
| _ => js_perf_illegal_constructor(), | ||
| }) | ||
| } | ||
|
|
||
| // ── performance.measure(name, startOrOptions?, end?) ───────────────────────── | ||
| /// Computes startTime/duration from positional marks or an options object, | ||
| /// appends a PerformanceMeasure to the timeline, and returns it. | ||
|
|
@@ -1366,7 +1447,19 @@ unsafe fn make_observer_object(id: usize) -> f64 { | |
| keys = crate::array::js_array_push(keys, JSValue::string_ptr(kp)); | ||
| } | ||
| crate::object::js_object_set_keys(obj, keys); | ||
| crate::value::js_nanbox_pointer(obj as i64) | ||
| link_perf_prototype( | ||
| crate::value::js_nanbox_pointer(obj as i64), | ||
| "PerformanceObserver", | ||
| ) | ||
| } | ||
|
|
||
| fn is_perf_observer_value(value: f64) -> bool { | ||
| unsafe { | ||
| let Some(obj) = as_object_ptr(value) else { | ||
| return false; | ||
| }; | ||
| string_of(js_object_get_field(obj, 0)).as_deref() == Some("perf_observer") | ||
| } | ||
| } | ||
|
|
||
| /// True if `v` is callable (matches `typeof v === "function"`) — covers | ||
|
|
@@ -1675,6 +1768,7 @@ pub extern "C" fn js_perf_observer_flush_all( | |
| let module = b"perf_observer_list"; | ||
| let list = | ||
| crate::object::js_create_native_module_namespace(module.as_ptr(), module.len()); | ||
| let list = link_perf_prototype(list, "PerformanceObserverEntryList"); | ||
| let cb_jv = JSValue::from_bits(cb_bits); | ||
| if cb_jv.is_pointer() { | ||
| // Node invokes the callback as `(list, observer)` with `this` | ||
|
|
@@ -1714,6 +1808,18 @@ pub unsafe fn current_list_get_entries() -> f64 { | |
| current_list_to_array(|_| true) | ||
| } | ||
|
|
||
| pub(crate) fn validate_perf_list_filter_arg(value: f64, name: &str, missing: bool) { | ||
| if missing || JSValue::from_bits(value.to_bits()).is_undefined() { | ||
| throw_type_error_with_code( | ||
| &format!("The \"{name}\" argument must be specified"), | ||
| "ERR_MISSING_ARGS", | ||
| ); | ||
| } | ||
| if unsafe { crate::symbol::js_is_symbol(value) } != 0 { | ||
| throw_type_error("Cannot convert a Symbol value to a string"); | ||
| } | ||
| } | ||
|
Comment on lines
+1811
to
+1821
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win 🧩 Analysis chain🌐 Web query:
💡 Result: In Node.js, the Citations:
🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- relevant implementation and tests ---'
rg -n -C 5 'validate_perf_list_filter_arg|getEntriesByType|ERR_MISSING_ARGS|Cannot convert a Symbol' crates/perry-runtime/src crates/perry-runtime/tests tests 2>/dev/null | head -n 400
printf '%s\n' '--- Node runtime behavior ---'
node - <<'JS'
const { performance, PerformanceObserver } = require('node:perf_hooks');
const list = {
getEntriesByType: PerformanceObserver
? null
: null
};
const cases = [
['performance.getEntriesByType()', () => performance.getEntriesByType()],
['performance.getEntriesByType(undefined)', () => performance.getEntriesByType(undefined)],
['performance.getEntriesByType("mark")', () => performance.getEntriesByType('mark')],
];
for (const [label, fn] of cases) {
try {
const result = fn();
console.log(label, '=>', JSON.stringify(result.map(e => e.name)));
} catch (error) {
console.log(label, 'THROW', error.name, error.code ?? null, JSON.stringify(error.message));
}
}
const observer = new PerformanceObserver((entryList) => {
for (const [label, fn] of [
['observer list.getEntriesByType()', () => entryList.getEntriesByType()],
['observer list.getEntriesByType(undefined)', () => entryList.getEntriesByType(undefined)],
['observer list.getEntriesByType("mark")', () => entryList.getEntriesByType('mark')],
]) {
try {
const result = fn();
console.log(label, '=>', JSON.stringify(result.map(e => e.name)));
} catch (error) {
console.log(label, 'THROW', error.name, error.code ?? null, JSON.stringify(error.message));
}
}
observer.disconnect();
});
performance.mark('__review_probe__');
observer.observe({ entryTypes: ['mark'] });
setImmediate(() => {});
JSRepository: PerryTS/perry Length of output: 34122 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- observer-list behavior and Node version ---'
node - <<'JS'
const { performance, PerformanceObserver } = require('node:perf_hooks');
console.log('node', process.version);
const observer = new PerformanceObserver((entryList) => {
for (const [label, fn] of [
['list.getEntriesByType()', () => entryList.getEntriesByType()],
['list.getEntriesByType(undefined)', () => entryList.getEntriesByType(undefined)],
['list.getEntriesByType("mark")', () => entryList.getEntriesByType('mark')],
]) {
try {
const result = fn();
console.log(label, '=>', JSON.stringify(result.map(e => e.name)));
} catch (error) {
console.log(label, 'THROW', error.name, error.code ?? null, JSON.stringify(error.message));
}
}
observer.disconnect();
});
observer.observe({ entryTypes: ['mark'] });
performance.mark('__review_probe__');
setTimeout(() => {}, 25);
JS
printf '%s\n' '--- declarations, lowering, and call sites ---'
rg -n -C 8 'validate_perf_list_filter_arg|current_list_get_by_type|perf_list_get_by_type|missing.*arg|arg_count|getEntriesByType' crates/perry-runtime/src/perf_hooks.rs crates/perry-runtime/src/perf_hooks crates/perry-codegen 2>/dev/null | head -n 500Repository: PerryTS/perry Length of output: 47100 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- all references to the validator ---'
rg -n -F 'validate_perf_list_filter_arg' . || true
printf '%s\n' '--- surrounding list implementations ---'
sed -n '1760,1855p' crates/perry-runtime/src/perf_hooks.rs
sed -n '90,145p' crates/perry-runtime/src/perf_hooks/prototypes.rs
printf '%s\n' '--- native ABI metadata for perf list methods ---'
rg -n -C 6 'perf_list_get_by_type_thunk|PerformanceObserverEntryList|getEntriesByType' crates/perry-runtime crates/perry-codegen | head -n 350Repository: PerryTS/perry Length of output: 41496 Preserve the distinction between omitted and explicit 🤖 Prompt for AI Agents |
||
|
|
||
| pub unsafe fn current_list_get_by_type(type_val: f64) -> f64 { | ||
| require_entry_query_argument(type_val, "type"); | ||
| let want = coerce_to_string(type_val); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Add root-cause, affected-path, and validation details.
This fragment describes the shipped behavior, but it does not explain the root cause, identify affected files, or record validation results. Add these details to make the release note complete.
Based on learnings: PerryTS/perry changelog fragments should include a long-form root-cause explanation, affected file paths, and validation notes.
Suggested changeset expansion
📝 Committable suggestion
🤖 Prompt for AI Agents
Source: Learnings