Skip to content

Commit 24e25c6

Browse files
rylevrylev
authored
Merge pull request #1023 from rylev/filter-by-magnitude
Add ability to filter out 'very small' changes
2 parents 672da94 + 5a2ec92 commit 24e25c6

File tree

3 files changed

+99
-70
lines changed

3 files changed

+99
-70
lines changed

‎site/src/api.rs‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -201,6 +201,7 @@ pub mod comparison {
201201
pub is_significant: bool,
202202
pub significance_factor: Option<f64>,
203203
pub is_dodgy: bool,
204+
pub magnitude: String,
204205
pub historical_statistics: Option<Vec<f64>>,
205206
pub statistics: (f64, f64),
206207
}

‎site/src/comparison.rs‎

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,7 @@ pub async fn handle_compare(
115115
is_dodgy: comparison.is_dodgy(),
116116
is_significant: comparison.is_significant(),
117117
significance_factor: comparison.significance_factor(),
118+
magnitude: comparison.magnitude().display().to_owned(),
118119
historical_statistics: comparison.variance.map(|v| v.data),
119120
statistics: comparison.results,
120121
})
@@ -149,7 +150,8 @@ async fn populate_report(
149150
}
150151

151152
pub struct ComparisonSummary {
152-
/// Significant comparisons ordered by magnitude from largest to smallest
153+
/// Significant comparisons of magnitude small and above
154+
/// and ordered by magnitude from largest to smallest
153155
comparisons: Vec<TestResultComparison>,
154156
}
155157

