Skip to content

Commit 98e218f

Browse files
authored
Merge pull request #1489 from rylev/adjust-historical-analysis
Handle Noise Better
2 parents 2ffd7a5 + 73ea936 commit 98e218f

File tree

3 files changed

+22
-11
lines changed

3 files changed

+22
-11
lines changed

site/src/api.rs

+1
Original file line numberDiff line numberDiff line change
@@ -194,6 +194,7 @@ pub mod comparison {
194194
pub profile: String,
195195
pub scenario: String,
196196
pub is_relevant: bool,
197+
pub significance_threshold: f64,
197198
pub significance_factor: f64,
198199
pub statistics: (f64, f64),
199200
}

site/src/comparison.rs

+4-4
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,7 @@ pub async fn handle_compare(
135135
profile: comparison.profile.to_string(),
136136
scenario: comparison.scenario.to_string(),
137137
is_relevant: comparison.is_relevant(),
138+
significance_threshold: comparison.significance_threshold(),
138139
significance_factor: comparison.significance_factor(),
139140
statistics: comparison.results,
140141
})
@@ -1015,8 +1016,7 @@ pub struct HistoricalDataMap {
10151016
}
10161017

10171018
impl HistoricalDataMap {
1018-
const NUM_PREVIOUS_COMMITS: usize = 100;
1019-
const MIN_PREVIOUS_COMMITS: usize = 50;
1019+
const NUM_PREVIOUS_COMMITS: usize = 30;
10201020

10211021
async fn calculate(
10221022
ctxt: &SiteCtxt,
@@ -1033,7 +1033,7 @@ impl HistoricalDataMap {
10331033
));
10341034

10351035
// Return early if we don't have enough data for historical analysis
1036-
if previous_commits.len() < Self::MIN_PREVIOUS_COMMITS {
1036+
if previous_commits.len() < Self::NUM_PREVIOUS_COMMITS {
10371037
return Ok(Self {
10381038
data: historical_data,
10391039
});
@@ -1057,7 +1057,7 @@ impl HistoricalDataMap {
10571057
}
10581058

10591059
// Only retain test cases for which we have enough data to calculate variance.
1060-
historical_data.retain(|_, v| v.data.len() >= Self::MIN_PREVIOUS_COMMITS);
1060+
historical_data.retain(|_, v| v.data.len() >= Self::NUM_PREVIOUS_COMMITS);
10611061

10621062
Ok(Self {
10631063
data: historical_data,

site/static/compare/script.js

+17-7
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,7 @@ const app = Vue.createApp({
134134
category: (benchmarkMap[c.benchmark] || {}).category || "secondary",
135135
isRelevant: c.is_relevant,
136136
significanceFactor: c.significance_factor,
137+
significanceThreshold: (c.significance_threshold * 100.0), // ensure the threshold is in %
137138
datumA,
138139
datumB,
139140
percent,
@@ -152,7 +153,7 @@ const app = Vue.createApp({
152153
bootstrapTotals() {
153154
const a = this.data.a.bootstrap_total / 1e9;
154155
const b = this.data.b.bootstrap_total / 1e9;
155-
return {a, b};
156+
return { a, b };
156157
},
157158
bootstraps() {
158159
return Object.entries(this.data.a.bootstrap).map(e => {
@@ -355,14 +356,20 @@ app.component('test-cases-table', {
355356
<th>Profile</th>
356357
<th>Scenario</th>
357358
<th>% Change</th>
359+
<th>
360+
Significance Threshold<span class="tooltip">?
361+
<span class="tooltiptext">
362+
The minimum % change that is considered significant. The higher the significance threshold, the noisier a test case is.
363+
You can see <a href="https://github.com/rust-lang/rustc-perf/blob/master/docs/comparison-analysis.md#what-makes-a-test-result-significant">
364+
here</a> how the significance threshold is calculated.
365+
</span>
366+
</span>
367+
</th>
358368
<th>
359369
Significance Factor<span class="tooltip">?
360370
<span class="tooltiptext">
361371
How much a particular result is over the significance threshold. A factor of 2.50x
362-
means
363-
the result is 2.5 times over the significance threshold. You can see <a
364-
href="https://github.com/rust-lang/rustc-perf/blob/master/docs/comparison-analysis.md#what-makes-a-test-result-significant">
365-
here</a> how the significance threshold is calculated.
372+
means the result is 2.5 times over the significance threshold.
366373
</span>
367374
</span>
368375
</th>
@@ -393,6 +400,9 @@ app.component('test-cases-table', {
393400
</span>
394401
</a>
395402
</td>
403+
<td>
404+
{{ testCase.significanceThreshold ? testCase.significanceThreshold.toFixed(2) + "%" : "-" }}
405+
</td>
396406
<td>
397407
{{ testCase.significanceFactor ? testCase.significanceFactor.toFixed(2) + "x" : "-" }}
398408
</td>
@@ -447,7 +457,7 @@ const SummaryRange = {
447457
[<SummaryPercentValue :value="range[0]" :padWidth="6" />, <SummaryPercentValue :value="range[1]" :padWidth="6" />]
448458
</div>
449459
<div v-else style="text-align: center;">-</div>
450-
`, components: {SummaryPercentValue}
460+
`, components: { SummaryPercentValue }
451461
};
452462
const SummaryCount = {
453463
props: {
@@ -510,7 +520,7 @@ app.component('summary-table', {
510520
</tbody>
511521
</table>
512522
`,
513-
components: {SummaryRange, SummaryPercentValue, SummaryCount}
523+
components: { SummaryRange, SummaryPercentValue, SummaryCount }
514524
});
515525

516526
app.component("aggregations", {

0 commit comments

Comments
 (0)