Skip to content

Commit

Permalink
refactor(semantic): pass Rc by value (#3586)
Browse files Browse the repository at this point in the history
Same as #3550.

`Rc<T>` is already a reference, so instead of passing an `&Rc<T>` to a function and then `Rc::clone()` it in the function, it's better to clone it first and pass `Rc<T>` to the function.

`Rc<T>` and `&Rc<T>` are both 8 bytes, so it introduces no additional overhead to the function call, and reduces indirection.

This is a very small optimization. Am only submitting these changes for purpose of code tidying - making the patterns around `Rc` consistent and optimal throughout the codebase.

We should probably look if we can remove some of these `Rc`s entirely and replace them with plain `&` refs. I suspect `Rc` is not actually required in most places and we're only using it to avoid dealing with lifetimes, but it's sub-optimal as `Rc::clone` has a cost, whereas copying a `&` ref has none.
  • Loading branch information
overlookmotel committed Jun 8, 2024
1 parent b09092c commit 7d61832
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 5 deletions.
7 changes: 4 additions & 3 deletions crates/oxc_semantic/src/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ impl<'a> SemanticBuilder<'a> {
symbols: SymbolTable::default(),
module_record: Arc::new(ModuleRecord::default()),
label_builder: LabelBuilder::default(),
jsdoc: JSDocBuilder::new(source_text, &trivias),
jsdoc: JSDocBuilder::new(source_text, trivias),
check_syntax_error: false,
cfg: ControlFlowGraphBuilder::default(),
class_table_builder: ClassTableBuilder::new(),
Expand All @@ -109,8 +109,9 @@ impl<'a> SemanticBuilder<'a> {

#[must_use]
pub fn with_trivias(mut self, trivias: Trivias) -> Self {
self.trivias = Rc::new(trivias);
self.jsdoc = JSDocBuilder::new(self.source_text, &self.trivias);
let trivias = Rc::new(trivias);
self.trivias = Rc::clone(&trivias);
self.jsdoc = JSDocBuilder::new(self.source_text, trivias);
self
}

Expand Down
4 changes: 2 additions & 2 deletions crates/oxc_semantic/src/jsdoc/builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ pub struct JSDocBuilder<'a> {
}

impl<'a> JSDocBuilder<'a> {
pub fn new(source_text: &'a str, trivias: &Rc<Trivias>) -> Self {
pub fn new(source_text: &'a str, trivias: Rc<Trivias>) -> Self {
Self {
source_text,
trivias: Rc::clone(trivias),
trivias,
attached_docs: BTreeMap::default(),
leading_comments_seen: FxHashSet::default(),
}
Expand Down

0 comments on commit 7d61832

Please sign in to comment.