Skip to content

Commit 3319a51

Browse files
committed
Optimize save_report_lines a bit
This avoids some intermediate allocations in particular for a `flag_map` case, as well as avoiding another allocation. In other cases its unfortunately not possible to avoid more intermediate allocations.
1 parent 54cbaa6 commit 3319a51

File tree

5 files changed

+41
-39
lines changed

5 files changed

+41
-39
lines changed

core/src/parsers/pyreport/utils.rs

Lines changed: 20 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ fn create_model_sets_for_line_session<R: Report, B: ReportBuilder<R>>(
7878
coverage_type: &models::CoverageType,
7979
line_no: i64,
8080
datapoint: Option<&CoverageDatapoint>,
81-
ctx: &mut ParseCtx<R, B>,
81+
ctx: &ParseCtx<R, B>,
8282
) -> LineSessionModels {
8383
let source_file_id = ctx.report_json_files[&ctx.chunk.index];
8484
let (hits, hit_branches, total_branches) = separate_pyreport_coverage(&line_session.coverage);
@@ -184,32 +184,30 @@ fn create_model_sets_for_line_session<R: Report, B: ReportBuilder<R>>(
184184
}
185185
}
186186

187-
fn create_model_sets_for_report_line<R: Report, B: ReportBuilder<R>>(
188-
report_line: &ReportLine,
189-
ctx: &mut ParseCtx<R, B>,
190-
) -> Vec<LineSessionModels> {
187+
fn create_model_sets_for_report_line<'a, R: Report, B: ReportBuilder<R>>(
188+
report_line: &'a ReportLine,
189+
ctx: &'a ParseCtx<R, B>,
190+
) -> impl Iterator<Item = LineSessionModels> + 'a {
191191
// A `ReportLine` is a collection of `LineSession`s, and each `LineSession` has
192192
// a set of models we need to insert for it. Build a list of those sets of
193193
// models.
194-
let mut line_session_models = vec![];
195-
for line_session in &report_line.sessions {
194+
report_line.sessions.iter().map(|session| {
196195
// Datapoints are effectively `LineSession`-scoped, but they don't actually live
197196
// in the `LineSession`. Get the `CoverageDatapoint` for this
198197
// `LineSession` if there is one.
199198
let datapoint = if let Some(Some(datapoints)) = &report_line.datapoints {
200-
datapoints.get(&(line_session.session_id as u32))
199+
datapoints.get(&(session.session_id as u32))
201200
} else {
202201
None
203202
};
204-
line_session_models.push(create_model_sets_for_line_session(
205-
line_session,
203+
create_model_sets_for_line_session(
204+
session,
206205
&report_line.coverage_type,
207206
report_line.line_no,
208207
datapoint,
209208
ctx,
210-
));
211-
}
212-
line_session_models
209+
)
210+
})
213211
}
214212

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

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

