From dc55d1f4b37289f2baba8e2151cf75a14c2e3921 Mon Sep 17 00:00:00 2001 From: Lars Haukland Date: Tue, 18 Nov 2025 11:45:09 +0100 Subject: [PATCH 1/3] refactor(map): flatten map data --- .../map_objects/map_constructor.rs | 9 +-- .../map_objects/map_prototype.rs | 16 ++--- nova_vm/src/ecmascript/builtins/map/data.rs | 68 ++++++++----------- 3 files changed, 38 insertions(+), 55 deletions(-) diff --git a/nova_vm/src/ecmascript/builtins/keyed_collections/map_objects/map_constructor.rs b/nova_vm/src/ecmascript/builtins/keyed_collections/map_objects/map_constructor.rs index 138106007..a2c3b90f7 100644 --- a/nova_vm/src/ecmascript/builtins/keyed_collections/map_objects/map_constructor.rs +++ b/nova_vm/src/ecmascript/builtins/keyed_collections/map_objects/map_constructor.rs @@ -27,10 +27,7 @@ use crate::{ keyed_collections::map_objects::map_prototype::{ MapPrototypeSet, canonicalize_keyed_collection_key, }, - map::{ - Map, - data::{MapData, MapHeapData}, - }, + map::{Map, data::MapHeapData}, ordinary::ordinary_create_from_constructor, }, execution::{ @@ -221,7 +218,7 @@ impl MapConstructor { let primitive_heap = PrimitiveHeap::new(bigints, numbers, strings); let map_entry = &mut maps[map]; - let MapData { + let MapHeapData { keys, values, map_data, @@ -364,7 +361,7 @@ pub fn add_entries_from_iterable_map_constructor<'a>( // Trivial, dense array of trivial, dense arrays of two elements. let gc = gc.nogc(); let length = arr_elements.len(); - let MapData { + let MapHeapData { keys, values, map_data, diff --git a/nova_vm/src/ecmascript/builtins/keyed_collections/map_objects/map_prototype.rs b/nova_vm/src/ecmascript/builtins/keyed_collections/map_objects/map_prototype.rs index c8b9917f9..92c72b402 100644 --- a/nova_vm/src/ecmascript/builtins/keyed_collections/map_objects/map_prototype.rs +++ b/nova_vm/src/ecmascript/builtins/keyed_collections/map_objects/map_prototype.rs @@ -17,7 +17,7 @@ use crate::{ ArgumentsList, Behaviour, Builtin, BuiltinGetter, BuiltinIntrinsic, indexed_collections::array_objects::array_iterator_objects::array_iterator::CollectionIteratorKind, keyed_collections::map_objects::map_iterator_objects::map_iterator::MapIterator, - map::{Map, data::MapData}, + map::{Map, data::MapHeapData}, }, execution::{Agent, JsResult, Realm, agent::ExceptionType}, types::{BUILTIN_STRING_MEMORY, HeapNumber, IntoValue, PropertyKey, String, Value}, @@ -157,12 +157,12 @@ impl MapPrototype { hasher.finish() }; // 4. For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do - let MapData { - keys, + let MapHeapData { values, + keys, map_data, .. - } = maps[m].borrow_mut(&primitive_heap); + } = &mut maps[m].borrow_mut(&primitive_heap); let map_data = map_data.get_mut(); // a. If p.[[Key]] is not EMPTY and SameValue(p.[[Key]], key) is true, then @@ -326,9 +326,9 @@ impl MapPrototype { hasher.finish() }; // 4. For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do - let MapData { - keys, + let MapHeapData { values, + keys, map_data, .. } = &maps[m].borrow(&primitive_heap); @@ -378,7 +378,7 @@ impl MapPrototype { hasher.finish() }; // 4. For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do - let MapData { keys, map_data, .. } = &mut maps[m].borrow_mut(&primitive_heap); + let MapHeapData { keys, map_data, .. } = &mut maps[m].borrow_mut(&primitive_heap); let map_data = map_data.get_mut(); // a. If p.[[Key]] is not EMPTY and SameValue(p.[[Key]], key) is true, return true. @@ -433,7 +433,7 @@ impl MapPrototype { } = &mut agent.heap; let primitive_heap = PrimitiveHeap::new(bigints, numbers, strings); - let MapData { + let MapHeapData { keys, values, map_data, diff --git a/nova_vm/src/ecmascript/builtins/map/data.rs b/nova_vm/src/ecmascript/builtins/map/data.rs index 240dbbee5..a15806daf 100644 --- a/nova_vm/src/ecmascript/builtins/map/data.rs +++ b/nova_vm/src/ecmascript/builtins/map/data.rs @@ -20,21 +20,13 @@ use hashbrown::{HashTable, hash_table::Entry}; #[derive(Debug, Default)] pub struct MapHeapData<'a> { - pub(crate) object_index: Option>, - map_data: MapData<'a>, - // TODO: When an non-terminal (start or end) iterator exists for the Map, - // the items in the map cannot be compacted. - // pub(crate) observed: bool; -} - -#[derive(Debug, Default)] -pub(crate) struct MapData<'a> { // TODO: Use a ParallelVec to remove one unnecessary allocation. // pub(crate) key_values: ParallelVec, Option> pub(crate) keys: Vec>>, pub(crate) values: Vec>>, /// Low-level hash table pointing to keys-values indexes. pub(crate) map_data: RefCell>, + pub(crate) object_index: Option>, /// Flag that lets the Map know if it needs to rehash its primitive keys. /// /// This happens when an object key needs to be moved in the map_data @@ -43,6 +35,9 @@ pub(crate) struct MapData<'a> { /// garbage collection due to the heap data being concurrently sweeped on /// another thread. pub(crate) needs_primitive_rehashing: AtomicBool, + // TODO: When an non-terminal (start or end) iterator exists for the Map, + // the items in the map cannot be compacted. + // pub(crate) observed: bool; } impl<'a> MapHeapData<'a> { @@ -56,50 +51,48 @@ impl<'a> MapHeapData<'a> { // 2. For each element e of setData, do // a. If e is not EMPTY, set count to count + 1. // 3. Return count. - self.map_data.map_data.borrow().len() as u32 + self.map_data.borrow().len() as u32 } pub fn keys(&self, _gc: NoGcScope<'a, '_>) -> &[Option>] { - &self.map_data.keys + &self.keys } pub fn values(&self, _gc: NoGcScope<'a, '_>) -> &[Option>] { - &self.map_data.values + &self.values } pub fn clear(&mut self) { // 3. For each Record { [[Key]], [[Value]] } p of M.[[MapData]], do // a. Set p.[[Key]] to EMPTY. // b. Set p.[[Value]] to EMPTY. - self.map_data.map_data.get_mut().clear(); - self.map_data.values.fill(None); - self.map_data.keys.fill(None); + self.map_data.get_mut().clear(); + self.values.fill(None); + self.keys.fill(None); } - pub(crate) fn borrow(&self, arena: &impl PrimitiveHeapIndexable) -> &MapData<'a> { - self.map_data.rehash_if_needed(arena); - &self.map_data + pub(crate) fn borrow(&self, arena: &impl PrimitiveHeapIndexable) -> &Self { + self.rehash_if_needed(arena); + self } - pub(crate) fn borrow_mut(&mut self, arena: &impl PrimitiveHeapIndexable) -> &mut MapData<'a> { - self.map_data.rehash_if_needed_mut(arena); - &mut self.map_data + pub(crate) fn borrow_mut(&mut self, arena: &impl PrimitiveHeapIndexable) -> &mut Self { + self.rehash_if_needed_mut(arena); + self } pub fn with_capacity(new_len: usize) -> Self { Self { - map_data: MapData { - keys: Vec::with_capacity(new_len), - values: Vec::with_capacity(new_len), - map_data: RefCell::new(HashTable::with_capacity(new_len)), - needs_primitive_rehashing: AtomicBool::new(false), - }, + keys: Vec::with_capacity(new_len), + values: Vec::with_capacity(new_len), + map_data: RefCell::new(HashTable::with_capacity(new_len)), + needs_primitive_rehashing: AtomicBool::new(false), object_index: None, } } } -impl MapData<'_> { +impl MapHeapData<'_> { fn rehash_if_needed_mut(&mut self, arena: &impl PrimitiveHeapIndexable) { if !*self.needs_primitive_rehashing.get_mut() { return; @@ -185,31 +178,24 @@ bindable_handle!(MapHeapData); impl HeapMarkAndSweep for MapHeapData<'static> { fn mark_values(&self, queues: &mut WorkQueues) { let Self { + keys, + values, object_index, - map_data, + .. } = self; object_index.mark_values(queues); - map_data - .keys - .iter() - .for_each(|value| value.mark_values(queues)); - map_data - .values - .iter() - .for_each(|value| value.mark_values(queues)); + keys.iter().for_each(|value| value.mark_values(queues)); + values.iter().for_each(|value| value.mark_values(queues)); } fn sweep_values(&mut self, compactions: &CompactionLists) { let Self { object_index, - map_data, - } = self; - let MapData { keys, values, needs_primitive_rehashing, map_data, - } = map_data; + } = self; let map_data = map_data.get_mut(); object_index.sweep_values(compactions); From c8429aa360171be0769ea6fde4023aafb7f62f1d Mon Sep 17 00:00:00 2001 From: Lars Haukland Date: Tue, 18 Nov 2025 11:47:43 +0100 Subject: [PATCH 2/3] feat(map): derive soable on MapHeapData --- nova_vm/src/ecmascript/builtins/map/data.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/nova_vm/src/ecmascript/builtins/map/data.rs b/nova_vm/src/ecmascript/builtins/map/data.rs index a15806daf..21ddf0873 100644 --- a/nova_vm/src/ecmascript/builtins/map/data.rs +++ b/nova_vm/src/ecmascript/builtins/map/data.rs @@ -17,8 +17,9 @@ use core::{ sync::atomic::{AtomicBool, Ordering}, }; use hashbrown::{HashTable, hash_table::Entry}; +use soavec_derive::SoAble; -#[derive(Debug, Default)] +#[derive(Debug, Default, SoAble)] pub struct MapHeapData<'a> { // TODO: Use a ParallelVec to remove one unnecessary allocation. // pub(crate) key_values: ParallelVec, Option> From 4f52e855c89e43b0a1c2ef4408cc1e14a7ab4f15 Mon Sep 17 00:00:00 2001 From: Lars Haukland Date: Tue, 18 Nov 2025 11:55:13 +0100 Subject: [PATCH 3/3] refactor(map): reorder fields in MapHeapData --- nova_vm/src/ecmascript/builtins/map/data.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/nova_vm/src/ecmascript/builtins/map/data.rs b/nova_vm/src/ecmascript/builtins/map/data.rs index 21ddf0873..470183e70 100644 --- a/nova_vm/src/ecmascript/builtins/map/data.rs +++ b/nova_vm/src/ecmascript/builtins/map/data.rs @@ -21,12 +21,12 @@ use soavec_derive::SoAble; #[derive(Debug, Default, SoAble)] pub struct MapHeapData<'a> { + /// Low-level hash table pointing to keys-values indexes. + pub(crate) map_data: RefCell>, // TODO: Use a ParallelVec to remove one unnecessary allocation. // pub(crate) key_values: ParallelVec, Option> - pub(crate) keys: Vec>>, pub(crate) values: Vec>>, - /// Low-level hash table pointing to keys-values indexes. - pub(crate) map_data: RefCell>, + pub(crate) keys: Vec>>, pub(crate) object_index: Option>, /// Flag that lets the Map know if it needs to rehash its primitive keys. ///