Intern composite per-member cache keys

Author: AJenbo

docs/todo/eager-resolution.md

@@ -367,27 +367,21 @@ checks, `base_name` extraction) were unaffected; only genuine
 `String`-typed sinks (`Vec<String>` keys, `Generic`'s `String` field)
 needed an explicit `.to_string()`.
 
-#### Phase 4h: Intern composite per-member cache keys
-
-**Impact: Low. Effort: Low.**
-
-`target_cache.rs` (`BODY_INFER_MEMO`, the re-entry set) and
-`property_access.rs` key thread-local caches on
-`format!("{}::{}", class_fqn, method)`. These allocate a `String` per
-lookup. Interning them into a single `Atom` key would remove the
-allocation, but the key space is the `FQN × member` cartesian product,
-which is effectively unbounded — feeding it to the global `ustr`
-interner would leak one entry per distinct pair for the process
-lifetime (the exact "constructed once, never compared" case
-`atom.rs` warns against). Any fix here should therefore use a
-composite `(Atom, Atom)` tuple key (both halves already interned,
-no new interner entries) rather than interning the joined string.
-
-**Success criteria:**
-- The `"FQN::member"` cache keys no longer allocate a `String` per
-  lookup.
-- No net growth in the global interner.
-- No test regressions.
+#### Phase 4h: Intern composite per-member cache keys ✓
+
+`target_cache.rs` (`BODY_INFER_MEMO` and its `BODY_INFER_VISITED`
+re-entry set) and `property_access.rs` (`RESOLVING_THIS_PROP`) keyed
+thread-local caches on `format!("{}::{}", class_fqn, member)`, allocating
+a `String` per lookup. These now use a composite `(Atom, Atom)` tuple
+key. Both halves are drawn from bounded symbol-name spaces (class FQNs
+and member identifiers) and are already interned, so the tuple key
+allocates nothing per lookup and adds no new entries to the global
+interner. Interning the *joined* string instead would have leaked one
+entry per distinct `FQN × member` pair for the process lifetime (the
+"constructed once, never compared" case `atom.rs` warns against), so the
+tuple is the correct shape. `CALLABLE_TARGET_CACHE` is intentionally left
+on a string key: its key embeds generic argument text
+(`"FQN<generics>::method"`) and does not fit a two-`Atom` tuple.
 
 ---
 

src/type_engine/call_resolution/target_cache.rs

@@ -7,6 +7,7 @@ use std::cell::{Cell, RefCell};
 use std::collections::{HashMap, HashSet};
 
 use crate::Backend;
+use crate::atom::{Atom, atom};
 use crate::php_type::PhpType;
 use crate::types::*;
 
@@ -26,6 +27,9 @@ type BodyReturnInferrerFn = Box<dyn Fn(&str, &MethodInfo) -> Option<PhpType>>;
 /// down.
 type AuthUserResolverFn = Box<dyn Fn(Option<&str>) -> Option<PhpType>>;
 
+/// Memoized body-return-inference results, keyed by `(FQN, method)`.
+type BodyInferMemo = HashMap<(Atom, Atom), Option<PhpType>>;
+
 thread_local! {
     /// When `Some`, `resolve_instance_method_callable` caches results
     /// by `"FQN::method_lower"`.  Activated by
@@ -44,14 +48,14 @@ thread_local! {
         const { RefCell::new(None) };
 
     /// Re-entry guard for body return inference.  Tracks
-    /// `"FQN::method"` keys currently being inferred to prevent
+    /// `(FQN, method)` keys currently being inferred to prevent
     /// infinite recursion when a method body references another
     /// method that also lacks a return type.
-    static BODY_INFER_VISITED: RefCell<HashSet<String>> =
+    static BODY_INFER_VISITED: RefCell<HashSet<(Atom, Atom)>> =
         RefCell::new(HashSet::new());
 
     /// When `Some`, memoizes completed body return inference results by
-    /// `"FQN::method"`.  Activated together with [`BODY_RETURN_INFERRER`]
+    /// `(FQN, method)`.  Activated together with [`BODY_RETURN_INFERRER`]
     /// and cleared when the owning guard drops, so the memo lives exactly
     /// as long as one request / one file's diagnostic pass.
     ///
@@ -60,7 +64,7 @@ thread_local! {
     /// files where most methods lack declared return types, the repeated
     /// walks compound into a multi-minute blowup (each walk itself
     /// triggers inference for the callees it contains).
-    static BODY_INFER_MEMO: RefCell<Option<HashMap<String, Option<PhpType>>>> =
+    static BODY_INFER_MEMO: RefCell<Option<BodyInferMemo>> =
         const { RefCell::new(None) };
 
     /// Current nesting depth of body return inference.  Caps the
@@ -176,8 +180,12 @@ pub(crate) fn with_body_return_inferrer(inferrer: BodyReturnInferrerFn) -> BodyR
 const MAX_BODY_INFER_DEPTH: u8 = 3;
 
 pub(crate) fn try_infer_body_return_type(class_fqn: &str, method: &MethodInfo) -> Option<PhpType> {
-    // Build the memo / re-entry key.
-    let key = format!("{}::{}", class_fqn, method.name);
+    // Build the memo / re-entry key as an interned `(FQN, method)`
+    // tuple.  Both halves come from bounded symbol-name spaces and are
+    // already interned, so the key allocates nothing and adds no new
+    // entries to the global interner (a joined `"FQN::method"` string
+    // would leak one entry per distinct pair for the process lifetime).
+    let key = (atom(class_fqn), method.name);
 
     // Serve a memoized result from an earlier completed inference in
     // this request.  Checked before the depth cap so that deep call
@@ -197,7 +205,7 @@ pub(crate) fn try_infer_body_return_type(class_fqn: &str, method: &MethodInfo) -
     // Check + insert into the visited set (re-entry guard).
     let already_visiting = BODY_INFER_VISITED.with(|cell| {
         let mut set = cell.borrow_mut();
-        !set.insert(key.clone())
+        !set.insert(key)
     });
     if already_visiting {
         return None;

src/type_engine/variable/rhs_resolution/property_access.rs

@@ -9,7 +9,7 @@ use std::sync::Arc;
 use mago_span::HasSpan;
 use mago_syntax::cst::*;
 
-use crate::atom::{atom, bytes_to_str};
+use crate::atom::{Atom, atom, bytes_to_str};
 use crate::parser::with_parsed_program;
 use crate::php_type::PhpType;
 use crate::types::{ClassInfo, ResolvedType};
@@ -433,11 +433,16 @@ pub(super) fn try_resolve_this_property_from_assignment(
     // resolved, return empty so the caller falls back to the declared
     // property type instead of recursing.
     thread_local! {
-        static RESOLVING_THIS_PROP: std::cell::RefCell<HashSet<String>> =
+        static RESOLVING_THIS_PROP: std::cell::RefCell<HashSet<(Atom, Atom)>> =
             std::cell::RefCell::new(HashSet::new());
     }
-    let key = format!("{}::{}", ctx.current_class.name, prop_name);
-    let newly_inserted = RESOLVING_THIS_PROP.with(|set| set.borrow_mut().insert(key.clone()));
+    // Key on `(class, prop)` as interned atoms rather than a joined
+    // `"Class::prop"` string: both halves are already interned, so the
+    // tuple key allocates nothing per lookup and adds no new entries to
+    // the global interner (the joined string would leak one entry per
+    // distinct pair for the process lifetime).
+    let key = (ctx.current_class.name, atom(prop_name));
+    let newly_inserted = RESOLVING_THIS_PROP.with(|set| set.borrow_mut().insert(key));
     if !newly_inserted {
         // Same property is already mid-resolution: break the cycle.
         return Vec::new();

src/virtual_members/laravel/config_values.rs

@@ -26,7 +26,7 @@ use mago_database::file::FileId;
 use mago_syntax::cst::*;
 
 use crate::Backend;
-use crate::atom::bytes_to_str;
+use crate::atom::{atom, bytes_to_str};
 use crate::php_type::{PhpType, ShapeEntry};
 
 /// A statically-classified Laravel config value.
@@ -142,7 +142,7 @@ impl ConfigValue {
             ConfigValue::Bool => PhpType::bool(),
             ConfigValue::Null => PhpType::null(),
             ConfigValue::ClassString(name) => {
-                PhpType::ClassString(Some(Box::new(PhpType::Named(name.clone()))))
+                PhpType::ClassString(Some(Box::new(PhpType::Named(atom(name)))))
             }
             ConfigValue::OneOf(arms) => {
                 let mut members: Vec<PhpType> = Vec::new();