924-
let model_sets = create_model_sets_for_report_line(&report_line, parse_ctx);
919+
let model_sets: Vec<_> =
920+
create_model_sets_for_report_line(&report_line, parse_ctx).collect();
925921
assert_eq!(
926922
model_sets,
927-
vec![
923+
&[
928924
LineSessionModels {
929925
sample: models::CoverageSample {
930926
raw_upload_id: 123,
@@ -1018,10 +1014,11 @@ mod tests {
10181014
datapoints: Some(Some(datapoints)),
10191015
};
10201016

1021-
let model_sets = create_model_sets_for_report_line(&report_line, parse_ctx);
1017+
let model_sets: Vec<_> =
1018+
create_model_sets_for_report_line(&report_line, parse_ctx).collect();
10221019
assert_eq!(
10231020
model_sets,
1024-
vec![
1021+
&[
10251022
LineSessionModels {
10261023
sample: models::CoverageSample {
10271024
raw_upload_id: 123,

core/src/report/mod.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,9 @@ pub trait ReportBuilder<R: Report> {
6363
/// passed-in models' `local_sample_id` fields are ignored and overwritten
6464
/// with values that are unique among all `CoverageSample`s with the same
6565
/// `raw_upload_id`.
66-
fn multi_insert_coverage_sample(
66+
fn multi_insert_coverage_sample<'a>(
6767
&mut self,
68-
samples: Vec<&mut models::CoverageSample>,
68+
samples: impl ExactSizeIterator<Item = &'a mut models::CoverageSample>,
6969
) -> Result<()>;
7070

7171
/// Create a [`models::BranchesData`] record and return it. The passed-in

core/src/report/sqlite/models.rs

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,9 +102,11 @@ pub trait Insertable {
102102
Ok(())
103103
}
104104

105-
fn multi_insert<'a, I>(mut models: I, conn: &rusqlite::Connection) -> Result<()>
105+
fn multi_insert<'a>(
106+
mut models: impl ExactSizeIterator<Item = &'a Self>,
107+
conn: &rusqlite::Connection,
108+
) -> Result<()>
106109
where
107-
I: Iterator<Item = &'a Self> + ExactSizeIterator,
108110
Self: 'a,
109111
{
110112
let chunk_size = Self::maximum_chunk_size(conn);

core/src/report/sqlite/report_builder.rs

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -95,9 +95,9 @@ impl ReportBuilder<SqliteReport> for SqliteReportBuilder {
9595
self.transaction()?.insert_coverage_sample(sample)
9696
}
9797

98-
fn multi_insert_coverage_sample(
98+
fn multi_insert_coverage_sample<'a>(
9999
&mut self,
100-
samples: Vec<&mut models::CoverageSample>,
100+
samples: impl ExactSizeIterator<Item = &'a mut models::CoverageSample>,
101101
) -> Result<()> {
102102
self.transaction()?.multi_insert_coverage_sample(samples)
103103
}
@@ -237,14 +237,17 @@ impl ReportBuilder<SqliteReport> for SqliteReportBuilderTx<'_> {
237237
Ok(sample)
238238
}
239239

240-
fn multi_insert_coverage_sample(
240+
fn multi_insert_coverage_sample<'a>(
241241
&mut self,
242-
mut samples: Vec<&mut models::CoverageSample>,
242+
samples: impl ExactSizeIterator<Item = &'a mut models::CoverageSample>,
243243
) -> Result<()> {
244-
for sample in &mut samples {
245-
sample.local_sample_id = self.id_sequence.next().unwrap();
246-
}
247-
models::CoverageSample::multi_insert(samples.iter().map(|v| &**v), &self.conn)?;
244+
models::CoverageSample::multi_insert(
245+
samples.map(|sample| {
246+
sample.local_sample_id = self.id_sequence.next().unwrap();
247+
&*sample
248+
}),
249+
&self.conn,
250+
)?;
248251
Ok(())
249252
}
250253

@@ -452,7 +455,7 @@ mod tests {
452455
.insert_raw_upload(Default::default())
453456
.unwrap();
454457

455-
let mut samples: Vec<models::CoverageSample> = vec![
458+
let mut samples = vec![
456459
models::CoverageSample {
457460
source_file_id: file.id,
458461
raw_upload_id: raw_upload.id,
@@ -461,7 +464,7 @@ mod tests {
461464
5
462465
];
463466
report_builder
464-
.multi_insert_coverage_sample(samples.iter_mut().collect())
467+
.multi_insert_coverage_sample(samples.iter_mut())
465468
.unwrap();
466469

467470
let report = report_builder.build().unwrap();

core/src/test_utils/test_report.rs

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -93,9 +93,9 @@ impl ReportBuilder<TestReport> for TestReportBuilder {
9393
Ok(sample)
9494
}
9595

96-
fn multi_insert_coverage_sample(
96+
fn multi_insert_coverage_sample<'a>(
9797
&mut self,
98-
samples: Vec<&mut CoverageSample>,
98+
samples: impl ExactSizeIterator<Item = &'a mut CoverageSample>,
9999
) -> error::Result<()> {
100100
self.report
101101
.samples

0 commit comments

Comments
 (0)