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

Avoid Unstable Sort #12455

Merged
merged 3 commits into from
Oct 11, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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