Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use sort_by_cached_key where appropriate #49525

Merged
merged 7 commits into from
Apr 12, 2018
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
1 change: 1 addition & 0 deletions src/liballoc/slice.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1400,6 +1400,7 @@ impl<T> [T] {
let sz_usize = mem::size_of::<(K, usize)>();

let len = self.len();
if len < 2 { return }
if sz_u8 < sz_u16 && len <= ( u8::MAX as usize) { return sort_by_key!( u8, self, f) }
if sz_u16 < sz_u32 && len <= (u16::MAX as usize) { return sort_by_key!(u16, self, f) }
if sz_u32 < sz_usize && len <= (u32::MAX as usize) { return sort_by_key!(u32, self, f) }
Expand Down
1 change: 1 addition & 0 deletions src/librustc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
#![feature(refcell_replace_swap)]
#![feature(rustc_diagnostic_macros)]
#![feature(slice_patterns)]
#![feature(slice_sort_by_cached_key)]
#![feature(specialization)]
#![feature(unboxed_closures)]
#![feature(trace_macros)]
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/middle/cstore.rs
Original file line number Diff line number Diff line change
Expand Up @@ -401,7 +401,7 @@ pub fn used_crates(tcx: TyCtxt, prefer: LinkagePreference)
.collect::<Vec<_>>();
let mut ordering = tcx.postorder_cnums(LOCAL_CRATE);
Lrc::make_mut(&mut ordering).reverse();
libs.sort_by_key(|&(a, _)| {
libs.sort_by_cached_key(|&(a, _)| {
ordering.iter().position(|x| *x == a)
});
libs
Expand Down
11 changes: 3 additions & 8 deletions src/librustc_driver/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#![cfg_attr(unix, feature(libc))]
#![feature(quote)]
#![feature(rustc_diagnostic_macros)]
#![feature(slice_sort_by_cached_key)]
#![feature(set_stdio)]
#![feature(rustc_stack_internals)]

Expand Down Expand Up @@ -83,7 +84,6 @@ use rustc_trans_utils::trans_crate::TransCrate;
use serialize::json::ToJson;

use std::any::Any;
use std::cmp::Ordering::Equal;
use std::cmp::max;
use std::default::Default;
use std::env::consts::{DLL_PREFIX, DLL_SUFFIX};
Expand Down Expand Up @@ -1177,13 +1177,8 @@ Available lint options:

fn sort_lints(sess: &Session, lints: Vec<(&'static Lint, bool)>) -> Vec<&'static Lint> {
let mut lints: Vec<_> = lints.into_iter().map(|(x, _)| x).collect();
lints.sort_by(|x: &&Lint, y: &&Lint| {
match x.default_level(sess).cmp(&y.default_level(sess)) {
// The sort doesn't case-fold but it's doubtful we care.
Equal => x.name.cmp(y.name),
r => r,
}
});
// The sort doesn't case-fold but it's doubtful we care.
lints.sort_by_cached_key(|x: &&Lint| (x.default_level(sess), x.name));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh cute!

lints
}

Expand Down
4 changes: 2 additions & 2 deletions src/librustc_metadata/encoder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1414,15 +1414,15 @@ impl<'a, 'b: 'a, 'tcx: 'b> IsolatedEncoder<'a, 'b, 'tcx> {
let mut all_impls: Vec<_> = visitor.impls.into_iter().collect();

// Bring everything into deterministic order for hashing
all_impls.sort_unstable_by_key(|&(trait_def_id, _)| {
all_impls.sort_by_cached_key(|&(trait_def_id, _)| {
tcx.def_path_hash(trait_def_id)
});

let all_impls: Vec<_> = all_impls
.into_iter()
.map(|(trait_def_id, mut impls)| {
// Bring everything into deterministic order for hashing
impls.sort_unstable_by_key(|&def_index| {
impls.sort_by_cached_key(|&def_index| {
tcx.hir.definitions().def_path_hash(def_index)
});

Expand Down
1 change: 1 addition & 0 deletions src/librustc_metadata/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#![feature(macro_lifetime_matcher)]
#![feature(quote)]
#![feature(rustc_diagnostic_macros)]
#![feature(slice_sort_by_cached_key)]
#![feature(specialization)]
#![feature(rustc_private)]

Expand Down
1 change: 1 addition & 0 deletions src/librustc_mir/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ Rust MIR: a lowered representation of Rust. Also: an experiment!
#![deny(warnings)]

#![feature(slice_patterns)]
#![feature(slice_sort_by_cached_key)]
#![feature(from_ref)]
#![feature(box_patterns)]
#![feature(box_syntax)]
Expand Down
12 changes: 5 additions & 7 deletions src/librustc_mir/monomorphize/partitioning.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,11 +112,11 @@ use rustc::ty::{self, TyCtxt, InstanceDef};
use rustc::ty::item_path::characteristic_def_id_of_type;
use rustc::util::nodemap::{FxHashMap, FxHashSet};
use std::collections::hash_map::Entry;
use std::cmp;
use syntax::ast::NodeId;
use syntax::symbol::{Symbol, InternedString};
use rustc::mir::mono::MonoItem;
use monomorphize::item::{MonoItemExt, InstantiationMode};
use core::usize;

pub use rustc::mir::mono::CodegenUnit;

Expand Down Expand Up @@ -189,11 +189,9 @@ pub trait CodegenUnitExt<'tcx> {
}, item.symbol_name(tcx))
}

let items: Vec<_> = self.items().iter().map(|(&i, &l)| (i, l)).collect();
let mut items : Vec<_> = items.iter()
.map(|il| (il, item_sort_key(tcx, il.0))).collect();
items.sort_by(|&(_, ref key1), &(_, ref key2)| key1.cmp(key2));
items.into_iter().map(|(&item_linkage, _)| item_linkage).collect()
let mut items: Vec<_> = self.items().iter().map(|(&i, &l)| (i, l)).collect();
items.sort_by_cached_key(|&(i, _)| item_sort_key(tcx, i));
items
}
}

Expand Down Expand Up @@ -509,7 +507,7 @@ fn merge_codegen_units<'tcx>(initial_partitioning: &mut PreInliningPartitioning<
// Merge the two smallest codegen units until the target size is reached.
while codegen_units.len() > target_cgu_count {
// Sort small cgus to the back
codegen_units.sort_by_key(|cgu| usize::MAX - cgu.size_estimate());
codegen_units.sort_by_cached_key(|cgu| cmp::Reverse(cgu.size_estimate()));
let mut smallest = codegen_units.pop().unwrap();
let second_smallest = codegen_units.last_mut().unwrap();

Expand Down
17 changes: 8 additions & 9 deletions src/librustc_resolve/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#![deny(warnings)]

#![feature(rustc_diagnostic_macros)]
#![feature(slice_sort_by_cached_key)]

#[macro_use]
extern crate log;
Expand Down Expand Up @@ -1150,13 +1151,9 @@ impl<'a> ModuleData<'a> {

fn for_each_child_stable<F: FnMut(Ident, Namespace, &'a NameBinding<'a>)>(&self, mut f: F) {
let resolutions = self.resolutions.borrow();
let mut resolutions = resolutions.iter().map(|(&(ident, ns), &resolution)| {
// Pre-compute keys for sorting
(ident.name.as_str(), ns, ident, resolution)
})
.collect::<Vec<_>>();
resolutions.sort_unstable_by_key(|&(str, ns, ..)| (str, ns));
for &(_, ns, ident, resolution) in resolutions.iter() {
let mut resolutions = resolutions.iter().collect::<Vec<_>>();
resolutions.sort_by_cached_key(|&(&(ident, ns), _)| (ident.name.as_str(), ns));
for &(&(ident, ns), &resolution) in resolutions.iter() {
resolution.borrow().binding.map(|binding| f(ident, ns, binding));
}
}
Expand Down Expand Up @@ -3341,7 +3338,9 @@ impl<'a> Resolver<'a> {
let is_mod = |def| match def { Def::Mod(..) => true, _ => false };
let mut candidates =
self.lookup_import_candidates(name, TypeNS, is_mod);
candidates.sort_by_key(|c| (c.path.segments.len(), c.path.to_string()));
candidates.sort_by_cached_key(|c| {
(c.path.segments.len(), c.path.to_string())
});
if let Some(candidate) = candidates.get(0) {
format!("Did you mean `{}`?", candidate.path)
} else {
Expand Down Expand Up @@ -3579,7 +3578,7 @@ impl<'a> Resolver<'a> {

let name = path[path.len() - 1].name;
// Make sure error reporting is deterministic.
names.sort_by_key(|name| name.as_str());
names.sort_by_cached_key(|name| name.as_str());
match find_best_match_for_name(names.iter(), &name.as_str(), None) {
Some(found) if found != name => Some(found),
_ => None,
Expand Down
5 changes: 3 additions & 2 deletions src/librustc_trans/base.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,8 @@ use std::ffi::CString;
use std::str;
use std::sync::Arc;
use std::time::{Instant, Duration};
use std::{i32, usize};
use std::i32;
use std::cmp;
use std::sync::mpsc;
use syntax_pos::Span;
use syntax_pos::symbol::InternedString;
Expand Down Expand Up @@ -830,7 +831,7 @@ pub fn trans_crate<'a, 'tcx>(tcx: TyCtxt<'a, 'tcx, 'tcx>,
// a bit more efficiently.
let codegen_units = {
let mut codegen_units = codegen_units;
codegen_units.sort_by_key(|cgu| usize::MAX - cgu.size_estimate());
codegen_units.sort_by_cached_key(|cgu| cmp::Reverse(cgu.size_estimate()));
codegen_units
};

Expand Down
1 change: 1 addition & 0 deletions src/librustc_trans/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
#![feature(libc)]
#![feature(quote)]
#![feature(rustc_diagnostic_macros)]
#![feature(slice_sort_by_cached_key)]
#![feature(optin_builtin_traits)]
#![feature(inclusive_range_fields)]
#![feature(underscore_lifetimes)]
Expand Down
2 changes: 1 addition & 1 deletion src/librustc_typeck/check/method/probe.rs
Original file line number Diff line number Diff line change
Expand Up @@ -799,7 +799,7 @@ impl<'a, 'gcx, 'tcx> ProbeContext<'a, 'gcx, 'tcx> {
.collect();

// sort them by the name so we have a stable result
names.sort_by_key(|n| n.as_str());
names.sort_by_cached_key(|n| n.as_str());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as_str comment

names
}

Expand Down
1 change: 1 addition & 0 deletions src/librustc_typeck/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ This API is completely unstable and subject to change.
#![feature(refcell_replace_swap)]
#![feature(rustc_diagnostic_macros)]
#![feature(slice_patterns)]
#![feature(slice_sort_by_cached_key)]
#![feature(dyn_trait)]

#[macro_use] extern crate log;
Expand Down
4 changes: 1 addition & 3 deletions src/librustdoc/clean/auto_trait.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1435,9 +1435,7 @@ impl<'a, 'tcx, 'rcx> AutoTraitFinder<'a, 'tcx, 'rcx> {
// involved (impls rarely have more than a few bounds) means that it
// shouldn't matter in practice.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also probably not this PR, but this comment makes me wonder if it would be worth adding if len < 2 { return } to the checks excerpted below, since sort_by_cached_key is currently a pessimization in the len==1 case, as it'll compute the key and allocate, which ordinary sort_by_key wouldn't.

rust/src/liballoc/slice.rs

Lines 1402 to 1406 in 1c5283b

let len = self.len();
if sz_u8 < sz_u16 && len <= ( u8::MAX as usize) { return sort_by_key!( u8, self, f) }
if sz_u16 < sz_u32 && len <= (u16::MAX as usize) { return sort_by_key!(u16, self, f) }
if sz_u32 < sz_usize && len <= (u32::MAX as usize) { return sort_by_key!(u32, self, f) }
sort_by_key!(usize, self, f)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's another good point: I'll add that extra check in! (len == 2 could also be special cased to not allocate, as only one comparison will be executed, but that's a less likely case to encounter.)

fn unstable_debug_sort<T: Debug>(&self, vec: &mut Vec<T>) {
vec.sort_unstable_by(|first, second| {
format!("{:?}", first).cmp(&format!("{:?}", second))
});
vec.sort_by_cached_key(|x| format!("{:?}", x))
}

fn is_fn_ty(&self, tcx: &TyCtxt, ty: &Type) -> bool {
Expand Down
1 change: 1 addition & 0 deletions src/librustdoc/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#![feature(box_syntax)]
#![feature(fs_read_write)]
#![feature(set_stdio)]
#![feature(slice_sort_by_cached_key)]
#![feature(test)]
#![feature(unicode)]
#![feature(vec_remove_item)]
Expand Down
1 change: 1 addition & 0 deletions src/libsyntax/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@

#![feature(unicode)]
#![feature(rustc_diagnostic_macros)]
#![feature(slice_sort_by_cached_key)]
#![feature(non_exhaustive)]
#![feature(const_atomic_usize_new)]
#![feature(rustc_attrs)]
Expand Down
2 changes: 1 addition & 1 deletion src/libsyntax/parse/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -689,7 +689,7 @@ impl<'a> Parser<'a> {
.chain(inedible.iter().map(|x| TokenType::Token(x.clone())))
.chain(self.expected_tokens.iter().cloned())
.collect::<Vec<_>>();
expected.sort_by(|a, b| a.to_string().cmp(&b.to_string()));
expected.sort_by_cached_key(|x| x.to_string());
expected.dedup();
let expect = tokens_to_string(&expected[..]);
let actual = self.this_token_to_string();
Expand Down