-
Notifications
You must be signed in to change notification settings - Fork 17
Description
When doing some memory profiling, I noticed that Locals::push ends up requiring a lot grows:

The location for the initial pushes appears to be here:
tree-house/highlighter/src/locals.rs
Lines 209 to 217 in 16e1189
| scope = locals.push(ScopeData { | |
| definitions: HashMap::new(), | |
| range: matched_node.node.byte_range(), | |
| inherit: !injection_query | |
| .not_scope_inherits | |
| .contains(&query_match.pattern()), | |
| children: Vec::new(), | |
| parent: Some(scope), | |
| }); |
With push also calling the inner Vec::push:
tree-house/highlighter/src/locals.rs
Lines 44 to 52 in 16e1189
| fn push(&mut self, scope: ScopeData) -> Scope { | |
| let new_scope_id = Scope(self.scopes.len() as u32); | |
| let parent = scope | |
| .parent | |
| .expect("push cannot be used for the root layer"); | |
| self[parent].children.push(new_scope_id); | |
| self.scopes.push(scope); | |
| new_scope_id | |
| } |
locals is instantiated via Locals::default. Currently the default impl is using 4 as a starting point:
tree-house/highlighter/src/locals.rs
Lines 28 to 41 in 16e1189
| impl Default for Locals { | |
| fn default() -> Self { | |
| let mut scopes = Vec::with_capacity(4); | |
| scopes.push(ScopeData { | |
| definitions: HashMap::new(), | |
| range: 0..u32::MAX, | |
| inherit: false, | |
| children: Vec::new(), | |
| parent: None, | |
| }); | |
| Self { scopes } | |
| } | |
| } |
I wonder if increasing the starting size a bit could end up removing the need for some of the grows? Maybe try 8? Or 16? I don't know enough about the full context of how this function works to know if this is a meaningful change or not, but thought I would share.
As far as other places for large allocation numbers (not amount allocated, just the amount of allocations), most other areas in the screenshot lead directly into the C tree-sitter implementation. Of the spots in the actual Rust code, found just a few so far.