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

Optimize save_report_lines a bit #62

Open
wants to merge 1 commit into
base: main
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
43 changes: 20 additions & 23 deletions core/src/parsers/pyreport/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ fn create_model_sets_for_line_session<R: Report, B: ReportBuilder<R>>(
coverage_type: &models::CoverageType,
line_no: i64,
datapoint: Option<&CoverageDatapoint>,
ctx: &mut ParseCtx<R, B>,
ctx: &ParseCtx<R, B>,
) -> LineSessionModels {
let source_file_id = ctx.report_json_files[&ctx.chunk.index];
let (hits, hit_branches, total_branches) = separate_pyreport_coverage(&line_session.coverage);
Expand Down Expand Up @@ -184,32 +184,30 @@ fn create_model_sets_for_line_session<R: Report, B: ReportBuilder<R>>(
}
}

fn create_model_sets_for_report_line<R: Report, B: ReportBuilder<R>>(
report_line: &ReportLine,
ctx: &mut ParseCtx<R, B>,
) -> Vec<LineSessionModels> {
fn create_model_sets_for_report_line<'a, R: Report, B: ReportBuilder<R>>(
report_line: &'a ReportLine,
ctx: &'a ParseCtx<R, B>,
) -> impl Iterator<Item = LineSessionModels> + 'a {
// A `ReportLine` is a collection of `LineSession`s, and each `LineSession` has
// a set of models we need to insert for it. Build a list of those sets of
// models.
let mut line_session_models = vec![];
for line_session in &report_line.sessions {
report_line.sessions.iter().map(|session| {
// Datapoints are effectively `LineSession`-scoped, but they don't actually live
// in the `LineSession`. Get the `CoverageDatapoint` for this
// `LineSession` if there is one.
let datapoint = if let Some(Some(datapoints)) = &report_line.datapoints {
datapoints.get(&(line_session.session_id as u32))
datapoints.get(&(session.session_id as u32))
} else {
None
};
line_session_models.push(create_model_sets_for_line_session(
line_session,
create_model_sets_for_line_session(
session,
&report_line.coverage_type,
report_line.line_no,
datapoint,
ctx,
));
}
line_session_models
)
})
}

