JavaScript: a collection counts the code units of property and binding names

A collection counted each property or binding at a fixed 64 bytes and
never counted the name's own buffer. Once the string that produced a long
name became garbage, the name kept its buffer alive while the estimate
dropped it, so a script could hold megabytes of names past the heap
limit. trace_object and trace_env now add each name's buffer, counting a
buffer that a flat string value or another name shares once.

crates/js/src/heap.rs

@@ -280,7 +280,8 @@ enum Cell {
     Env(Box<Env>),
 }
 
-/// The estimated bytes of a property or a binding, its name included.
+/// The estimated bytes of a property or a binding entry. A collection adds its name's code units
+/// separately, counting a buffer that several names or strings share once.
 pub(crate) const PROP_BYTES: usize = 64;
 const OBJECT_BYTES: usize = 96;
 const ENV_BYTES: usize = 64;
@@ -406,7 +407,8 @@ pub(crate) struct Roots<'a> {
     pub(crate) codes: Vec<&'a Code>,
 }
 
-/// Sums the sizes of the strings reached, counting each node once.
+/// Sums the sizes of the strings and property names reached, counting each node and each
+/// code-unit buffer once.
 #[derive(Default)]
 struct StringSizes {
     seen: HashSet<usize>,
@@ -426,10 +428,21 @@ impl StringSizes {
                     stack.push(l);
                     stack.push(r);
                 }
-                None => self.bytes += string_bytes(s.len()),
+                None => {
+                    if s.buffer_addr().is_none_or(|a| self.seen.insert(a)) {
+                        self.bytes += string_bytes(s.len());
+                    }
+                }
             }
         }
     }
+
+    /// Adds a property or binding name, unless a string or another name shares its buffer.
+    fn add_key(&mut self, k: &Key) {
+        if self.seen.insert(Rc::as_ptr(k) as *const () as usize) {
+            self.bytes += string_bytes(k.len());
+        }
+    }
 }
 
 fn mark_value(v: &Value, work: &mut Vec<u32>, strings: &mut StringSizes) {
@@ -450,7 +463,8 @@ fn trace_object(o: &Object, work: &mut Vec<u32>, strings: &mut StringSizes) -> u
     if let Some(p) = o.proto {
         work.push(p.0);
     }
-    for (_, prop) in o.props.iter() {
+    for (k, prop) in o.props.iter() {
+        strings.add_key(k);
         match prop {
             Prop::Data { value, .. } => mark_value(value, work, strings),
             Prop::Accessor { get, set, .. } => work.extend(get.iter().chain(set.iter()).map(|f| f.0)),
@@ -476,7 +490,8 @@ fn trace_env(e: &Env, work: &mut Vec<u32>, strings: &mut StringSizes) -> usize {
     match e {
         Env::Decl { bindings, outer } => {
             work.extend(outer.iter().map(|o| o.0));
-            for (_, b) in bindings.iter() {
+            for (k, b) in bindings.iter() {
+                strings.add_key(k);
                 mark_value(&b.value, work, strings);
             }
             ENV_BYTES + PROP_BYTES * bindings.len()
@@ -491,7 +506,20 @@ fn trace_env(e: &Env, work: &mut Vec<u32>, strings: &mut StringSizes) -> usize {
 
 #[cfg(test)]
 mod tests {
-    use super::{PropMap, key};
+    use super::{Class, Heap, OBJECT_BYTES, Object, PROP_BYTES, Prop, PropMap, Roots, key, string_bytes};
+    use crate::value::{Str, Value};
+
+    #[test]
+    fn a_collection_counts_a_name_and_a_value_that_share_a_buffer_once() {
+        let mut heap = Heap::default();
+        let name = key(&"n".repeat(1000));
+        let mut object = Object::new(Class::Ordinary, None);
+        object.props.insert(name.clone(), Prop::Data { value: Value::String(Str::from_rc(name.clone())), flags: 0 });
+        object.props.insert(key("other"), Prop::Data { value: Value::Undefined, flags: 0 });
+        let id = heap.alloc_object(object);
+        heap.collect(&Roots { objects: vec![id], ..Roots::default() });
+        assert_eq!(heap.bytes, OBJECT_BYTES + 2 * PROP_BYTES + string_bytes(1000) + string_bytes(5));
+    }
 
     #[test]
     fn prop_maps_keep_insertion_order_through_removals_and_growth() {

crates/js/src/interp_tests.rs

@@ -437,6 +437,21 @@ fn for_in_over_a_long_string_checks_the_heap_before_listing_its_indices() {
     assert_eq!(realm.run("var n = ''; for (var k in 'abc') n += k; n"), Ok(JsValue::String("012".into())));
 }
 
+#[test]
+fn long_property_names_count_toward_the_heap_after_their_strings_are_collected() {
+    // Each name is a 16,385-unit buffer that only the object's key or the function's binding
+    // holds once the string that made it is garbage, so about 32 of them fill the heap well before
+    // the fuel runs out. In the second script the property lookup flattens a long string each
+    // time, so collections run.
+    let mut realm = small(Limits { heap_bytes: 1 << 20, fuel: 20_000, ..Limits::default() });
+    let src = "var s = 'x'; while (s.length < 10000) s += s; var o = {}; for (var i = 0; ; i++) o[s + i] = 1;";
+    assert_eq!(limit(&mut realm, src), LimitKind::Heap);
+    let src = "var s = 'x'; while (s.length < 10000) s += s; \
+        (function () { for (var i = 0; ; i++) { eval('var ' + s + i + ' = 1'); ({})[s + i]; } })();";
+    let mut realm = small(Limits { heap_bytes: 1 << 20, fuel: 20_000, ..Limits::default() });
+    assert_eq!(limit(&mut realm, src), LimitKind::Heap);
+}
+
 #[test]
 fn for_in_over_a_long_string_takes_fuel_for_each_index() {
     let mut realm = small(Limits { fuel: 10_000, ..Limits::default() });

crates/js/src/value.rs

@@ -106,6 +106,14 @@ impl Str {
         Rc::as_ptr(&self.0) as *const () as usize
     }
 
+    /// The address of the code-unit buffer of a flat node, which a property name can share.
+    pub(crate) fn buffer_addr(&self) -> Option<usize> {
+        match &*self.0.borrow() {
+            Node::Flat(f) => Some(Rc::as_ptr(f) as *const () as usize),
+            _ => None,
+        }
+    }
+
     /// The two halves of a concatenation node.
     pub(crate) fn halves(&self) -> Option<(Str, Str)> {
         match &*self.0.borrow() {