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

Track -Cprofile-use and -Cprofile-sample-use value by file hash, not file path #100413

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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 Cargo.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4032,6 +4032,7 @@ dependencies = [
"rustc_ty_utils",
"rustc_typeck",
"smallvec",
"tempfile",
"tracing",
"winapi",
]
Expand Down
2 changes: 1 addition & 1 deletion compiler/rustc_codegen_ssa/src/back/linker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -304,7 +304,7 @@ impl<'a> GccLinker<'a> {
};

if let Some(path) = &self.sess.opts.unstable_opts.profile_sample_use {
self.linker_arg(&format!("-plugin-opt=sample-profile={}", path.display()));
self.linker_arg(&format!("-plugin-opt=sample-profile={}", path.as_path().display()));
};
self.linker_args(&[
&format!("-plugin-opt={}", opt_level),
Expand Down
7 changes: 5 additions & 2 deletions compiler/rustc_codegen_ssa/src/back/write.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,8 +180,11 @@ impl ModuleConfig {
sess.opts.cg.profile_generate.clone(),
SwitchWithOptPath::Disabled
),
pgo_use: if_regular!(sess.opts.cg.profile_use.clone(), None),
pgo_sample_use: if_regular!(sess.opts.unstable_opts.profile_sample_use.clone(), None),
pgo_use: if_regular!(sess.opts.cg.profile_use.clone().map(|p| p.into()), None),
pgo_sample_use: if_regular!(
sess.opts.unstable_opts.profile_sample_use.clone().map(|p| p.into()),
None
),
debug_info_for_profiling: sess.opts.unstable_opts.debug_info_for_profiling,
instrument_coverage: if_regular!(sess.instrument_coverage(), false),
instrument_gcov: if_regular!(
Expand Down
1 change: 1 addition & 0 deletions compiler/rustc_interface/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ winapi = { version = "0.3", features = ["libloaderapi"] }

[dev-dependencies]
rustc_target = { path = "../rustc_target" }
tempfile = "3.2"

[features]
llvm = ['rustc_codegen_llvm']
Expand Down
39 changes: 36 additions & 3 deletions compiler/rustc_interface/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use rustc_session::config::{
use rustc_session::config::{CFGuard, ExternEntry, LinkerPluginLto, LtoCli, SwitchWithOptPath};
use rustc_session::lint::Level;
use rustc_session::search_paths::SearchPath;
use rustc_session::utils::{CanonicalizedPath, NativeLib, NativeLibKind};
use rustc_session::utils::{CanonicalizedPath, ContentHashedFilePath, NativeLib, NativeLibKind};
use rustc_session::{build_session, getopts, DiagnosticOutput, Session};
use rustc_span::edition::{Edition, DEFAULT_EDITION};
use rustc_span::symbol::sym;
Expand Down Expand Up @@ -594,7 +594,6 @@ fn test_codegen_options_tracking_hash() {
tracked!(passes, vec![String::from("1"), String::from("2")]);
tracked!(prefer_dynamic, true);
tracked!(profile_generate, SwitchWithOptPath::Enabled(None));
tracked!(profile_use, Some(PathBuf::from("abc")));
tracked!(relocation_model, Some(RelocModel::Pic));
tracked!(soft_float, true);
tracked!(split_debuginfo, Some(SplitDebuginfo::Packed));
Expand Down Expand Up @@ -777,7 +776,6 @@ fn test_unstable_options_tracking_hash() {
tracked!(profile, true);
tracked!(profile_emit, Some(PathBuf::from("abc")));
tracked!(profiler_runtime, "abc".to_string());
tracked!(profile_sample_use, Some(PathBuf::from("abc")));
tracked!(relax_elf_relocations, Some(true));
tracked!(relro_level, Some(RelroLevel::Full));
tracked!(remap_cwd_prefix, Some(PathBuf::from("abc")));
Expand Down Expand Up @@ -818,6 +816,41 @@ fn test_unstable_options_tracking_hash() {
tracked_no_crate_hash!(no_codegen, true);
}

#[test]
fn test_hashed_file_different_hash() {
let tempdir = tempfile::TempDir::new().unwrap();

let mut options = Options::default();

macro_rules! check {
($opt: expr, $file: expr) => {
let path = tempdir.path().join($file);

// Write some data into the file
std::fs::write(&path, &[1, 2, 3]).unwrap();

// The hash is calculated now
*$opt = Some(ContentHashedFilePath::new(path.clone()));

let hash_no_crate = options.dep_tracking_hash(false);
let hash_crate = options.dep_tracking_hash(true);

// Write different data into the file
std::fs::write(&path, &[1, 2, 3, 4]).unwrap();

// The hash is re-calculated now
*$opt = Some(ContentHashedFilePath::new(path));

// Check that the hash has changed
assert_ne!(options.dep_tracking_hash(true), hash_crate);
assert_ne!(options.dep_tracking_hash(false), hash_no_crate);
};
}

check!(&mut options.cg.profile_use, "profile-instr.profdata");
check!(&mut options.unstable_opts.profile_sample_use, "profile-sample.profdata");
}

#[test]
fn test_edition_parsing() {
// test default edition
Expand Down
20 changes: 17 additions & 3 deletions compiler/rustc_session/src/options.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use crate::config::*;
use crate::early_error;
use crate::lint;
use crate::search_paths::SearchPath;
use crate::utils::NativeLib;
use crate::utils::{ContentHashedFilePath, NativeLib};
use rustc_errors::LanguageIdentifier;
use rustc_target::spec::{CodeModel, LinkerFlavor, MergeFunctions, PanicStrategy, SanitizerSet};
use rustc_target::spec::{
Expand Down Expand Up @@ -364,6 +364,7 @@ mod desc {
pub const parse_string_push: &str = parse_string;
pub const parse_opt_langid: &str = "a language identifier";
pub const parse_opt_pathbuf: &str = "a path";
pub const parse_opt_hashed_filepath: &str = "a path";
pub const parse_list: &str = "a space-separated list of strings";
pub const parse_list_with_polarity: &str =
"a comma-separated list of strings, with elements beginning with + or -";
Expand Down Expand Up @@ -509,6 +510,19 @@ mod parse {
}
}

pub(crate) fn parse_opt_hashed_filepath(
slot: &mut Option<ContentHashedFilePath>,
v: Option<&str>,
) -> bool {
match v {
Some(s) => {
*slot = Some(ContentHashedFilePath::new(PathBuf::from(s)));
true
}
None => false,
}
}

pub(crate) fn parse_string_push(slot: &mut Vec<String>, v: Option<&str>) -> bool {
match v {
Some(s) => {
Expand Down Expand Up @@ -1176,7 +1190,7 @@ options! {
profile_generate: SwitchWithOptPath = (SwitchWithOptPath::Disabled,
parse_switch_with_opt_path, [TRACKED],
"compile the program with profiling instrumentation"),
profile_use: Option<PathBuf> = (None, parse_opt_pathbuf, [TRACKED],
profile_use: Option<ContentHashedFilePath> = (None, parse_opt_hashed_filepath, [TRACKED],
"use the given `.profdata` file for profile-guided optimization"),
#[rustc_lint_opt_deny_field_access("use `Session::relocation_model` instead of this field")]
relocation_model: Option<RelocModel> = (None, parse_relocation_model, [TRACKED],
Expand Down Expand Up @@ -1488,7 +1502,7 @@ options! {
(default based on relative source path)"),
profiler_runtime: String = (String::from("profiler_builtins"), parse_string, [TRACKED],
"name of the profiler runtime crate to automatically inject (default: `profiler_builtins`)"),
profile_sample_use: Option<PathBuf> = (None, parse_opt_pathbuf, [TRACKED],
profile_sample_use: Option<ContentHashedFilePath> = (None, parse_opt_hashed_filepath, [TRACKED],
"use the given `.prof` file for sampled profile-guided optimization (also known as AutoFDO)"),
query_dep_graph: bool = (false, parse_bool, [UNTRACKED],
"enable queries of the dependency graph for regression testing (default: no)"),
Expand Down
8 changes: 4 additions & 4 deletions compiler/rustc_session/src/session.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1459,20 +1459,20 @@ fn validate_commandline_args_with_session_available(sess: &Session) {
// Make sure that any given profiling data actually exists so LLVM can't
// decide to silently skip PGO.
if let Some(ref path) = sess.opts.cg.profile_use {
if !path.exists() {
if !path.as_path().exists() {
sess.err(&format!(
"File `{}` passed to `-C profile-use` does not exist.",
path.display()
path.as_path().display()
));
}
}

// Do the same for sample profile data.
if let Some(ref path) = sess.opts.unstable_opts.profile_sample_use {
if !path.exists() {
if !path.as_path().exists() {
sess.err(&format!(
"File `{}` passed to `-C profile-sample-use` does not exist.",
path.display()
path.as_path().display()
));
}
}
Expand Down
62 changes: 62 additions & 0 deletions compiler/rustc_session/src/utils.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
use crate::config::dep_tracking::DepTrackingHash;
use crate::config::ErrorOutputType;
use crate::session::Session;
use rustc_data_structures::profiling::VerboseTimingGuard;
use rustc_data_structures::stable_hasher::{HashStable, StableHasher};
use std::collections::hash_map::DefaultHasher;
use std::fs::File;
use std::io::Read;
use std::path::{Path, PathBuf};

impl Session {
Expand Down Expand Up @@ -91,3 +97,59 @@ impl CanonicalizedPath {
&self.original
}
}

/// A path that should be invalidated when the file that it points to has changed.
/// `ContentHashedFilePath` is identified by its contents only, so even if the filepath itself
/// changes, but the contents stay the same, it will contain the same hash.
#[derive(Clone, Debug)]
pub struct ContentHashedFilePath {
path: PathBuf,
hash: (u64, u64),
}

impl ContentHashedFilePath {
pub fn new(path: PathBuf) -> Self {
// If the file does not exist or couldn't be hashed, just use a placeholder hash
let hash = hash_file(&path).unwrap_or((0, 0));
Self { path, hash }
}

pub fn as_path(&self) -> &Path {
self.path.as_path()
}
}

impl From<ContentHashedFilePath> for PathBuf {
fn from(path: ContentHashedFilePath) -> Self {
path.path
}
}

impl DepTrackingHash for ContentHashedFilePath {
fn hash(
&self,
hasher: &mut DefaultHasher,
_error_format: ErrorOutputType,
_for_crate_hash: bool,
) {
std::hash::Hash::hash(&self.hash, hasher);
}
}

fn hash_file(path: &Path) -> std::io::Result<(u64, u64)> {
let mut hasher = StableHasher::new();

let mut file = File::open(path)?;
let mut buffer = [0; 128 * 1024];

loop {
let count = file.read(&mut buffer)?;
if count == 0 {
break;
}

buffer[..count].hash_stable(&mut (), &mut hasher);
}

Ok(hasher.finalize())
}