/// Each [`ReportLine`] from a chunks file is comprised of a number of
Expand All @@ -231,12 +229,9 @@ pub fn save_report_lines<R: Report, B: ReportBuilder<R>>(
// assigned as a side-effect of this insertion. That lets us populate the
// `local_sample_id` foreign key on all of the models associated with each
// `CoverageSample`.
ctx.db.report_builder.multi_insert_coverage_sample(
models
.iter_mut()
.map(|LineSessionModels { sample, .. }| sample)
.collect(),
)?;
ctx.db
.report_builder
.multi_insert_coverage_sample(models.iter_mut().map(|m| &mut m.sample))?;

// Populate `local_sample_id` and insert all of the context assocs for each
// `LineSession` (if there are any)
Expand Down Expand Up @@ -921,10 +916,11 @@ mod tests {
datapoints: None,
};

let model_sets = create_model_sets_for_report_line(&report_line, parse_ctx);
let model_sets: Vec<_> =
create_model_sets_for_report_line(&report_line, parse_ctx).collect();
assert_eq!(
model_sets,
vec![
&[
LineSessionModels {
sample: models::CoverageSample {
raw_upload_id: 123,
Expand Down Expand Up @@ -1018,10 +1014,11 @@ mod tests {
datapoints: Some(Some(datapoints)),
};

let model_sets = create_model_sets_for_report_line(&report_line, parse_ctx);
let model_sets: Vec<_> =
create_model_sets_for_report_line(&report_line, parse_ctx).collect();
assert_eq!(
model_sets,
vec![
&[
LineSessionModels {
sample: models::CoverageSample {
raw_upload_id: 123,
Expand Down
4 changes: 2 additions & 2 deletions core/src/report/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,9 +63,9 @@ pub trait ReportBuilder<R: Report> {
/// passed-in models' `local_sample_id` fields are ignored and overwritten
/// with values that are unique among all `CoverageSample`s with the same
/// `raw_upload_id`.
fn multi_insert_coverage_sample(
fn multi_insert_coverage_sample<'a>(
&mut self,
samples: Vec<&mut models::CoverageSample>,
samples: impl ExactSizeIterator<Item = &'a mut models::CoverageSample>,
) -> Result<()>;

/// Create a [`models::BranchesData`] record and return it. The passed-in
Expand Down
6 changes: 4 additions & 2 deletions core/src/report/sqlite/models.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,11 @@ pub trait Insertable {
Ok(())
}

fn multi_insert<'a, I>(mut models: I, conn: &rusqlite::Connection) -> Result<()>
fn multi_insert<'a>(
mut models: impl ExactSizeIterator<Item = &'a Self>,
conn: &rusqlite::Connection,
) -> Result<()>
where
I: Iterator<Item = &'a Self> + ExactSizeIterator,
Self: 'a,
{
let chunk_size = Self::maximum_chunk_size(conn);
Expand Down
23 changes: 13 additions & 10 deletions core/src/report/sqlite/report_builder.rs
Original file line number Diff line number Diff line change
Expand Up @@ -95,9 +95,9 @@ impl ReportBuilder<SqliteReport> for SqliteReportBuilder {
self.transaction()?.insert_coverage_sample(sample)
}

fn multi_insert_coverage_sample(
fn multi_insert_coverage_sample<'a>(
&mut self,
samples: Vec<&mut models::CoverageSample>,
samples: impl ExactSizeIterator<Item = &'a mut models::CoverageSample>,
) -> Result<()> {
self.transaction()?.multi_insert_coverage_sample(samples)
}
Expand Down Expand Up @@ -237,14 +237,17 @@ impl ReportBuilder<SqliteReport> for SqliteReportBuilderTx<'_> {
Ok(sample)
}

fn multi_insert_coverage_sample(
fn multi_insert_coverage_sample<'a>(
&mut self,
mut samples: Vec<&mut models::CoverageSample>,
samples: impl ExactSizeIterator<Item = &'a mut models::CoverageSample>,
) -> Result<()> {
for sample in &mut samples {
sample.local_sample_id = self.id_sequence.next().unwrap();
}
models::CoverageSample::multi_insert(samples.iter().map(|v| &**v), &self.conn)?;
models::CoverageSample::multi_insert(
samples.map(|sample| {
sample.local_sample_id = self.id_sequence.next().unwrap();
&*sample
}),
&self.conn,
)?;
Ok(())
}

Expand Down Expand Up @@ -452,7 +455,7 @@ mod tests {
.insert_raw_upload(Default::default())
.unwrap();

let mut samples: Vec<models::CoverageSample> = vec![
let mut samples = vec![
models::CoverageSample {
source_file_id: file.id,
raw_upload_id: raw_upload.id,
Expand All @@ -461,7 +464,7 @@ mod tests {
5
];
report_builder
.multi_insert_coverage_sample(samples.iter_mut().collect())
.multi_insert_coverage_sample(samples.iter_mut())
.unwrap();

let report = report_builder.build().unwrap();
Expand Down
4 changes: 2 additions & 2 deletions core/src/test_utils/test_report.rs
Original file line number Diff line number Diff line change
Expand Up @@ -93,9 +93,9 @@ impl ReportBuilder<TestReport> for TestReportBuilder {
Ok(sample)
}

fn multi_insert_coverage_sample(
fn multi_insert_coverage_sample<'a>(
&mut self,
samples: Vec<&mut CoverageSample>,
samples: impl ExactSizeIterator<Item = &'a mut CoverageSample>,
) -> error::Result<()> {
self.report
.samples
Expand Down
Loading