Skip to content

Refactoring & Optional data concurrency #35

Open
@chris-ha458

Description

I was thinking about incorporating rayon in certain sections of the code behind a feature flag.
For instance
pub fn most_common_tiebreaker<F>
could be changed as follows

pub fn most_common_tiebreaker<F>(&self, mut tiebreaker: F) -> Vec<(T, N)>
where
    F: FnMut(&T, &T) -> ::std::cmp::Ordering + Send + Sync,
{
    let mut items = self
        .map
        .iter() // i'm not sure if changing this into par_iter() would be more useful or needless complication
        .map(|(key, count)| (key.clone(), count.clone()))
        .collect::<Vec<_>>();
    #[cfg(feature = "use_rayon")]
    {
        use rayon::prelude::*;
        items.par_sort_unstable_by(|(a_item, a_count), (b_item, b_count)| {
            b_count
                .cmp(a_count)
                .then_with(|| tiebreaker(a_item, b_item))
        });
    }
    #[cfg(not(feature = "use_rayon"))]
    {
        items.sort_unstable_by(|(a_item, a_count), (b_item, b_count)| {
            b_count
                .cmp(a_count)
                .then_with(|| tiebreaker(a_item, b_item))
        });
    }
    items
}

Similar feature flag based dependencies would be set in Cargo.toml as well.
Overall, this would be integrated in a way that preserves default behavior unless a feature flag is set (likely as rayon)

If you are open to this, I would first begin by initiating a bit of refactoring first.
Most if not all of the code is situated inside lib.rs which is now pushing 2k loc.
I think the unit tests can be extracted into /src/lib/tests.rs as a separate file.
This is in contrast to /tests/integration_tests.rs which would be for integration tests.
Of course, if any specific test is identified to be more suitable for integration tests I would extract them and organise them as such.

What do you feel about each?

  • Refactoring the code, especially for the tests.
  • Adding rayon (behind a feature flag to preserve default behavior)

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions