Skip to content

Keep usage of aid and cid straight #900

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

Merged
merged 1 commit into from
Jul 9, 2021
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
33 changes: 19 additions & 14 deletions collector/src/execute.rs
Original file line number Diff line number Diff line change
Expand Up @@ -557,10 +557,10 @@ pub struct MeasureProcessor<'a> {
krate: &'a BenchmarkName,
conn: &'a mut dyn database::Connection,
artifact: &'a database::ArtifactId,
cid: database::ArtifactIdNumber,
artifact_row_id: database::ArtifactIdNumber,
upload: Option<Upload>,
is_first_collection: bool,
self_profile: bool,
is_self_profile: bool,
tries: u8,
}

Expand All @@ -570,8 +570,8 @@ impl<'a> MeasureProcessor<'a> {
conn: &'a mut dyn database::Connection,
krate: &'a BenchmarkName,
artifact: &'a database::ArtifactId,
cid: database::ArtifactIdNumber,
self_profile: bool,
artifact_row_id: database::ArtifactIdNumber,
is_self_profile: bool,
) -> Self {
// Check we have `perf` or (`xperf.exe` and `tracelog.exe`) available.
if cfg!(unix) {
Expand All @@ -596,10 +596,9 @@ impl<'a> MeasureProcessor<'a> {
conn,
krate,
artifact,
cid,
artifact_row_id,
is_first_collection: true,
// Command::new("summarize").status().is_ok()
self_profile,
is_self_profile,
tries: 0,
}
}
Expand Down Expand Up @@ -644,14 +643,14 @@ impl<'a> MeasureProcessor<'a> {
u.wait();
}
let prefix = PathBuf::from("self-profile")
.join(self.cid.0.to_string())
.join(self.artifact_row_id.0.to_string())
.join(self.krate.0.as_str())
.join(profile.to_string())
.join(cache.to_id());
self.upload = Some(Upload::new(prefix, collection, files));
self.rt.block_on(self.conn.record_raw_self_profile(
collection,
self.cid,
self.artifact_row_id,
self.krate.0.as_str(),
profile,
cache,
Expand All @@ -663,7 +662,7 @@ impl<'a> MeasureProcessor<'a> {
for (stat, value) in stats.0.iter() {
buf.push(self.conn.record_statistic(
collection,
self.cid,
self.artifact_row_id,
self.krate.0.as_str(),
profile,
cache,
Expand All @@ -674,12 +673,12 @@ impl<'a> MeasureProcessor<'a> {

if let Some(sp) = &stats.1 {
let conn = &*self.conn;
let cid = self.cid;
let artifact_row_id = self.artifact_row_id;
let krate = self.krate.0.as_str();
for qd in &sp.query_data {
buf.push(conn.record_self_profile_query(
collection,
cid,
artifact_row_id,
krate,
profile,
cache,
Expand Down Expand Up @@ -788,7 +787,7 @@ impl Upload {

impl<'a> Processor for MeasureProcessor<'a> {
fn profiler(&self, _build: BuildKind) -> Profiler {
if self.is_first_collection && self.self_profile {
if self.is_first_collection && self.is_self_profile {
if cfg!(unix) {
Profiler::PerfStatSelfProfile
} else {
Expand Down Expand Up @@ -865,7 +864,13 @@ impl<'a> Processor for MeasureProcessor<'a> {
}

fn measure_rustc(&mut self, compiler: Compiler<'_>) -> anyhow::Result<()> {
rustc::measure(self.rt, self.conn, compiler, self.artifact, self.cid)
rustc::measure(
self.rt,
self.conn,
compiler,
self.artifact,
self.artifact_row_id,
)
}
}

Expand Down
43 changes: 23 additions & 20 deletions collector/src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -186,20 +186,23 @@ impl BenchmarkErrors {
fn bench(
rt: &mut Runtime,
pool: database::Pool,
cid: &ArtifactId,
artifact_id: &ArtifactId,
build_kinds: &[BuildKind],
run_kinds: &[RunKind],
compiler: Compiler<'_>,
benchmarks: &[Benchmark],
iterations: Option<usize>,
self_profile: bool,
is_self_profile: bool,
) -> BenchmarkErrors {
let mut conn = rt.block_on(pool.connection());
let mut errors = BenchmarkErrors::new();
eprintln!("Benchmarking {} for triple {}", cid, compiler.triple);
eprintln!(
"Benchmarking {} for triple {}",
artifact_id, compiler.triple
);

let has_measureme = Command::new("summarize").output().is_ok();
if self_profile {
if is_self_profile {
assert!(
has_measureme,
"needs `summarize` in PATH for self profile.\n\
Expand All @@ -214,20 +217,20 @@ fn bench(

// Make sure there is no observable time when the artifact ID is available
// but the in-progress steps are not.
let interned_cid = {
let artifact_row_id = {
let mut tx = rt.block_on(conn.transaction());
let interned_cid = rt.block_on(tx.conn().artifact_id(&cid));
rt.block_on(tx.conn().collector_start(interned_cid, &steps));
let artifact_row_id = rt.block_on(tx.conn().artifact_id(&artifact_id));
rt.block_on(tx.conn().collector_start(artifact_row_id, &steps));

rt.block_on(tx.commit()).unwrap();
interned_cid
artifact_row_id
};

let start = Instant::now();
let mut skipped = false;
for (nth_benchmark, benchmark) in benchmarks.iter().enumerate() {
let is_fresh =
rt.block_on(conn.collector_start_step(interned_cid, &benchmark.name.to_string()));
rt.block_on(conn.collector_start_step(artifact_row_id, &benchmark.name.to_string()));
if !is_fresh {
skipped = true;
eprintln!("skipping {} -- already benchmarked", benchmark.name);
Expand All @@ -247,9 +250,9 @@ fn bench(
rt,
tx.conn(),
&benchmark.name,
&cid,
interned_cid,
self_profile,
&artifact_id,
artifact_row_id,
is_self_profile,
);
let result =
benchmark.measure(&mut processor, build_kinds, run_kinds, compiler, iterations);
Expand All @@ -260,14 +263,14 @@ fn bench(
);
errors.incr();
rt.block_on(tx.conn().record_error(
interned_cid,
artifact_row_id,
benchmark.name.0.as_str(),
&format!("{:?}", s),
));
};
rt.block_on(
tx.conn()
.collector_end_step(interned_cid, &benchmark.name.to_string()),
.collector_end_step(artifact_row_id, &benchmark.name.to_string()),
);
rt.block_on(tx.commit()).expect("committed");
}
Expand All @@ -281,7 +284,7 @@ fn bench(
if skipped {
log::info!("skipping duration record -- skipped parts of run");
} else {
rt.block_on(conn.record_duration(interned_cid, end));
rt.block_on(conn.record_duration(artifact_row_id, end));
}

rt.block_on(async move {
Expand Down Expand Up @@ -574,7 +577,7 @@ fn main_result() -> anyhow::Result<i32> {
let include = sub_m.value_of("INCLUDE");
let run_kinds = run_kinds_from_arg(&sub_m.value_of("RUNS"))?;
let rustdoc = sub_m.value_of("RUSTDOC");
let self_profile = sub_m.is_present("SELF_PROFILE");
let is_self_profile = sub_m.is_present("SELF_PROFILE");

let pool = database::Pool::open(db);

Expand All @@ -597,7 +600,7 @@ fn main_result() -> anyhow::Result<i32> {
},
&benchmarks,
Some(1),
self_profile,
is_self_profile,
);
res.fail_if_nonzero()?;
Ok(0)
Expand All @@ -609,7 +612,7 @@ fn main_result() -> anyhow::Result<i32> {

// Options
let db = sub_m.value_of("DB").unwrap_or(default_db);
let self_profile = sub_m.is_present("SELF_PROFILE");
let is_self_profile = sub_m.is_present("SELF_PROFILE");

println!("processing commits");
let client = reqwest::blocking::Client::new();
Expand Down Expand Up @@ -646,7 +649,7 @@ fn main_result() -> anyhow::Result<i32> {
Compiler::from_sysroot(&sysroot),
&benchmarks,
next.runs.map(|v| v as usize),
self_profile,
is_self_profile,
);

client.post(&format!("{}/perf/onpush", site_url)).send()?;
Expand Down Expand Up @@ -722,7 +725,7 @@ fn main_result() -> anyhow::Result<i32> {
},
&benchmarks,
Some(3),
/* self_profile */ false,
/* is_self_profile */ false,
);
res.fail_if_nonzero()?;
Ok(0)
Expand Down
39 changes: 28 additions & 11 deletions database/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -417,13 +417,21 @@ impl From<Commit> for ArtifactId {

#[async_trait::async_trait]
pub trait SeriesType: Sized {
async fn get(conn: &dyn pool::Connection, series: u32, cid: ArtifactIdNumber) -> Option<Self>;
async fn get(
conn: &dyn pool::Connection,
series: u32,
artifact_row_id: ArtifactIdNumber,
) -> Option<Self>;
}

#[async_trait::async_trait]
impl SeriesType for f64 {
async fn get(conn: &dyn pool::Connection, series: u32, cid: ArtifactIdNumber) -> Option<Self> {
conn.get_pstats(&[series], &[Some(cid)]).await[0][0]
async fn get(
conn: &dyn pool::Connection,
series: u32,
artifact_row_id: ArtifactIdNumber,
) -> Option<Self> {
conn.get_pstats(&[series], &[Some(artifact_row_id)]).await[0][0]
}
}

Expand All @@ -440,13 +448,18 @@ pub struct QueryDatum {

#[async_trait::async_trait]
impl SeriesType for QueryDatum {
async fn get(conn: &dyn pool::Connection, series: u32, cid: ArtifactIdNumber) -> Option<Self> {
conn.get_self_profile_query(series, cid).await
async fn get(
conn: &dyn pool::Connection,
series: u32,
artifact_row_id: ArtifactIdNumber,
) -> Option<Self> {
conn.get_self_profile_query(series, artifact_row_id).await
}
}
#[derive(Hash, Copy, Clone, Debug, PartialEq, Eq, PartialOrd, Ord)]
pub struct LabelId(pub u8, pub u32);

/// A database row ID for an artifact in the artifact table
#[derive(Serialize, Deserialize, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Hash, Debug)]
pub struct ArtifactIdNumber(pub u32);

Expand Down Expand Up @@ -638,20 +651,24 @@ impl Index {
conn.load_index().await
}

pub fn lookup(&self, path: &DbLabel, cid: &ArtifactId) -> Option<(u32, ArtifactIdNumber)> {
let cid = cid.lookup(self)?;
pub fn lookup(
&self,
path: &DbLabel,
artifact_id: &ArtifactId,
) -> Option<(u32, ArtifactIdNumber)> {
let artifact_row_id = artifact_id.lookup(self)?;
let series = path.lookup(self)?;
Some((series, cid))
Some((series, artifact_row_id))
}

pub async fn get<T: SeriesType>(
&self,
db: &mut dyn pool::Connection,
path: &DbLabel,
cid: &ArtifactId,
artifact_id: &ArtifactId,
) -> Option<T> {
let (series, cid) = self.lookup(path, cid)?;
T::get(db, series, cid).await
let (series, artifact_row_id) = self.lookup(path, artifact_id)?;
T::get(db, series, artifact_row_id).await
}

pub fn artifacts(&self) -> impl Iterator<Item = &'_ str> + '_ {
Expand Down
9 changes: 5 additions & 4 deletions database/src/pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,21 +75,22 @@ pub trait Connection: Send + Sync {
async fn get_pstats(
&self,
series: &[u32],
cid: &[Option<ArtifactIdNumber>],
artifact_row_id: &[Option<ArtifactIdNumber>],
) -> Vec<Vec<Option<f64>>>;
async fn get_self_profile(
&self,
cid: ArtifactIdNumber,
artifact_row_id: ArtifactIdNumber,
crate_: &str,
profile: &str,
cache: &str,
) -> HashMap<crate::QueryLabel, QueryDatum>;
async fn get_self_profile_query(
&self,
series: u32,
cid: ArtifactIdNumber,
artifact_row_id: ArtifactIdNumber,
) -> Option<QueryDatum>;
async fn get_error(&self, cid: ArtifactIdNumber) -> HashMap<String, Option<String>>;
async fn get_error(&self, artifact_row_id: ArtifactIdNumber)
-> HashMap<String, Option<String>>;

async fn queue_pr(
&self,
Expand Down
Loading