Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
Avoid Unstable Sort (#12455)
Browse files Browse the repository at this point in the history
* dont use unstable sort

* remove comment

* add clippy rule
  • Loading branch information
shawntabrizi authored Oct 11, 2022
1 parent 1bf2e6d commit ccc8f6c
Show file tree
Hide file tree
Showing 9 changed files with 14 additions and 17 deletions.
5 changes: 3 additions & 2 deletions .cargo/config.toml
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,15 @@ rustflags = [
"-Aclippy::borrowed-box", # Reasonable to fix this one
"-Aclippy::too-many-arguments", # (Turning this on would lead to)
"-Aclippy::unnecessary_cast", # Types may change
"-Aclippy::identity-op", # One case where we do 0 +
"-Aclippy::identity-op", # One case where we do 0 +
"-Aclippy::useless_conversion", # Types may change
"-Aclippy::unit_arg", # styalistic.
"-Aclippy::option-map-unit-fn", # styalistic
"-Aclippy::bind_instead_of_map", # styalistic
"-Aclippy::bind_instead_of_map", # styalistic
"-Aclippy::erasing_op", # E.g. 0 * DOLLARS
"-Aclippy::eq_op", # In tests we test equality.
"-Aclippy::while_immutable_condition", # false positives
"-Aclippy::needless_option_as_deref", # false positives
"-Aclippy::derivable_impls", # false positives
"-Aclippy::stable_sort_primitive", # prefer stable sort
]
2 changes: 1 addition & 1 deletion bin/node/bench/src/core.rs
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ pub fn run_benchmark(benchmark: Box<dyn BenchmarkDescription>, mode: Mode) -> Be
durations.push(duration.as_nanos());
}

durations.sort_unstable();
durations.sort();

let raw_average = (durations.iter().sum::<u128>() / (durations.len() as u128)) as u64;
let average = (durations.iter().skip(10).take(30).sum::<u128>() / 30) as u64;
Expand Down
2 changes: 1 addition & 1 deletion client/rpc-servers/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -198,7 +198,7 @@ fn format_allowed_hosts(addrs: &[SocketAddr]) -> Vec<String> {

fn build_rpc_api<M: Send + Sync + 'static>(mut rpc_api: RpcModule<M>) -> RpcModule<M> {
let mut available_methods = rpc_api.method_names().collect::<Vec<_>>();
available_methods.sort_unstable();
available_methods.sort();

rpc_api
.register_method("rpc_methods", move |_, _| {
Expand Down
4 changes: 2 additions & 2 deletions frame/benchmarking/src/analysis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,7 @@ impl Analysis {
})
.collect();

values.sort_unstable();
values.sort();
let mid = values.len() / 2;

Some(Self {
Expand Down Expand Up @@ -311,7 +311,7 @@ impl Analysis {
}

for (_, rs) in results.iter_mut() {
rs.sort_unstable();
rs.sort();
let ql = rs.len() / 4;
*rs = rs[ql..rs.len() - ql].to_vec();
}
Expand Down
6 changes: 1 addition & 5 deletions frame/contracts/src/wasm/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1976,11 +1976,7 @@ pub mod env {
data_len: u32,
) -> Result<(), TrapReason> {
fn has_duplicates<T: Ord>(items: &mut Vec<T>) -> bool {
// # Warning
//
// Unstable sorts are non-deterministic across architectures. The usage here is OK
// because we are rejecting duplicates which removes the non determinism.
items.sort_unstable();
items.sort();
// Find any two consecutive equal elements.
items.windows(2).any(|w| match &w {
&[a, b] => a == b,
Expand Down
2 changes: 1 addition & 1 deletion frame/election-provider-multi-phase/src/mock.rs
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ pub fn trim_helpers() -> TrimHelpers {
seq_phragmen(desired_targets as usize, targets.clone(), voters.clone(), None).unwrap();

// sort by decreasing order of stake
assignments.sort_unstable_by_key(|assignment| {
assignments.sort_by_key(|assignment| {
std::cmp::Reverse(stakes.get(&assignment.who).cloned().unwrap_or_default())
});

Expand Down
2 changes: 1 addition & 1 deletion frame/examples/basic/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ benchmarks! {
}
}: {
// The benchmark execution phase could also be a closure with custom code
m.sort_unstable();
m.sort();
}

// This line generates test cases for benchmarking, and could be run by:
Expand Down
6 changes: 3 additions & 3 deletions primitives/npos-elections/fuzzer/src/common.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ pub fn generate_random_npos_inputs(
}
candidates.push(id);
}
candidates.sort_unstable();
candidates.sort();
candidates.dedup();
assert_eq!(candidates.len(), candidate_count);

Expand All @@ -99,11 +99,11 @@ pub fn generate_random_npos_inputs(

let mut chosen_candidates = Vec::with_capacity(n_candidates_chosen);
chosen_candidates.extend(candidates.choose_multiple(&mut rng, n_candidates_chosen));
chosen_candidates.sort_unstable();
chosen_candidates.sort();
voters.push((id, vote_weight, chosen_candidates));
}

voters.sort_unstable();
voters.sort();
voters.dedup_by_key(|(id, _weight, _chosen_candidates)| *id);
assert_eq!(voters.len(), voter_count);

Expand Down
2 changes: 1 addition & 1 deletion utils/frame/benchmarking-cli/src/shared/stats.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ impl Stats {
/// Returns the specified percentile for the given data.
/// This is best effort since it ignores the interpolation case.
fn percentile(mut xs: Vec<u64>, p: f64) -> u64 {
xs.sort_unstable();
xs.sort();
let index = (xs.len() as f64 * p).ceil() as usize - 1;
xs[index.clamp(0, xs.len() - 1)]
}
Expand Down

0 comments on commit ccc8f6c

Please sign in to comment.