From 9514859206e6a401892c0190201fca100b7f9450 Mon Sep 17 00:00:00 2001 From: Predrag Gruevski <2348618+obi1kenobi@users.noreply.github.com> Date: Wed, 28 Aug 2024 16:56:44 -0400 Subject: [PATCH] feat: faster IndexedCrate::new with rayon (#418) (#420) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Added a new *optional* rayon feature that improves performance. ```console $ cargo criterion --bench indexed_crate IndexedCrate/new(aws-sdk-ec2) time: [1.3074 s 1.3080 s 1.3085 s] $ cargo criterion --bench indexed_crate --features rayon IndexedCrate/new(aws-sdk-ec2) time: [568.84 ms 569.98 ms 571.14 ms] change: [-56.521% -56.422% -56.333%] (p = 0.00 < 0.05) Performance has improved. ``` Co-authored-by: Jalil David Salamé Messina <60845989+jalil-salame@users.noreply.github.com> --- .github/workflows/ci.yml | 6 +++ Cargo.lock | 1 + Cargo.toml | 8 ++++ src/indexed_crate.rs | 96 +++++++++++++++++++++++++++++++++------- 4 files changed, 96 insertions(+), 15 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f366c7e..2adb96b 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -88,6 +88,12 @@ jobs: - name: test run: cargo test + - name: compile with rayon + run: cargo test --no-run --features rayon + + - name: test with rayon + run: cargo test --features rayon + publish: name: Publish to crates.io runs-on: ubuntu-latest diff --git a/Cargo.lock b/Cargo.lock index b2d3ac8..6b6f3e7 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -640,6 +640,7 @@ dependencies = [ "criterion", "itertools 0.12.1", "maplit", + "rayon", "rustdoc-types", "serde", "serde_json", diff --git a/Cargo.toml b/Cargo.toml index d27eadf..e126708 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -13,6 +13,14 @@ readme = "./README.md" [dependencies] trustfall = "0.7.1" rustdoc-types = "0.26.0" +rayon = { version = "1.10.0", optional = true } + +[features] +default = [] +# Enable rayon for speeding up building `IndexedCrate`, this massively improves +# performance on big crates, but may impact performance on small crates and +# definitely increases the memory usage +rayon = ["dep:rayon"] [[bench]] name = "indexed_crate" diff --git a/src/indexed_crate.rs b/src/indexed_crate.rs index e7fbe81..7b0508d 100644 --- a/src/indexed_crate.rs +++ b/src/indexed_crate.rs @@ -3,6 +3,8 @@ use std::{ collections::{hash_map::Entry, HashMap, HashSet}, }; +#[cfg(feature = "rayon")] +use rayon::prelude::*; use rustdoc_types::{Crate, Id, Item}; use crate::{adapter::supported_item_kind, sealed_trait, visibility_tracker::VisibilityTracker}; @@ -47,6 +49,29 @@ pub struct IndexedCrate<'a> { /// many values. struct MapList(HashMap>); +#[cfg(feature = "rayon")] +impl FromParallelIterator<(K, V)> + for MapList +{ + #[inline] + fn from_par_iter(par_iter: I) -> Self + where + I: IntoParallelIterator, + { + par_iter + .into_par_iter() + .fold(Self::new, |mut map, (key, value)| { + map.insert(key, value); + map + }) + // Reduce left is faster than reduce right (about 19% less time in our benchmarks) + .reduce(Self::new, |mut l, r| { + l.merge(r); + l + }) + } +} + impl FromIterator<(K, V)> for MapList { #[inline] fn from_iter>(iter: T) -> Self { @@ -91,6 +116,26 @@ impl MapList { } } } + + #[inline] + #[cfg(feature = "rayon")] + pub fn insert_many(&mut self, key: K, mut value: Vec) { + match self.0.entry(key) { + Entry::Occupied(mut entry) => entry.get_mut().append(&mut value), + Entry::Vacant(entry) => { + entry.insert(value); + } + } + } + + #[inline] + #[cfg(feature = "rayon")] + pub fn merge(&mut self, other: Self) { + self.0.reserve(other.0.len()); + for (key, value) in other.0 { + self.insert_many(key, value); + } + } } /// Build the impl index @@ -98,6 +143,9 @@ impl MapList { /// When compiled using the `rayon` feature, build it in parallel. Specifically, this paralelizes /// the work of gathering all of the impls for the items in the index. fn build_impl_index(index: &HashMap) -> MapList, (&Item, &Item)> { + #[cfg(feature = "rayon")] + let iter = index.par_iter(); + #[cfg(not(feature = "rayon"))] let iter = index.iter(); iter.filter_map(|(id, item)| { let impls = match &item.inner { @@ -107,6 +155,9 @@ fn build_impl_index(index: &HashMap) -> MapList, (&Item, _ => return None, }; + #[cfg(feature = "rayon")] + let iter = impls.par_iter(); + #[cfg(not(feature = "rayon"))] let iter = impls.iter(); Some((id, iter.filter_map(|impl_id| index.get(impl_id)))) @@ -136,6 +187,9 @@ fn build_impl_index(index: &HashMap) -> MapList, (&Item, }) .unwrap_or(&[]); + #[cfg(feature = "rayon")] + let trait_items = trait_items.par_iter(); + #[cfg(not(feature = "rayon"))] let trait_items = trait_items.iter(); let trait_provided_items = trait_items @@ -159,6 +213,9 @@ fn build_impl_index(index: &HashMap) -> MapList, (&Item, ) }); + #[cfg(feature = "rayon")] + let impl_items = impl_inner.items.par_iter(); + #[cfg(not(feature = "rayon"))] let impl_items = impl_inner.items.iter(); impl_items @@ -187,23 +244,32 @@ impl<'a> IndexedCrate<'a> { // // This is inlined because we need access to `value`, but `value` is not a valid // `IndexedCrate` yet. Do not extract into a separate function. + #[cfg(feature = "rayon")] + let iter = crate_.index.par_iter(); + #[cfg(not(feature = "rayon"))] + let iter = crate_.index.iter(); + value.imports_index = Some( - crate_ - .index - .iter() - .filter_map(|(_id, item)| { - if !supported_item_kind(item) { - return None; - } - let importable_paths = value.publicly_importable_names(&item.id); - Some(importable_paths.into_iter().map(move |importable_path| { - (importable_path.path, (item, importable_path.modifiers)) - })) - }) - .flatten() - .collect::>() - .into_inner(), + iter.filter_map(|(_id, item)| { + if !supported_item_kind(item) { + return None; + } + let importable_paths = value.publicly_importable_names(&item.id); + + #[cfg(feature = "rayon")] + let iter = importable_paths.into_par_iter(); + #[cfg(not(feature = "rayon"))] + let iter = importable_paths.into_iter(); + + Some(iter.map(move |importable_path| { + (importable_path.path, (item, importable_path.modifiers)) + })) + }) + .flatten() + .collect::>() + .into_inner(), ); + value.impl_index = Some(build_impl_index(&crate_.index).into_inner()); value