Skip to content

Commit e03e7b7

Browse files
refactor(rust): simplify name_to_index handling in GraphSession
- Removed RefCell from name_to_index, allowing direct access to the HashMap for improved performance and clarity. - Updated methods to reflect the change, ensuring name_to_index is built and accessed without unnecessary borrowing. - Replaced HashSet with FxHashSet for edge pair tracking to enhance performance.
1 parent 351cf8c commit e03e7b7

1 file changed

Lines changed: 19 additions & 35 deletions

File tree

src/rust/src/graph/session.rs

Lines changed: 19 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ use super::CaugiGraph;
1717
use super::RegistrySnapshot;
1818
use crate::edges::{EdgeRegistry, EdgeSpec};
1919
use crate::graph::NeighborMode;
20-
use std::cell::RefCell;
21-
use std::collections::{HashMap, HashSet};
20+
use rustc_hash::FxHashSet;
21+
use std::collections::HashMap;
2222
use std::sync::Arc;
2323

2424
/// The target graph class for typed view construction.
@@ -141,8 +141,7 @@ pub struct GraphSession {
141141
edges: EdgeBuffer,
142142
names: Vec<String>,
143143
/// Maps node names to their 0-based indices for fast lookup.
144-
/// Built lazily for sessions constructed from known index inputs.
145-
name_to_index: RefCell<Option<HashMap<String, u32>>>,
144+
name_to_index: HashMap<String, u32>,
146145

147146
// ═══════════════════════════════════════════════════════════════════════════
148147
// VALIDITY FLAGS
@@ -175,7 +174,7 @@ impl GraphSession {
175174
let snapshot = Arc::new(RegistrySnapshot::from_specs(specs, registry.len() as u32));
176175

177176
let names: Vec<String> = (0..n).map(|i| format!("{}", i)).collect();
178-
let name_to_index = Some(Self::build_name_to_index(&names));
177+
let name_to_index = Self::build_name_to_index(&names);
179178

180179
Self {
181180
n,
@@ -184,7 +183,7 @@ impl GraphSession {
184183
registry: snapshot,
185184
edges: EdgeBuffer::new(),
186185
names,
187-
name_to_index: RefCell::new(name_to_index),
186+
name_to_index,
188187

189188
core_valid: false,
190189
view_valid: false,
@@ -203,7 +202,7 @@ impl GraphSession {
203202
class: GraphClass,
204203
) -> Self {
205204
let names: Vec<String> = (0..n).map(|i| format!("{}", i)).collect();
206-
let name_to_index = Some(Self::build_name_to_index(&names));
205+
let name_to_index = Self::build_name_to_index(&names);
207206

208207
Self {
209208
n,
@@ -212,7 +211,7 @@ impl GraphSession {
212211
registry,
213212
edges: EdgeBuffer::new(),
214213
names,
215-
name_to_index: RefCell::new(name_to_index),
214+
name_to_index,
216215

217216
core_valid: false,
218217
view_valid: false,
@@ -233,14 +232,15 @@ impl GraphSession {
233232
names: Vec<String>,
234233
) -> Self {
235234
let n = names.len() as u32;
235+
let name_to_index = Self::build_name_to_index(&names);
236236
Self {
237237
n,
238238
simple,
239239
graph_class: class,
240240
registry,
241241
edges,
242242
names,
243-
name_to_index: RefCell::new(None),
243+
name_to_index,
244244
core_valid: false,
245245
view_valid: false,
246246
edges_trusted: true,
@@ -271,14 +271,15 @@ impl GraphSession {
271271
}
272272
}
273273

274+
let name_to_index = Self::build_name_to_index(&names);
274275
Self {
275276
n,
276277
simple,
277278
graph_class: class,
278279
registry,
279280
edges,
280281
names,
281-
name_to_index: RefCell::new(None),
282+
name_to_index,
282283
core_valid: true,
283284
view_valid: false,
284285
edges_trusted: true,
@@ -299,7 +300,7 @@ impl GraphSession {
299300
registry: Arc::clone(&self.registry),
300301
edges: self.edges.clone(),
301302
names: self.names.clone(),
302-
name_to_index: RefCell::new(self.name_to_index.borrow().as_ref().cloned()),
303+
name_to_index: self.name_to_index.clone(),
303304

304305
// Invalidate all declarations in the clone
305306
core_valid: false,
@@ -350,7 +351,7 @@ impl GraphSession {
350351
}
351352

352353
pub fn replace_edges_for_pairs(&mut self, new_edges: EdgeBuffer) {
353-
let mut remove_pairs: HashSet<(u32, u32)> = HashSet::with_capacity(new_edges.len());
354+
let mut remove_pairs: FxHashSet<(u32, u32)> = FxHashSet::with_capacity_and_hasher(new_edges.len(), Default::default());
354355
if self.simple {
355356
for i in 0..new_edges.len() {
356357
let u = new_edges.from[i];
@@ -365,8 +366,8 @@ impl GraphSession {
365366
}
366367

367368
let mut kept = EdgeBuffer::with_capacity(self.edges.len() + new_edges.len());
368-
let mut seen: HashSet<(u32, u32, u8)> =
369-
HashSet::with_capacity(self.edges.len() + new_edges.len());
369+
let mut seen: FxHashSet<(u32, u32, u8)> =
370+
FxHashSet::with_capacity_and_hasher(self.edges.len() + new_edges.len(), Default::default());
370371

371372
for i in 0..self.edges.len() {
372373
let u = self.edges.from[i];
@@ -409,7 +410,7 @@ impl GraphSession {
409410
}
410411

411412
pub fn set_names(&mut self, names: Vec<String>) {
412-
*self.name_to_index.get_mut() = Some(Self::build_name_to_index(&names));
413+
self.name_to_index = Self::build_name_to_index(&names);
413414
self.names = names;
414415
// No invalidation - names are metadata
415416
}
@@ -434,7 +435,7 @@ impl GraphSession {
434435
self.graph_class = class;
435436
self.registry = registry;
436437
self.edges = edges;
437-
*self.name_to_index.get_mut() = Some(Self::build_name_to_index(&names));
438+
self.name_to_index = Self::build_name_to_index(&names);
438439
self.names = names;
439440
self.invalidate_core();
440441
}
@@ -897,32 +898,15 @@ impl GraphSession {
897898
/// Look up the 0-based index of a node by name.
898899
/// Returns `None` if the name is not found.
899900
pub fn index_of(&self, name: &str) -> Option<u32> {
900-
if let Some(map) = self.name_to_index.borrow().as_ref() {
901-
return map.get(name).copied();
902-
}
903-
904-
let built = Self::build_name_to_index(&self.names);
905-
let out = built.get(name).copied();
906-
*self.name_to_index.borrow_mut() = Some(built);
907-
out
901+
self.name_to_index.get(name).copied()
908902
}
909903

910904
/// Look up the 0-based indices of multiple nodes by name.
911905
/// Returns an error if any name is not found.
912906
pub fn indices_of(&self, names: &[String]) -> Result<Vec<u32>, String> {
913-
if self.name_to_index.borrow().is_none() {
914-
let built = Self::build_name_to_index(&self.names);
915-
*self.name_to_index.borrow_mut() = Some(built);
916-
}
917-
918-
let map_ref = self.name_to_index.borrow();
919-
let map = map_ref
920-
.as_ref()
921-
.expect("name_to_index must be initialized before lookup");
922-
923907
let mut result = Vec::with_capacity(names.len());
924908
for name in names {
925-
match map.get(name) {
909+
match self.name_to_index.get(name) {
926910
Some(&idx) => result.push(idx),
927911
None => return Err(format!("Non-existent node name: {}", name)),
928912
}

0 commit comments

Comments
 (0)