Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions changelog.d/7435-gate-url-shape-probes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
size: the URL-shape probes on generic value paths (`ToString`, `+` coercion, `JSON.stringify`, `Object.fromEntries`, by-name field set) are now compiled only under `url-engine` — they were pinning the URL parser into every binary. Hello world drops 33,064 bytes (4,708,736 → 4,675,672).
2 changes: 2 additions & 0 deletions crates/perry-runtime/src/json/replacer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -274,6 +274,7 @@ unsafe fn dispatch_pointer_with_replacer(
// the URL, so the generic object walk below would trip the circular-
// structure detector. The href is a plain string, so the emit is identical
// for compact and pretty walks. See `write_url_href_json`.
#[cfg(feature = "url-engine")]
if crate::url::is_url_object_shape(ptr as *mut crate::ObjectHeader) {
super::stringify::write_url_href_json(ptr as *mut crate::ObjectHeader, buf);
return;
Expand Down Expand Up @@ -823,6 +824,7 @@ pub(crate) unsafe fn stringify_value_pretty(
// be walked as a plain object (its `searchParams` back-reference trips
// the circular-structure detector). Mirrors the compact-path branch in
// `stringify_object_inner`; see `write_url_href_json`.
#[cfg(feature = "url-engine")]
if crate::url::is_url_object_shape(ptr as *mut crate::ObjectHeader) {
super::stringify::write_url_href_json(ptr as *mut crate::ObjectHeader, buf);
return;
Expand Down
5 changes: 5 additions & 0 deletions crates/perry-runtime/src/object/field_set_by_name/tail.rs
Original file line number Diff line number Diff line change
Expand Up @@ -314,6 +314,11 @@ pub(super) fn set_field_by_name_object_tail(
}
}

// Binary size: this whole arm statically references eight
// `js_url_set_*` entry points, which is what kept the URL parser in
// every binary. Gated with the other URL shape probes; see
// `value/to_string.rs`.
#[cfg(feature = "url-engine")]
if !plan_fast
&& !key.is_null()
&& (key as usize) > 0x10000
Expand Down
4 changes: 4 additions & 0 deletions crates/perry-runtime/src/object/object_ops/from_entries.rs
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ unsafe fn object_from_entries_has_iterator(value: f64, raw: i64, gc_type: Option
| Some(crate::gc::GC_TYPE_SET) => return true,
Some(crate::gc::GC_TYPE_OBJECT) => {
let obj = raw as *mut ObjectHeader;
#[cfg(feature = "url-engine")]
if crate::url::try_read_as_search_params(obj).is_some() {
return true;
}
Expand Down Expand Up @@ -107,6 +108,9 @@ unsafe fn object_from_entries_materialize_entries(entries_value: f64) -> *mut Ar
return crate::map::js_map_entries(raw as *const crate::map::MapHeader);
}

// The whole arm exists to serve `URLSearchParams`, so it goes with the
// feature rather than leaving `obj` bound for a body that isn't compiled.
#[cfg(feature = "url-engine")]
if gc_type == Some(crate::gc::GC_TYPE_OBJECT) {
let obj = raw as *mut ObjectHeader;
if crate::url::try_read_as_search_params(obj).is_some() {
Expand Down
29 changes: 18 additions & 11 deletions crates/perry-runtime/src/value/dynamic_arith.rs
Original file line number Diff line number Diff line change
Expand Up @@ -276,17 +276,24 @@ unsafe fn to_primitive_default_for_add(value: f64) -> f64 {
// registry ids, not heap `ObjectHeader`s, so the shape probe would
// dereference unmapped memory.
if !crate::value::addr_class::is_handle_band(ptr) {
let boxed =
f64::from_bits(crate::value::POINTER_TAG | ((ptr as u64) & crate::value::POINTER_MASK));
let href = crate::url::url_class::js_url_href_if_url(boxed);
if href.to_bits() != crate::value::TAG_UNDEFINED {
let s = js_jsvalue_to_string(href);
return crate::value::js_nanbox_string(s as i64);
}
let obj = ptr as *mut crate::object::ObjectHeader;
if crate::url::try_read_as_search_params(obj).is_some() {
let s = crate::url::search_params::js_url_search_params_to_string(obj);
return crate::value::js_nanbox_string(s as i64);
// See the matching note in `value/to_string.rs`: shape probes on a
// generic path, kept out of non-URL binaries so the parser can strip.
// `boxed` is bound inside the gate: it feeds only these probes.
#[cfg(feature = "url-engine")]
{
let boxed = f64::from_bits(
crate::value::POINTER_TAG | ((ptr as u64) & crate::value::POINTER_MASK),
);
let href = crate::url::url_class::js_url_href_if_url(boxed);
if href.to_bits() != crate::value::TAG_UNDEFINED {
let s = js_jsvalue_to_string(href);
return crate::value::js_nanbox_string(s as i64);
}
let obj = ptr as *mut crate::object::ObjectHeader;
if crate::url::try_read_as_search_params(obj).is_some() {
let s = crate::url::search_params::js_url_search_params_to_string(obj);
return crate::value::js_nanbox_string(s as i64);
}
}
}

Expand Down
30 changes: 21 additions & 9 deletions crates/perry-runtime/src/value/to_string.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1163,17 +1163,29 @@ pub extern "C" fn js_jsvalue_to_string(value: f64) -> *mut crate::string::String
// heap `ObjectHeader`s, so the shape check would dereference
// unmapped memory.
if !crate::value::addr_class::is_handle_band(ptr as usize) {
let boxed = f64::from_bits(POINTER_TAG | ((ptr as u64) & POINTER_MASK));
let url_href = crate::url::url_class::js_url_href_if_url(boxed);
if url_href.to_bits() != crate::value::TAG_UNDEFINED {
return js_jsvalue_to_string(url_href);
}
if crate::url::try_read_as_search_params(ptr as *mut crate::object::ObjectHeader)
.is_some()
// Binary size: these are SHAPE probes on a generic path, so the
// static reference keeps the whole URL class + parser alive in
// every binary even though the runtime check can never pass
// without `url-engine`. `uses_url` (zero-false-negative by
// construction) is what turns that feature on, so a program
// with no URL API cannot own a URL or URLSearchParams here.
// `boxed` is bound inside the gate: it feeds only these probes.
#[cfg(feature = "url-engine")]
{
return crate::url::search_params::js_url_search_params_to_string(
let boxed = f64::from_bits(POINTER_TAG | ((ptr as u64) & POINTER_MASK));
let url_href = crate::url::url_class::js_url_href_if_url(boxed);
if url_href.to_bits() != crate::value::TAG_UNDEFINED {
return js_jsvalue_to_string(url_href);
}
if crate::url::try_read_as_search_params(
ptr as *mut crate::object::ObjectHeader,
);
)
.is_some()
{
return crate::url::search_params::js_url_search_params_to_string(
ptr as *mut crate::object::ObjectHeader,
);
}
}
}
// OrdinaryToPrimitive(obj, "string"): the object has no
Expand Down
19 changes: 19 additions & 0 deletions test-files/test_gap_url_shape_probe_gate.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
// The generic value paths (`ToString`, `+` coercion, `JSON.stringify`,
// `Object.fromEntries`, and the by-name field setter) each carry a runtime
// "is this object URL-shaped?" probe. Those probes are compiled only under
// `url-engine`, because their static references otherwise pin the whole URL
// parser into every binary. This file uses the URL API, so `uses_url` turns
// the feature on and every probe below must behave exactly as before.

const u = new URL("https://user:pw@example.com:8443/a/b?x=1&y=2#frag");
console.log(String(u));
console.log("" + u);
console.log(u.hostname, u.port, u.pathname, u.search, u.hash);
u.pathname = "/changed";
u.search = "?z=9";
u.href = "https://other.example/zzz?q=1";
console.log(u.href, u.hostname);
const sp = new URLSearchParams("a=1&b=2");
console.log(String(sp), "" + sp);
console.log(JSON.stringify({ u, sp: String(sp) }));
console.log(JSON.stringify(Object.fromEntries(sp)));
Comment on lines +18 to +19

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== repo files of interest =="
git ls-files | rg '(^\.node-version$|^test-files/test_gap_url_shape_probe_gate\.ts$|^crates/perry-runtime/src/json/replacer\.rs$)' || true

echo
echo "== .node-version =="
if [ -f .node-version ]; then cat -n .node-version; fi

echo
echo "== test file =="
if [ -f test-files/test_gap_url_shape_probe_gate.ts ]; then cat -n test-files/test_gap_url_shape_probe_gate.ts; fi

echo
echo "== replacer relevant snippets =="
if [ -f crates/perry-runtime/src/json/replacer.rs ]; then
  wc -l crates/perry-runtime/src/json/replacer.rs
  sed -n '240,300p' crates/perry-runtime/src/json/replacer.rs | cat -n
  echo "---"
  sed -n '800,845p' crates/perry-runtime/src/json/replacer.rs | cat -n
fi

Repository: PerryTS/perry

Length of output: 7774


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== JSON.stringify implementation references =="
rg -n "stringify|JSON\.stringify|fn stringify|stringify_pretty|fn stringify_object\(|stringify_object_inner|write_url_href_json" crates/perry-runtime/src test-files -T ts -T rs

echo
echo "== stringify.rs relevant sections =="
fd '^stringify\.rs$' crates/perry-runtime/src && fd '^stringify\.rs$' crates/perry-runtime/src -x sh -c 'echo "--- $1"; wc -l "$1"; sed -n "1,220p" "$1" | cat -n' sh {}

Repository: PerryTS/perry

Length of output: 229


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== JSON.stringify implementation references =="
rg -n "stringify|JSON\.stringify|fn stringify|stringify_pretty|fn stringify_object\(|stringify_object_inner|write_url_href_json" crates/perry-runtime/src test-files -g '*.ts' -g '*.rs'

echo
echo "== stringify.rs relevant sections =="
fd '^stringify\.rs$' crates/perry-runtime/src -x sh -c 'echo "--- $1"; wc -l "$1"; sed -n "1,220p" "$1" | cat -n' sh {}

Repository: PerryTS/perry

Length of output: 50370


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Node availability =="
if command -v node >/dev/null 2>&1; then
  node -e "console.log(process.version)"
else
  echo "node not available"
fi

echo
echo "== JSON.stringify branches from Node 26.5.1 if available =="
if command -v node >/dev/null 2>&1; then
  node - <<'JS'
const cases = {
  compact_replacer_url: JSON.stringify({ value: new URL("https://host/path?a=1") }, (k, v) => v),
  pretty_url: JSON.stringify({ value: new URL("https://host/path?a=1") }, null, 2),
  compact_default_url: JSON.stringify({ value: new URL("https://host/path?a=1") }),
};
for (const [name, value] of Object.entries(cases)) {
  console.log(name, value);
}
JS
fi

Repository: PerryTS/perry

Length of output: 404


Cover the compact replacer and pretty JSON.stringify paths.

The current test uses no replacer and no space argument, so it does not hit the changed compact replacer implementation path or the pretty-print implementation path.

Add one compact JSON.stringify call with a replacer and one pretty call.

Proposed test additions
 console.log(JSON.stringify({ u, sp: String(sp) }));
+const identity_replacer = (_key: string, value: unknown) => value;
+console.log(JSON.stringify({ u }, identity_replacer));
+console.log(JSON.stringify({ u }, null, 2));

Run this parity/gap check against the Node version in .node-version.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
console.log(JSON.stringify({ u, sp: String(sp) }));
console.log(JSON.stringify(Object.fromEntries(sp)));
console.log(JSON.stringify({ u, sp: String(sp) }));
const identity_replacer = (_key: string, value: unknown) => value;
console.log(JSON.stringify({ u }, identity_replacer));
console.log(JSON.stringify({ u }, null, 2));
console.log(JSON.stringify(Object.fromEntries(sp)));
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test-files/test_gap_url_shape_probe_gate.ts` around lines 18 - 19, Update the
test around the existing JSON.stringify probes to add one compact call that
supplies a replacer and one pretty-print call with a space argument, covering
both changed implementation paths while preserving the current outputs. Run the
parity/gap check using the Node version specified by .node-version.

Source: Coding guidelines

Loading