Skip to content

Commit

Permalink
Rollup merge of rust-lang#47656 - ishitatsuyuki:patch-1, r=nikomatsakis
Browse files Browse the repository at this point in the history
[perf] Use std based dedup in projection

Unstable sort was added recently, and the code that is being modified is 3 years old. As quicksort doesn't allocate it will likely perform as well as, or better than linear search.

I didn't benchmark. Have a perf run.
  • Loading branch information
alexcrichton committed Jan 25, 2018
2 parents 8dd36af + c6772b4 commit 014931b
Show file tree
Hide file tree
Showing 4 changed files with 39 additions and 20 deletions.
23 changes: 7 additions & 16 deletions src/librustc/traits/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,7 @@ pub struct MismatchedProjectionTypes<'tcx> {
pub err: ty::error::TypeError<'tcx>
}

#[derive(PartialEq, Eq, Debug)]
#[derive(PartialEq, Eq, PartialOrd, Ord, Debug)]
enum ProjectionTyCandidate<'tcx> {
// from a where-clause in the env or object type
ParamEnv(ty::PolyProjectionPredicate<'tcx>),
Expand Down Expand Up @@ -838,21 +838,12 @@ fn project_type<'cx, 'gcx, 'tcx>(
// Drop duplicates.
//
// Note: `candidates.vec` seems to be on the critical path of the
// compiler. Replacing it with an hash set was also tried, which would
// render the following dedup unnecessary. It led to cleaner code but
// prolonged compiling time of `librustc` from 5m30s to 6m in one test, or
// ~9% performance lost.
if candidates.vec.len() > 1 {
let mut i = 0;
while i < candidates.vec.len() {
let has_dup = (0..i).any(|j| candidates.vec[i] == candidates.vec[j]);
if has_dup {
candidates.vec.swap_remove(i);
} else {
i += 1;
}
}
}
// compiler. Replacing it with an HashSet was also tried, which would
// render the following dedup unnecessary. The original comment indicated
// that it was 9% slower, but that data is now obsolete and a new
// benchmark should be performed.
candidates.vec.sort_unstable();
candidates.vec.dedup();

// Prefer where-clauses. As in select, if there are multiple
// candidates, we prefer where-clause candidates over impls. This
Expand Down
30 changes: 29 additions & 1 deletion src/librustc/ty/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ use serialize::{self, Encodable, Encoder};
use std::cell::RefCell;
use std::collections::BTreeMap;
use std::cmp;
use std::cmp::Ordering;
use std::fmt;
use std::hash::{Hash, Hasher};
use std::iter::FromIterator;
Expand Down Expand Up @@ -499,6 +500,20 @@ impl<'tcx> Hash for TyS<'tcx> {
}
}

impl<'tcx> Ord for TyS<'tcx> {
#[inline]
fn cmp(&self, other: &TyS<'tcx>) -> Ordering {
// (self as *const _).cmp(other as *const _)
(self as *const TyS<'tcx>).cmp(&(other as *const TyS<'tcx>))
}
}
impl<'tcx> PartialOrd for TyS<'tcx> {
#[inline]
fn partial_cmp(&self, other: &TyS<'tcx>) -> Option<Ordering> {
Some(self.cmp(other))
}
}

impl<'tcx> TyS<'tcx> {
pub fn is_primitive_ty(&self) -> bool {
match self.sty {
Expand Down Expand Up @@ -568,6 +583,19 @@ impl<T> PartialEq for Slice<T> {
}
impl<T> Eq for Slice<T> {}

impl<T> Ord for Slice<T> {
#[inline]
fn cmp(&self, other: &Slice<T>) -> Ordering {
(&self.0 as *const [T]).cmp(&(&other.0 as *const [T]))
}
}
impl<T> PartialOrd for Slice<T> {
#[inline]
fn partial_cmp(&self, other: &Slice<T>) -> Option<Ordering> {
Some(self.cmp(other))
}
}

impl<T> Hash for Slice<T> {
fn hash<H: Hasher>(&self, s: &mut H) {
(self.as_ptr(), self.len()).hash(s)
Expand Down Expand Up @@ -1103,7 +1131,7 @@ pub type PolySubtypePredicate<'tcx> = ty::Binder<SubtypePredicate<'tcx>>;
/// equality between arbitrary types. Processing an instance of
/// Form #2 eventually yields one of these `ProjectionPredicate`
/// instances to normalize the LHS.
#[derive(Copy, Clone, PartialEq, Eq, Hash, RustcEncodable, RustcDecodable)]
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, RustcEncodable, RustcDecodable)]
pub struct ProjectionPredicate<'tcx> {
pub projection_ty: ProjectionTy<'tcx>,
pub ty: Ty<'tcx>,
Expand Down
4 changes: 2 additions & 2 deletions src/librustc/ty/sty.rs
Original file line number Diff line number Diff line change
Expand Up @@ -638,7 +638,7 @@ impl<'tcx> PolyExistentialTraitRef<'tcx> {
/// erase, or otherwise "discharge" these bound regions, we change the
/// type from `Binder<T>` to just `T` (see
/// e.g. `liberate_late_bound_regions`).
#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug, RustcEncodable, RustcDecodable)]
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, RustcEncodable, RustcDecodable)]
pub struct Binder<T>(pub T);

impl<T> Binder<T> {
Expand Down Expand Up @@ -738,7 +738,7 @@ impl<T> Binder<T> {

/// Represents the projection of an associated type. In explicit UFCS
/// form this would be written `<T as Trait<..>>::N`.
#[derive(Copy, Clone, PartialEq, Eq, Hash, Debug, RustcEncodable, RustcDecodable)]
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug, RustcEncodable, RustcDecodable)]
pub struct ProjectionTy<'tcx> {
/// The parameters of the associated item.
pub substs: &'tcx Substs<'tcx>,
Expand Down
2 changes: 1 addition & 1 deletion src/librustc/ty/subst.rs
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ use std::mem;
/// To reduce memory usage, a `Kind` is a interned pointer,
/// with the lowest 2 bits being reserved for a tag to
/// indicate the type (`Ty` or `Region`) it points to.
#[derive(Copy, Clone, PartialEq, Eq, Hash)]
#[derive(Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash)]
pub struct Kind<'tcx> {
ptr: NonZero<usize>,
marker: PhantomData<(Ty<'tcx>, ty::Region<'tcx>)>
Expand Down

0 comments on commit 014931b

Please sign in to comment.