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

[do not merge] Remove PackedFingerprint #81230

Closed
Closed
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
58 changes: 0 additions & 58 deletions compiler/rustc_data_structures/src/fingerprint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -159,61 +159,3 @@ impl FingerprintDecoder for opaque::Decoder<'_> {
Fingerprint::decode_opaque(self)
}
}

// `PackedFingerprint` wraps a `Fingerprint`. Its purpose is to, on certain
// architectures, behave like a `Fingerprint` without alignment requirements.
// This behavior is only enabled on x86 and x86_64, where the impact of
// unaligned accesses is tolerable in small doses.
//
// This may be preferable to use in large collections of structs containing
// fingerprints, as it can reduce memory consumption by preventing the padding
// that the more strictly-aligned `Fingerprint` can introduce. An application of
// this is in the query dependency graph, which contains a large collection of
// `DepNode`s. As of this writing, the size of a `DepNode` decreases by ~30%
// (from 24 bytes to 17) by using the packed representation here, which
// noticeably decreases total memory usage when compiling large crates.
//
// The wrapped `Fingerprint` is private to reduce the chance of a client
// invoking undefined behavior by taking a reference to the packed field.
#[cfg_attr(any(target_arch = "x86", target_arch = "x86_64"), repr(packed))]
#[derive(Eq, PartialEq, Ord, PartialOrd, Debug, Clone, Copy, Hash)]
pub struct PackedFingerprint(Fingerprint);

impl std::fmt::Display for PackedFingerprint {
#[inline]
fn fmt(&self, formatter: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
// Copy to avoid taking reference to packed field.
let copy = self.0;
copy.fmt(formatter)
}
}

impl<E: rustc_serialize::Encoder> Encodable<E> for PackedFingerprint {
#[inline]
fn encode(&self, s: &mut E) -> Result<(), E::Error> {
// Copy to avoid taking reference to packed field.
let copy = self.0;
copy.encode(s)
}
}

impl<D: rustc_serialize::Decoder> Decodable<D> for PackedFingerprint {
#[inline]
fn decode(d: &mut D) -> Result<Self, D::Error> {
Fingerprint::decode(d).map(PackedFingerprint)
}
}

impl From<Fingerprint> for PackedFingerprint {
#[inline]
fn from(f: Fingerprint) -> PackedFingerprint {
PackedFingerprint(f)
}
}

impl From<PackedFingerprint> for Fingerprint {
#[inline]
fn from(f: PackedFingerprint) -> Fingerprint {
f.0
}
}
1 change: 0 additions & 1 deletion compiler/rustc_data_structures/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
#![feature(once_cell)]
#![feature(maybe_uninit_uninit_array)]
#![allow(rustc::default_hash_types)]
#![deny(unaligned_references)]

#[macro_use]
extern crate tracing;
Expand Down
11 changes: 2 additions & 9 deletions compiler/rustc_middle/src/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,10 +359,6 @@ pub type DepNode = rustc_query_system::dep_graph::DepNode<DepKind>;
// We keep a lot of `DepNode`s in memory during compilation. It's not
// required that their size stay the same, but we don't want to change
// it inadvertently. This assert just ensures we're aware of any change.
#[cfg(any(target_arch = "x86", target_arch = "x86_64"))]
static_assert_size!(DepNode, 17);

#[cfg(not(any(target_arch = "x86", target_arch = "x86_64")))]
static_assert_size!(DepNode, 24);

pub trait DepNodeExt: Sized {
Expand Down Expand Up @@ -396,7 +392,7 @@ impl DepNodeExt for DepNode {
/// single DefId/DefPathHash parameter.
fn from_def_path_hash(def_path_hash: DefPathHash, kind: DepKind) -> DepNode {
debug_assert!(kind.can_reconstruct_query_key() && kind.has_params);
DepNode { kind, hash: def_path_hash.0.into() }
DepNode { kind, hash: def_path_hash.0 }
}

/// Extracts the DefId corresponding to this DepNode. This will work
Expand All @@ -411,10 +407,7 @@ impl DepNodeExt for DepNode {
/// has been removed.
fn extract_def_id(&self, tcx: TyCtxt<'tcx>) -> Option<DefId> {
if self.kind.can_reconstruct_query_key() {
tcx.queries
.on_disk_cache
.as_ref()?
.def_path_hash_to_def_id(tcx, DefPathHash(self.hash.into()))
tcx.queries.on_disk_cache.as_ref()?.def_path_hash_to_def_id(tcx, DefPathHash(self.hash))
} else {
None
}
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_query_system/src/dep_graph/dep_node.rs
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@

use super::{DepContext, DepKind};

use rustc_data_structures::fingerprint::{Fingerprint, PackedFingerprint};
use rustc_data_structures::fingerprint::Fingerprint;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};

use std::fmt;
Expand All @@ -65,7 +65,7 @@ pub struct DepNode<K> {
//
// FIXME: Enforce this by preventing manual construction of `DefNode`
// (e.g. add a `_priv: ()` field)
pub hash: PackedFingerprint,
pub hash: Fingerprint,
}

impl<K: DepKind> DepNode<K> {
Expand All @@ -74,7 +74,7 @@ impl<K: DepKind> DepNode<K> {
/// does not require any parameters.
pub fn new_no_params(kind: K) -> DepNode<K> {
debug_assert!(!kind.has_params());
DepNode { kind, hash: Fingerprint::ZERO.into() }
DepNode { kind, hash: Fingerprint::ZERO }
}

pub fn construct<Ctxt, Key>(tcx: Ctxt, kind: K, arg: &Key) -> DepNode<K>
Expand All @@ -83,7 +83,7 @@ impl<K: DepKind> DepNode<K> {
Key: DepNodeParams<Ctxt>,
{
let hash = arg.to_fingerprint(tcx);
let dep_node = DepNode { kind, hash: hash.into() };
let dep_node = DepNode { kind, hash: hash };

#[cfg(debug_assertions)]
{
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_query_system/src/dep_graph/graph.rs
Original file line number Diff line number Diff line change
Expand Up @@ -390,7 +390,7 @@ impl<K: DepKind> DepGraph<K> {
// Fingerprint::combine() is faster than sending Fingerprint
// through the StableHasher (at least as long as StableHasher
// is so slow).
hash: data.current.anon_id_seed.combine(hasher.finish()).into(),
hash: data.current.anon_id_seed.combine(hasher.finish()),
};

let dep_node_index = data.current.intern_new_node(
Expand Down