diff --git a/changelog.d/8830-declared-map-get-dispatch.md b/changelog.d/8830-declared-map-get-dispatch.md new file mode 100644 index 0000000000..17d29a947d --- /dev/null +++ b/changelog.d/8830-declared-map-get-dispatch.md @@ -0,0 +1,4 @@ +Speed up declared `Map.get` and `ReadonlyMap.get` calls that pass through +nested interface or object fields. Genuine native Maps now bypass generic +method dispatch, while structural values, subclasses, proxies, primitives, +and nullish receivers retain ordinary JavaScript behavior on a brand miss. diff --git a/crates/perry-codegen/src/expr/readonly_collection_tests.rs b/crates/perry-codegen/src/expr/readonly_collection_tests.rs index fcd8fff304..c18f6f3b80 100644 --- a/crates/perry-codegen/src/expr/readonly_collection_tests.rs +++ b/crates/perry-codegen/src/expr/readonly_collection_tests.rs @@ -1,6 +1,8 @@ use crate::{compile_module, CompileOptions, ImportedClass}; use perry_hir::types::Type; -use perry_hir::{Class, ClassField, Expr, Function, Module, Param, Stmt}; +use perry_hir::{ + Class, ClassField, Expr, Function, Interface, InterfaceProperty, Module, Param, Stmt, +}; fn number_param(id: u32, name: &str) -> Param { Param { @@ -223,6 +225,105 @@ fn compile_imported_has_ir() -> String { .expect("LLVM IR is UTF-8") } +fn compile_nested_map_get_ir() -> String { + let mut module = Module::new("command_executor.ts"); + module.interfaces.push(Interface { + id: 1, + name: "CommandExecutorContext".to_string(), + type_params: Vec::new(), + extends: Vec::new(), + properties: vec![InterfaceProperty { + name: "entityToArchetype".to_string(), + ty: Type::Generic { + base: "Map".to_string(), + type_args: vec![Type::Number, Type::Number], + }, + optional: false, + readonly: false, + }], + methods: Vec::new(), + is_exported: false, + }); + module.classes.push(Class { + id: 2, + name: "CommandExecutor".to_string(), + type_params: Vec::new(), + extends: None, + extends_name: None, + native_extends: None, + extends_expr: None, + heritage_lexically_shadowed: false, + fields: vec![ClassField { + name: "ctx".to_string(), + key_expr: None, + ty: Type::Named("CommandExecutorContext".to_string()), + init: None, + is_private: false, + is_readonly: true, + decorators: Vec::new(), + }], + constructor: None, + methods: vec![Function { + id: 3, + name: "lookup".to_string(), + type_params: Vec::new(), + params: vec![number_param(1, "entityId")], + return_type: Type::Number, + body: vec![Stmt::Return(Some(Expr::Call { + callee: Box::new(Expr::PropertyGet { + object: Box::new(Expr::PropertyGet { + object: Box::new(Expr::PropertyGet { + object: Box::new(Expr::This), + property: "ctx".to_string(), + byte_offset: 0, + }), + property: "entityToArchetype".to_string(), + byte_offset: 0, + }), + property: "get".to_string(), + byte_offset: 0, + }), + args: vec![Expr::LocalGet(1)], + type_args: Vec::new(), + byte_offset: 0, + }))], + is_async: false, + is_generator: false, + is_strict: true, + is_exported: false, + captures: Vec::new(), + decorators: Vec::new(), + was_plain_async: false, + was_unrolled: false, + }], + getters: Vec::new(), + setters: Vec::new(), + static_accessor_names: Vec::new(), + static_accessor_fn_ids: Vec::new(), + computed_members: Vec::new(), + static_fields: Vec::new(), + static_methods: Vec::new(), + decorators: Vec::new(), + is_exported: false, + aliases: Vec::new(), + is_nested: false, + alloc_width_hint: 0, + specialized_from: None, + }); + + String::from_utf8( + compile_module( + &module, + CompileOptions { + emit_ir_only: true, + ..Default::default() + }, + ) + .expect("nested declared Map.get compiles"), + ) + .expect("LLVM IR is UTF-8") +} + fn method_ir<'a>(ir: &'a str, owner: &str, method: &str) -> &'a str { let suffix = format!("__{owner}__{method}("); let suffix_start = ir.find(&suffix).expect("requested method is present"); @@ -280,3 +381,18 @@ fn imported_class_readonly_set_field_uses_branded_fast_path() { "cross-module field metadata must not force native Sets through generic dispatch:\n{method_ir}" ); } + +#[test] +fn nested_interface_map_field_get_uses_branded_dispatch() { + let ir = compile_nested_map_get_ir(); + let method_ir = method_ir(&ir, "CommandExecutor", "lookup"); + + assert!( + method_ir.contains("call double @js_declared_map_get("), + "a Map reached through a nested interface field must retain a branded dispatch candidate:\n{method_ir}" + ); + assert!( + !method_ir.contains("call double @js_native_call_method_by_id("), + "a genuine native Map must not enter generic method dispatch at this site:\n{method_ir}" + ); +} diff --git a/crates/perry-codegen/src/lower_call/property_get/map_set.rs b/crates/perry-codegen/src/lower_call/property_get/map_set.rs index 6d90dee044..68076e8e34 100644 --- a/crates/perry-codegen/src/lower_call/property_get/map_set.rs +++ b/crates/perry-codegen/src/lower_call/property_get/map_set.rs @@ -30,7 +30,7 @@ use crate::expr::{lower_expr, unbox_to_i64, FnCtx}; use crate::nanbox::double_literal; use crate::rooting; use crate::type_analysis::{ - is_map_expr, is_readonly_set_expr, is_set_expr, is_url_search_params_expr, + is_declared_map_expr, is_map_expr, is_readonly_set_expr, is_set_expr, is_url_search_params_expr, }; use crate::types::{DOUBLE, I64}; @@ -59,7 +59,23 @@ pub(crate) fn try_lower_map_set_methods( ))) }); } - if is_map_expr(ctx, object) { + let is_native_map = is_map_expr(ctx, object); + // A nested interface/object field can retain its `Map` or `ReadonlyMap` + // declaration after the stronger native-layout proof is lost. Avoid the + // full property/method dispatcher for a genuine native Map, but preserve + // structural and subclass behavior through the runtime brand miss. + if !is_native_map && is_declared_map_expr(ctx, object) && property == "get" && args.len() == 1 { + return rooting::with_operands_rooted(ctx, &[object, &args[0]], |ctx, vals| { + let receiver = vals[0].clone(); + let key = vals[1].clone(); + Ok(Some(ctx.block().call( + DOUBLE, + "js_declared_map_get", + &[(DOUBLE, &receiver), (DOUBLE, &key)], + ))) + }); + } + if is_native_map { match property { "set" if args.len() == 2 => { // #6970: each finished operand is live in an SSA register diff --git a/crates/perry-codegen/src/runtime_decls/strings.rs b/crates/perry-codegen/src/runtime_decls/strings.rs index 2a90e2db99..17aeb903e9 100644 --- a/crates/perry-codegen/src/runtime_decls/strings.rs +++ b/crates/perry-codegen/src/runtime_decls/strings.rs @@ -400,6 +400,7 @@ pub fn declare_phase_b_strings(module: &mut LlModule) { module.declare_function("js_map_set_string_string", I64, &[I64, I64, I64]); module.declare_function("js_map_set_number_key", I64, &[I64, DOUBLE, DOUBLE]); module.declare_function("js_map_get", DOUBLE, &[I64, DOUBLE]); + module.declare_function("js_declared_map_get", DOUBLE, &[DOUBLE, DOUBLE]); module.declare_function("js_map_get_string_key", DOUBLE, &[I64, I64]); module.declare_function("js_map_get_number_key", DOUBLE, &[I64, DOUBLE]); module.declare_function("js_map_has", I32, &[I64, DOUBLE]); diff --git a/crates/perry-codegen/src/type_analysis.rs b/crates/perry-codegen/src/type_analysis.rs index 2212a195c2..fc0e7a4c50 100644 --- a/crates/perry-codegen/src/type_analysis.rs +++ b/crates/perry-codegen/src/type_analysis.rs @@ -59,10 +59,10 @@ pub(crate) use refine::{ proven_type_from_init, refine_type_from_init, }; pub(crate) use strings::{ - class_name_extends_url_search_params, is_declared_string_expr, is_definitely_string_expr, - is_map_expr, is_readonly_set_expr, is_set_expr, is_string_expr, is_url_search_params_expr, - is_url_search_params_subclass_expr, map_static_type_args, set_static_type_args, - string_proof_is_declared_only, string_value_is_runtime_guaranteed, + class_name_extends_url_search_params, is_declared_map_expr, is_declared_string_expr, + is_definitely_string_expr, is_map_expr, is_readonly_set_expr, is_set_expr, is_string_expr, + is_url_search_params_expr, is_url_search_params_subclass_expr, map_static_type_args, + set_static_type_args, string_proof_is_declared_only, string_value_is_runtime_guaranteed, }; #[cfg(test)] diff --git a/crates/perry-codegen/src/type_analysis/strings.rs b/crates/perry-codegen/src/type_analysis/strings.rs index 13231d7db8..969d14472a 100644 --- a/crates/perry-codegen/src/type_analysis/strings.rs +++ b/crates/perry-codegen/src/type_analysis/strings.rs @@ -60,6 +60,33 @@ pub(crate) fn is_readonly_set_expr(ctx: &FnCtx<'_>, e: &Expr) -> bool { } } +/// True when a declared type says that the expression is a `Map` or +/// `ReadonlyMap`, but does not by itself prove Perry's native Map +/// layout. +/// +/// In particular this retains a useful candidate through nested structural +/// fields such as `this.ctx.entityToArchetype`. Callers must use a branded +/// runtime operation with ordinary method dispatch on a brand miss. +pub(crate) fn is_declared_map_expr(ctx: &FnCtx<'_>, e: &Expr) -> bool { + match e { + Expr::LocalGet(id) => ctx.local_type_hint(id).is_some_and(type_is_declared_map), + Expr::PropertyGet { + object, property, .. + } => static_type_of(ctx, object).is_some_and(|owner_ty| { + type_may_declare_collection_field(ctx, &owner_ty, property, type_is_declared_map, 0) + }), + _ => false, + } +} + +#[inline] +fn type_is_declared_map(ty: &HirType) -> bool { + matches!( + ty, + HirType::Generic { base, .. } if base == "Map" || base == "ReadonlyMap" + ) +} + #[inline] fn type_is_readonly_set(ty: &HirType) -> bool { matches!(ty, HirType::Generic { base, .. } if base == "ReadonlySet") @@ -76,6 +103,16 @@ fn type_may_declare_readonly_set_field( owner_ty: &HirType, property: &str, depth: usize, +) -> bool { + type_may_declare_collection_field(ctx, owner_ty, property, type_is_readonly_set, depth) +} + +fn type_may_declare_collection_field( + ctx: &FnCtx<'_>, + owner_ty: &HirType, + property: &str, + matches_collection: fn(&HirType) -> bool, + depth: usize, ) -> bool { if depth > 32 { return false; @@ -83,28 +120,41 @@ fn type_may_declare_readonly_set_field( match owner_ty { HirType::Union(variants) => variants.iter().any(|variant| { !matches!(variant, HirType::Null | HirType::Void | HirType::Never) - && type_may_declare_readonly_set_field(ctx, variant, property, depth + 1) + && type_may_declare_collection_field( + ctx, + variant, + property, + matches_collection, + depth + 1, + ) }), HirType::Named(name) | HirType::Generic { base: name, .. } => { if let Some(class) = ctx.classes.get(name) { if let Some(field) = class.fields.iter().find(|field| field.name == property) { - return type_is_readonly_set(&field.ty); + return matches_collection(&field.ty); } if let Some(parent) = class.extends_name.as_deref() { - return type_may_declare_readonly_set_field( + return type_may_declare_collection_field( ctx, &HirType::Named(parent.to_string()), property, + matches_collection, depth + 1, ); } } if let Some(iface) = ctx.interfaces.get(name) { if let Some(field) = iface.properties.iter().find(|field| field.name == property) { - return type_is_readonly_set(&field.ty); + return matches_collection(&field.ty); } if iface.extends.iter().any(|parent| { - type_may_declare_readonly_set_field(ctx, parent, property, depth + 1) + type_may_declare_collection_field( + ctx, + parent, + property, + matches_collection, + depth + 1, + ) }) { return true; } @@ -115,13 +165,13 @@ fn type_may_declare_readonly_set_field( if object .properties .get(property) - .is_some_and(|field| type_is_readonly_set(&field.ty)) + .is_some_and(|field| matches_collection(&field.ty)) ) } HirType::Object(object) => object .properties .get(property) - .is_some_and(|field| type_is_readonly_set(&field.ty)), + .is_some_and(|field| matches_collection(&field.ty)), _ => false, } } diff --git a/crates/perry-runtime/src/map.rs b/crates/perry-runtime/src/map.rs index 84352f507b..d94075f83b 100644 --- a/crates/perry-runtime/src/map.rs +++ b/crates/perry-runtime/src/map.rs @@ -1637,6 +1637,11 @@ pub extern "C" fn js_map_get(map: *const MapHeader, key: f64) -> f64 { if map.is_null() { return f64::from_bits(TAG_UNDEFINED); } + map_get_resolved(map, key) +} + +#[inline(always)] +fn map_get_resolved(map: *const MapHeader, key: f64) -> f64 { let key = normalize_zero(key); unsafe { let idx = find_key_index(map, key); @@ -1650,6 +1655,40 @@ pub extern "C" fn js_map_get(map: *const MapHeader, key: f64) -> f64 { } } +/// Fast `Map.get`/`ReadonlyMap.get` for a declared structural receiver. +/// +/// A TypeScript collection annotation does not prove Perry's native layout. +/// Genuine `GC_TYPE_MAP` receivers bypass generic property/method dispatch; +/// structural objects, proxies, subclasses, primitives, and nullish values +/// retain ordinary `receiver.get(key)` behavior on a brand miss. +#[no_mangle] +pub unsafe extern "C-unwind" fn js_declared_map_get(receiver: f64, key: f64) -> f64 { + let receiver_value = crate::value::JSValue::from_bits(receiver.to_bits()); + if receiver_value.is_pointer() { + let raw = receiver_value.as_pointer::(); + if matches!( + crate::value::addr_class::try_read_gc_header(raw as usize), + Some(header) if header.obj_type == crate::gc::GC_TYPE_MAP + ) { + return map_get_resolved(raw, key); + } + } + + // Generic dispatch can allocate and re-enter generated code. Keep both + // operands rooted and refresh them before crossing that boundary. + let scope = crate::gc::RuntimeHandleScope::new(); + let receiver_handle = scope.root_nanbox_f64(receiver); + let key_handle = scope.root_nanbox_f64(key); + let refreshed_key = key_handle.get_nanbox_f64(); + crate::object::js_native_call_method( + receiver_handle.get_nanbox_f64(), + b"get".as_ptr() as *const i8, + 3, + &refreshed_key, + 1, + ) +} + #[no_mangle] pub extern "C" fn js_map_get_number_key(map: *const MapHeader, key: f64) -> f64 { let Some(key) = normalize_number_key_from_boxed(key) else { diff --git a/crates/perry/tests/declared_map_branded_dispatch.rs b/crates/perry/tests/declared_map_branded_dispatch.rs new file mode 100644 index 0000000000..c8bccb07c8 --- /dev/null +++ b/crates/perry/tests/declared_map_branded_dispatch.rs @@ -0,0 +1,101 @@ +//! Executable semantics for guarded `Map.get` / `ReadonlyMap.get` dispatch. +//! Native Maps bypass generic method lookup while structural values and Map +//! subclasses retain ordinary JavaScript behavior. + +use std::path::PathBuf; +use std::process::Command; + +fn perry_bin() -> PathBuf { + PathBuf::from(env!("CARGO_BIN_EXE_perry")) +} + +fn compile_and_run(source: &str) -> String { + let dir = tempfile::tempdir().expect("tempdir"); + let entry = dir.path().join("main.ts"); + let output = dir.path().join("main_bin"); + std::fs::write(&entry, source).expect("write entry"); + + let compile = Command::new(perry_bin()) + .current_dir(dir.path()) + .arg("compile") + .arg(&entry) + .arg("-o") + .arg(&output) + .arg("--no-cache") + .output() + .expect("run perry compile"); + assert!( + compile.status.success(), + "perry compile failed\nstdout:\n{}\nstderr:\n{}", + String::from_utf8_lossy(&compile.stdout), + String::from_utf8_lossy(&compile.stderr) + ); + + let run = Command::new(&output) + .current_dir(dir.path()) + .output() + .expect("run compiled binary"); + assert!( + run.status.success(), + "compiled binary failed\nstatus: {:?}\nstdout:\n{}\nstderr:\n{}", + run.status, + String::from_utf8_lossy(&run.stdout), + String::from_utf8_lossy(&run.stderr) + ); + String::from_utf8_lossy(&run.stdout).into_owned() +} + +#[test] +fn native_structural_subclass_and_nullish_receivers_keep_get_semantics() { + let stdout = compile_and_run( + r#" +interface Context { + values: ReadonlyMap; +} + +class Holder { + constructor(public readonly ctx: Context) {} + lookup(key: number): string | undefined { + return this.ctx.values.get(key); + } +} + +const native = new Holder({ values: new Map([[2, "two"]]) }); +console.log("native", native.lookup(2), native.lookup(3)); + +let customCalls = 0; +const structural = { + get(key: number) { + customCalls++; + return key === 7 ? "seven" : undefined; + }, +} as unknown as ReadonlyMap; +const custom = new Holder({ values: structural }); +console.log("structural", custom.lookup(7), custom.lookup(8), customCalls); + +class OddMap extends Map { + override get(key: number): string | undefined { + return key === 99 ? "override" : undefined; + } +} +const subclass = new Holder({ values: new OddMap([[1, "one"]]) }); +console.log("subclass", subclass.lookup(99), subclass.lookup(1)); + +let nullishRejected = false; +try { + new Holder({ values: undefined as unknown as ReadonlyMap }).lookup(1); +} catch (_error) { + nullishRejected = true; +} +console.log("nullish", nullishRejected); +"#, + ); + + assert_eq!( + stdout, + "native two undefined\n\ + structural seven undefined 2\n\ + subclass override undefined\n\ + nullish true\n" + ); +}