@@ -159,6 +161,7 @@ impl ComparisonSummary {
159161
.statistics
160162
.iter()
161163
.filter(|c| c.is_significant())
164+
.filter(|c| c.magnitude().is_small_or_above())
162165
.cloned()
163166
.collect::<Vec<_>>();
164167
// Skip empty commits, sometimes happens if there's a compiler bug or so.
@@ -269,27 +272,22 @@ impl ComparisonSummary {
269272
}
270273

271274
pub fn confidence(&self) -> ComparisonConfidence {
272-
let mut num_very_small_changes = 0;
273275
let mut num_small_changes = 0;
274276
let mut num_medium_changes = 0;
275277
for c in self.comparisons.iter() {
276278
match c.magnitude() {
277-
Magnitude::VerySmall => num_very_small_changes += 1,
278279
Magnitude::Small => num_small_changes += 1,
279280
Magnitude::Medium => num_medium_changes += 1,
280281
Magnitude::Large => return ComparisonConfidence::DefinitelyRelevant,
281282
Magnitude::VeryLarge => return ComparisonConfidence::DefinitelyRelevant,
283+
Magnitude::VerySmall => unreachable!(),
282284
}
283285
}
284286

285-
match (
286-
num_very_small_changes,
287-
num_small_changes,
288-
num_medium_changes,
289-
) {
290-
(_, _, m) if m > 1 => ComparisonConfidence::DefinitelyRelevant,
291-
(_, _, m) if m > 0 => ComparisonConfidence::ProbablyRelevant,
292-
(vs, s, _) if (s * 2) + vs > 10 => ComparisonConfidence::ProbablyRelevant,
287+
match (num_small_changes, num_medium_changes) {
288+
(_, m) if m > 1 => ComparisonConfidence::DefinitelyRelevant,
289+
(_, 1) => ComparisonConfidence::ProbablyRelevant,
290+
(s, 0) if s > 10 => ComparisonConfidence::ProbablyRelevant,
293291
_ => ComparisonConfidence::MaybeRelevant,
294292
}
295293
}
@@ -866,9 +864,9 @@ impl TestResultComparison {
866864
Magnitude::VerySmall
867865
} else if change < threshold * 3.0 {
868866
Magnitude::Small
869-
} else if change < threshold * 10.0 {
867+
} else if change < threshold * 6.0 {
870868
Magnitude::Medium
871-
} else if change < threshold * 25.0 {
869+
} else if change < threshold * 18.0 {
872870
Magnitude::Large
873871
} else {
874872
Magnitude::VeryLarge
@@ -1002,6 +1000,10 @@ pub enum Magnitude {
10021000
}
10031001

10041002
impl Magnitude {
1003+
fn is_small_or_above(&self) -> bool {
1004+
*self >= Self::Small
1005+
}
1006+
10051007
fn is_medium_or_above(&self) -> bool {
10061008
*self >= Self::Medium
10071009
}

‎site/static/compare.html‎

Lines changed: 83 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -313,7 +313,7 @@ <h2>Comparing <span id="stat-header">{{stat}}</span> between <span id="before">{
313313
<ul id="states-list">
314314
<li>
315315
<label>
316-
<input type="checkbox" id="build-full" v-model="filter.cache.full" />
316+
<input type="checkbox" id="build-full" v-model="filter.scenario.full" />
317317
<span class="cache-label">full</span>
318318
</label>
319319
<div class="tooltip">?
@@ -324,7 +324,7 @@ <h2>Comparing <span id="stat-header">{{stat}}</span> between <span id="before">{
324324
</li>
325325
<li>
326326
<label>
327-
<input type="checkbox" id="build-incremental-full" v-model="filter.cache.incrFull" />
327+
<input type="checkbox" id="build-incremental-full" v-model="filter.scenario.incrFull" />
328328
<span class="cache-label">incr-full</span>
329329
</label>
330330
<div class="tooltip">?
@@ -336,7 +336,7 @@ <h2>Comparing <span id="stat-header">{{stat}}</span> between <span id="before">{
336336
<li>
337337
<label>
338338
<input type="checkbox" id="build-incremental-unchanged"
339-
v-model="filter.cache.incrUnchanged" />
339+
v-model="filter.scenario.incrUnchanged" />
340340
<span class="cache-label">incr-unchanged</span>
341341
</label>
342342
<div class="tooltip">?
@@ -349,7 +349,7 @@ <h2>Comparing <span id="stat-header">{{stat}}</span> between <span id="before">{
349349
<li>
350350
<label>
351351
<input type="checkbox" id="build-incremental-patched"
352-
v-model="filter.cache.incrPatched" />
352+
v-model="filter.scenario.incrPatched" />
353353
<span class="cache-label">incr-patched</span>
354354
</label>
355355
<div class="tooltip">?
@@ -374,6 +374,19 @@ <h2>Comparing <span id="stat-header">{{stat}}</span> between <span id="before">{
374374
</div>
375375
<input type="checkbox" v-model="filter.showOnlySignificant" style="margin-left: 20px;" />
376376
</div>
377+
<div class="section">
378+
<div class="section-heading"><span>Filter out very small changes</span>
379+
<span class="tooltip">?
380+
<span class="tooltiptext">
381+
Whether to filter out test cases that have a very small magnitude. Magnitude is
382+
calculated both on the absolute magnitude (i.e., how large of a percentage change)
383+
as well as the magnitude of the significance (i.e., by how many time the change was
384+
over the significance threshold).
385+
</span>
386+
</span>
387+
</div>
388+
<input type="checkbox" v-model="filter.filterVerySmall" style="margin-left: 20px;" />
389+
</div>
377390
</div>
378391
</fieldset>
379392
<div v-if="data" id="content" style="margin-top: 15px">
@@ -430,30 +443,31 @@ <h2>Comparing <span id="stat-header">{{stat}}</span> between <span id="before">{
430443
</tbody>
431444
<template v-for="bench in benches">
432445
<tbody>
433-
<template v-for="run in bench.variants">
446+
<template v-for="testCase in bench.testCases">
434447
<tr>
435-
<th v-if="run.first" v-bind:rowspan="bench.variants.length">{{bench.name}}</th>
436-
<td>{{ run.casename }}</td>
448+
<th v-if="testCase.first" v-bind:rowspan="bench.testCases.length">{{bench.name}}</th>
449+
<td>{{ testCase.scenario }}</td>
437450
<td>
438-
<a v-bind:href="detailedQueryLink(data.a.commit, bench.name, run.casename)">
439-
{{ run.datumA }}
451+
<a v-bind:href="detailedQueryLink(data.a.commit, bench.name, testCase.scenario)">
452+
{{ testCase.datumA }}
440453
</a>
441454
</td>
442455
<td>
443-
<a v-bind:href="detailedQueryLink(data.b.commit, bench.name, run.casename)">
444-
{{ run.datumB }}
456+
<a v-bind:href="detailedQueryLink(data.b.commit, bench.name, testCase.scenario)">
457+
{{ testCase.datumB }}
445458
</a>
446459
</td>
447460
<td>
448461
<a
449-
v-bind:href="percentLink(data.b.commit, data.a.commit, bench.name, run.casename)">
450-
<span v-bind:class="percentClass(run.percent)">
451-
{{ run.percent.toFixed(2) }}%{{run.isDodgy ? "?" : ""}}
462+
v-bind:href="percentLink(data.b.commit, data.a.commit, bench.name, testCase.scenario)">
463+
<span v-bind:class="percentClass(testCase.percent)">
464+
{{ testCase.percent.toFixed(2) }}%{{testCase.isDodgy ? "?" : ""}}
452465
</span>
453466
</a>
454467
</td>
455468
<td>
456-
{{ run.significance_factor ? run.significance_factor.toFixed(2) + "x" :"-" }}
469+
{{ testCase.significanceFactor ? testCase.significanceFactor.toFixed(2) + "x" :"-"
470+
}}
457471
</td>
458472
</tr>
459473
</template>
@@ -515,7 +529,8 @@ <h2>Comparing <span id="stat-header">{{stat}}</span> between <span id="before">{
515529
filter: {
516530
name: null,
517531
showOnlySignificant: true,
518-
cache: {
532+
filterVerySmall: true,
533+
scenario: {
519534
full: true,
520535
incrFull: true,
521536
incrUnchanged: true,
@@ -541,51 +556,62 @@ <h2>Comparing <span id="stat-header">{{stat}}</span> between <span id="before">{
541556
let data = this.data;
542557
const filter = this.filter;
543558

544-
function shouldShowBuild(name) {
545-
if (name === "full") {
546-
return filter.cache.full;
547-
} else if (name === "incr-full") {
548-
return filter.cache.incrFull;
549-
} else if (name === "incr-unchanged") {
550-
return filter.cache.incrUnchanged;
551-
} else if (name.startsWith("incr-patched")) {
552-
return filter.cache.incrPatched;
559+
function scenarioFilter(scenario) {
560+
if (scenario === "full") {
561+
return filter.scenario.full;
562+
} else if (scenario === "incr-full") {
563+
return filter.scenario.incrFull;
564+
} else if (scenario === "incr-unchanged") {
565+
return filter.scenario.incrUnchanged;
566+
} else if (scenario.startsWith("incr-patched")) {
567+
return filter.scenario.incrPatched;
553568
} else {
554569
// Unknown, but by default we should show things
555570
return true;
556571
}
557572
}
558-
function toVariants(results) {
559-
let variants = [];
573+
574+
function shouldShowTestCase(testCase) {
575+
let nameFilter = filter.name && filter.name.trim();
576+
nameFilter = !nameFilter || (testCase.benchmark + "-" + testCase.profile).includes(nameFilter);
577+
578+
const significanceFilter = filter.showOnlySignificant ? testCase.isSignificant : true;
579+
580+
const magnitudeFilter = filter.filterVerySmall ? testCase.magnitude != "very small" : true;
581+
582+
return scenarioFilter(testCase.scenario) && significanceFilter && nameFilter && magnitudeFilter;
583+
}
584+
585+
function toTestCases(results) {
586+
let testCases = [];
560587
for (let r of results) {
561-
const scenarioName = r.scenario;
588+
const scenario = r.scenario;
562589
const datumA = r.statistics[0];
563590
const datumB = r.statistics[1];
564591
const isSignificant = r.is_significant;
565-
let percent = (100 * (datumB - datumA) / datumA);
566-
let isDodgy = r.is_dodgy;
567-
if (shouldShowBuild(scenarioName)) {
568-
variants.push({
569-
casename: scenarioName,
570-
datumA,
571-
datumB,
572-
percent,
573-
isDodgy,
574-
significance_factor: r.significance_factor,
575-
isSignificant
576-
});
592+
const significanceFactor = r.significance_factor;
593+
const isDodgy = r.is_dodgy;
594+
let percent = 100 * ((datumB - datumA) / datumA);
595+
let testCase = {
596+
scenario,
597+
datumA,
598+
datumB,
599+
isSignificant,
600+
magnitude: r.magnitude,
601+
significanceFactor,
602+
isDodgy,
603+
percent,
604+
};
605+
if (shouldShowTestCase(testCase)) {
606+
testCases.push(testCase);
577607
}
578608
}
579609

580-
return variants;
610+
return testCases;
581611
}
582612

583613
let benches =
584614
data.comparisons.
585-
filter(n => {
586-
const f = filter.name && filter.name.trim();
587-
return !f || (n.benchmark + "-" + n.profile).includes(f);
588-
}).
589615
reduce((accum, next) => {
590616
const key = next.benchmark + "-" + next.profile;
591617
if (!accum[key]) {
@@ -598,22 +624,22 @@ <h2>Comparing <span id="stat-header">{{stat}}</span> between <span id="before">{
598624
map(c => {
599625
const name = c[0];
600626
const comparison = c[1];
601-
const variants = toVariants(comparison).filter(v => filter.showOnlySignificant ? v.isSignificant : true);
602-
const pcts = variants.map(field => parseFloat(field.percent));
627+
const testCases = toTestCases(comparison);
628+
const pcts = testCases.map(tc => parseFloat(tc.percent));
603629
const maxPct = Math.max(...pcts).toFixed(1);
604630
const minPct = Math.min(...pcts).toFixed(1);
605-
if (variants.length > 0) {
606-
variants[0].first = true;
631+
if (testCases.length > 0) {
632+
testCases[0].first = true;
607633
}
608634

609635
return {
610636
name,
611-
variants,
637+
testCases,
612638
maxPct,
613639
minPct,
614640
};
615641
}).
616-
filter(b => b.variants.length > 0);
642+
filter(b => b.testCases.length > 0);
617643

618644
const largestChange = a => Math.max(Math.abs(a.minPct), Math.abs(a.maxPct));
619645
// Sort by name first, so that there is a canonical ordering
@@ -679,7 +705,7 @@ <h2>Comparing <span id="stat-header">{{stat}}</span> between <span id="before">{
679705
},
680706
summary() {
681707
// Create object with each test case that is not filtered out as a key
682-
const filtered = Object.fromEntries(this.benches.flatMap(b => b.variants.map(v => [b.name + "-" + v.casename, true])));
708+
const filtered = Object.fromEntries(this.benches.flatMap(b => b.testCases.map(v => [b.name + "-" + v.scenario, true])));
683709
const newCount = { regressions: 0, improvements: 0, unchanged: 0 }
684710
let result = { all: { ...newCount }, filtered: { ...newCount } }
685711
for (let d of this.data.comparisons) {
@@ -734,11 +760,11 @@ <h2>Comparing <span id="stat-header">{{stat}}</span> between <span id="before">{
734760
return klass;
735761

736762
},
737-
detailedQueryLink(commit, bench, run) {
738-
return `/detailed-query.html?commit=${commit}&benchmark=${bench}&run_name=${run}`;
763+
detailedQueryLink(commit, bench, testCase) {
764+
return `/detailed-query.html?commit=${commit}&benchmark=${bench}&run_name=${testCase}`;
739765
},
740-
percentLink(commit, baseCommit, bench, run) {
741-
return `/detailed-query.html?commit=${commit}&base_commit=${baseCommit}&benchmark=${bench}&run_name=${run}`;
766+
percentLink(commit, baseCommit, bench, testCase) {
767+
return `/detailed-query.html?commit=${commit}&base_commit=${baseCommit}&benchmark=${bench}&run_name=${testCase}`;
742768
},
743769
commitLink(commit) {
744770
return `https://github.com/rust-lang/rust/commit/${commit}`;
@@ -823,4 +849,4 @@ <h2>Comparing <span id="stat-header">{{stat}}</span> between <span id="before">{
823849
</script>
824850
</body>
825851

826-
</html>
852+
</html>

0 commit comments

Comments
 (0)