Skip to content

Commit

Permalink
feat: faster VisibilityTracker::from_crate (#404)
Browse files Browse the repository at this point in the history
* feat: faster VisibilityTracker::from_crate

We reduce the number of times `&Id`s get hashed and get a nice perf
boost thanks to that.

Numbers on my machine:

```console
$ cargo criterion --bench indexed_crate
IndexedCrate/new(aws-sdk-ec2)
                        time:   [1.5595 s 1.5713 s 1.5827 s]
                        change: [-8.3651% -7.5024% -6.6525%] (p = 0.00 < 0.05)
                        Performance has improved.
```

* Update src/visibility_tracker.rs

---------

Co-authored-by: Predrag Gruevski <[email protected]>
  • Loading branch information
jalil-salame and obi1kenobi committed Aug 28, 2024
1 parent f159a30 commit 3fd81c0
Showing 1 changed file with 17 additions and 15 deletions.
32 changes: 17 additions & 15 deletions src/visibility_tracker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,14 @@ pub(crate) struct VisibilityTracker<'a> {

impl<'a> VisibilityTracker<'a> {
pub(crate) fn from_crate(crate_: &'a Crate) -> Self {
let visible_parent_ids = compute_parent_ids_for_public_items(crate_)
.into_iter()
.map(|(key, values)| {
// Ensure a consistent order, since queries can observe this order directly.
let mut values: Vec<_> = values.into_iter().collect();
values.sort_unstable_by_key(|x| &x.0);
(key, values)
})
.collect();
let mut visible_parent_ids = compute_parent_ids_for_public_items(crate_);

// Sort and deduplicate parent ids.
// This ensures a consistent order, since queries can observe this order directly.
visible_parent_ids.iter_mut().for_each(|(_id, parent_ids)| {
parent_ids.sort_unstable_by_key(|id| id.0.as_str());
parent_ids.dedup_by_key(|id| id.0.as_str());
});

Self {
inner: crate_,
Expand Down Expand Up @@ -249,8 +248,7 @@ struct NameResolution<'a> {
duplicated_glob_names_in_module: HashMap<&'a Id, HashSet<NamespacedName<'a>>>,
}

fn compute_parent_ids_for_public_items(crate_: &Crate) -> HashMap<&Id, HashSet<&Id>> {
let mut result = Default::default();
fn compute_parent_ids_for_public_items(crate_: &Crate) -> HashMap<&Id, Vec<&Id>> {
let root_id = &crate_.root;

if let Some(root_module) = crate_.index.get(root_id) {
Expand All @@ -260,6 +258,8 @@ fn compute_parent_ids_for_public_items(crate_: &Crate) -> HashMap<&Id, HashSet<&
// Avoid cycles by keeping track of which items we're in the middle of visiting.
let mut currently_visited_items: HashSet<&Id> = Default::default();

let mut result = Default::default();

visit_root_reachable_public_items(
crate_,
&mut result,
Expand All @@ -268,10 +268,12 @@ fn compute_parent_ids_for_public_items(crate_: &Crate) -> HashMap<&Id, HashSet<&
root_module,
None,
);

return result;
}
}

result
Default::default()
}

fn get_names_for_item<'a>(
Expand Down Expand Up @@ -471,7 +473,7 @@ fn resolve_glob_imported_names<'a>(crate_: &'a Crate, traversal_state: &mut Name

// Glob-of-glob import chains might still produce `names` and `duplicated_names` entries
// that would be shadowed by locally-defined names in this module. Apply the shadowing
// rules by removing any conflicing names from both of those collections.
// rules by removing any conflicting names from both of those collections.
if let Some(local_names) = traversal_state.names_defined_in_module.get(module_id) {
for local_name in local_names.keys() {
names.remove(local_name);
Expand Down Expand Up @@ -606,7 +608,7 @@ fn register_name<'a>(
/// Collect all public items that are reachable from the crate root and record their parent Ids.
fn visit_root_reachable_public_items<'a>(
crate_: &'a Crate,
parents: &mut HashMap<&'a Id, HashSet<&'a Id>>,
parents: &mut HashMap<&'a Id, Vec<&'a Id>>,
traversal_state: &NameResolution<'a>,
currently_visited_items: &mut HashSet<&'a Id>,
item: &'a Item,
Expand All @@ -628,7 +630,7 @@ fn visit_root_reachable_public_items<'a>(

let item_parents = parents.entry(&item.id).or_default();
if let Some(parent_id) = parent_id {
item_parents.insert(parent_id);
item_parents.push(parent_id);
}

if !currently_visited_items.insert(&item.id) {
Expand Down

0 comments on commit 3fd81c0

Please sign in to comment.