Skip to content

Commit ad00a78

Browse files
epompeiiepompeii
committed
short_units
1 parent 6d5d8c2 commit ad00a78

File tree

3 files changed

+85
-15
lines changed

3 files changed

+85
-15
lines changed

‎lib/bencher_comment/src/lib.rs‎

Lines changed: 43 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -293,6 +293,7 @@ impl ReportComment {
293293
alert.metric.value,
294294
alert.boundary.baseline,
295295
factor,
296+
&units,
296297
true,
297298
);
298299
if self.has_lower_boundary_alert() {
@@ -301,6 +302,7 @@ impl ReportComment {
301302
alert.metric.value,
302303
alert.boundary.lower_limit,
303304
factor,
305+
&units,
304306
alert.limit == BoundaryLimit::Lower,
305307
);
306308
}
@@ -310,6 +312,7 @@ impl ReportComment {
310312
alert.metric.value,
311313
alert.boundary.upper_limit,
312314
factor,
315+
&units,
313316
alert.limit == BoundaryLimit::Upper,
314317
);
315318
}
@@ -438,8 +441,10 @@ impl ReportComment {
438441
name = result.benchmark.name,
439442
));
440443
for (measure, boundary_limits) in mbl {
441-
let factor =
442-
Units::new(boundary_limits.min.into(), measure.units.clone()).scale_factor();
444+
let (factor, units) = {
445+
let units = Units::new(boundary_limits.min.into(), measure.units.clone());
446+
(units.scale_factor(), units.scale_units())
447+
};
443448

444449
let report_measure = result
445450
.measures
@@ -464,6 +469,7 @@ impl ReportComment {
464469
report_measure.metric.value,
465470
report_measure.boundary.and_then(|b| b.baseline),
466471
factor,
472+
&units,
467473
alert.is_some(),
468474
);
469475
} else {
@@ -476,6 +482,7 @@ impl ReportComment {
476482
report_measure.metric.value,
477483
report_measure.boundary.and_then(|b| b.lower_limit),
478484
factor,
485+
&units,
479486
alert.is_some_and(|a| a.limit == BoundaryLimit::Lower),
480487
);
481488
} else {
@@ -489,6 +496,7 @@ impl ReportComment {
489496
report_measure.metric.value,
490497
report_measure.boundary.and_then(|b| b.upper_limit),
491498
factor,
499+
&units,
492500
alert.is_some_and(|a| a.limit == BoundaryLimit::Upper),
493501
);
494502
} else {
@@ -794,14 +802,20 @@ fn value_cell(
794802
value: OrderedFloat<f64>,
795803
baseline: Option<OrderedFloat<f64>>,
796804
factor: OrderedFloat<f64>,
805+
units: &str,
797806
bold: bool,
798807
) {
799808
fn value_cell_inner(
800809
value: OrderedFloat<f64>,
801810
baseline: Option<OrderedFloat<f64>>,
802811
factor: OrderedFloat<f64>,
812+
units: &str,
803813
) -> String {
804814
let mut cell = Units::format_float((value / factor).into());
815+
let short_units = short_units(units);
816+
if let Some(short_units) = &short_units {
817+
cell.push_str(short_units);
818+
}
805819

806820
if let Some(baseline) = baseline {
807821
let percent = if value.is_normal() && baseline.is_normal() {
@@ -818,6 +832,9 @@ fn value_cell(
818832
cell.push_str(&format!("({plus}{percent}%)"));
819833
cell.push_str("</summary>");
820834
cell.push_str(&format!("Baseline: {baseline}"));
835+
if let Some(short_units) = &short_units {
836+
cell.push_str(short_units);
837+
}
821838
cell.push_str("</details>");
822839
}
823840

@@ -828,10 +845,10 @@ fn value_cell(
828845
if bold {
829846
html.push_str(&format!(
830847
"<b>{}</b>",
831-
value_cell_inner(value, baseline, factor)
848+
value_cell_inner(value, baseline, factor, units)
832849
));
833850
} else {
834-
html.push_str(&value_cell_inner(value, baseline, factor));
851+
html.push_str(&value_cell_inner(value, baseline, factor, units));
835852
}
836853
html.push_str("</td>");
837854
}
@@ -841,6 +858,7 @@ fn lower_limit_cell(
841858
value: OrderedFloat<f64>,
842859
lower_limit: Option<OrderedFloat<f64>>,
843860
factor: OrderedFloat<f64>,
861+
units: &str,
844862
bold: bool,
845863
) {
846864
let Some(limit) = lower_limit else {
@@ -854,14 +872,15 @@ fn lower_limit_cell(
854872
0.0.into()
855873
};
856874

857-
limit_cell(html, limit, percent, factor, bold);
875+
limit_cell(html, limit, percent, factor, units, bold);
858876
}
859877

860878
fn upper_limit_cell(
861879
html: &mut String,
862880
value: OrderedFloat<f64>,
863881
upper_limit: Option<OrderedFloat<f64>>,
864882
factor: OrderedFloat<f64>,
883+
units: &str,
865884
bold: bool,
866885
) {
867886
let Some(limit) = upper_limit else {
@@ -875,39 +894,54 @@ fn upper_limit_cell(
875894
0.0.into()
876895
};
877896

878-
limit_cell(html, limit, percent, factor, bold);
897+
limit_cell(html, limit, percent, factor, units, bold);
879898
}
880899

881900
fn limit_cell(
882901
html: &mut String,
883902
limit: OrderedFloat<f64>,
884903
percent: OrderedFloat<f64>,
885904
factor: OrderedFloat<f64>,
905+
units: &str,
886906
bold: bool,
887907
) {
888908
fn limit_cell_inner(
889909
limit: OrderedFloat<f64>,
890910
percent: OrderedFloat<f64>,
891911
factor: OrderedFloat<f64>,
912+
units: &str,
892913
) -> String {
893914
let mut cell = Units::format_float((limit / factor).into());
915+
if let Some(short_units) = short_units(units) {
916+
cell.push_str(&short_units);
917+
}
894918
let percent = Units::format_float(percent.into());
895919
cell.push_str(&format!("<br />({percent}%)"));
896920
cell
897921
}
898922

899923
html.push_str("<td>");
900924
if bold {
925+
// The two extra line breaks are here to make the text line up
926+
// with the value cell on GitHub,
927+
// where the row cells are vertically aligned to each other.
901928
html.push_str(&format!(
902-
"<b>{}</b>",
903-
limit_cell_inner(limit, percent, factor)
929+
"<b>{}<br /><br /></b>",
930+
limit_cell_inner(limit, percent, factor, units)
904931
));
905932
} else {
906-
html.push_str(&limit_cell_inner(limit, percent, factor));
933+
html.push_str(&limit_cell_inner(limit, percent, factor, units));
907934
}
908935
html.push_str("</td>");
909936
}
910937

938+
fn short_units(units: &str) -> Option<String> {
939+
units
940+
.split_once('(')
941+
.and_then(|(_, delimited)| delimited.split_once(')'))
942+
.map(|(units, _)| format!(" {units}"))
943+
}
944+
911945
#[derive(Clone, Copy)]
912946
pub struct BoundaryLimits {
913947
min: OrderedFloat<f64>,

‎services/console/src/chunks/docs-reference/changelog/en/changelog.mdx‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
## Pending `v0.4.37`
22
- Add dimension search in Threshold creation UI
33
- Add Threshold list to Branch, Testbed, and Measure pages in Console UI
4+
- Add unit abbreviation to Perf Report tables in Console UI and PR comments (Thank you [@BD103](https://github.com/BD103))
45

56
## `v0.4.36`
67
- Fix CORS failures when API server returns an error

‎services/console/src/components/console/deck/hand/card/ReportCard.tsx‎

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -248,13 +248,15 @@ const ReportCard = (props: Props) => {
248248
value={value}
249249
baseline={baseline}
250250
factor={factor}
251+
units={units}
251252
bold
252253
/>
253254
<Show when={hasLowerBoundaryAlert()}>
254255
<LowerLimitCell
255256
value={value}
256257
lowerLimit={lowerLimit}
257258
factor={factor}
259+
units={units}
258260
bold={alert?.limit === BoundaryLimit.Lower}
259261
/>
260262
</Show>
@@ -263,6 +265,7 @@ const ReportCard = (props: Props) => {
263265
value={value}
264266
upperLimit={upperLimit}
265267
factor={factor}
268+
units={units}
266269
bold={alert?.limit === BoundaryLimit.Upper}
267270
/>
268271
</Show>
@@ -378,6 +381,10 @@ const ReportCard = (props: Props) => {
378381
boundaryLimits.min,
379382
measure.units,
380383
);
384+
const units = scale_units(
385+
boundaryLimits.min,
386+
measure.units,
387+
);
381388

382389
const reportMeasure = result.measures.find(
383390
(report_measure) =>
@@ -482,6 +489,7 @@ const ReportCard = (props: Props) => {
482489
value={value as number}
483490
baseline={baseline}
484491
factor={factor}
492+
units={units}
485493
bold={!!alert}
486494
/>
487495
</Show>
@@ -490,6 +498,7 @@ const ReportCard = (props: Props) => {
490498
value={value as number}
491499
lowerLimit={lowerLimit}
492500
factor={factor}
501+
units={units}
493502
bold={
494503
alert?.limit === BoundaryLimit.Lower
495504
}
@@ -500,6 +509,7 @@ const ReportCard = (props: Props) => {
500509
value={value as number}
501510
upperLimit={upperLimit}
502511
factor={factor}
512+
units={units}
503513
bold={
504514
alert?.limit === BoundaryLimit.Upper
505515
}
@@ -528,12 +538,14 @@ const ValueCell = (props: {
528538
value: number;
529539
baseline: null | undefined | number;
530540
factor: number;
541+
units: string;
531542
bold: boolean;
532543
}) => {
533544
const ValueCellInner = (props: {
534545
value: number;
535546
baseline: null | undefined | number;
536547
factor: number;
548+
units: string;
537549
}) => {
538550
if (typeof props.value !== "number") {
539551
return <></>;
@@ -546,9 +558,12 @@ const ValueCell = (props: {
546558
: 0.0
547559
: null;
548560

561+
const short_units = shortUnits(props.units);
562+
549563
return (
550564
<>
551565
{prettyPrintFloat(props.value / props.factor)}
566+
{short_units}
552567
<Show when={percent !== null}>
553568
<br />
554569
<details>
@@ -558,6 +573,7 @@ const ValueCell = (props: {
558573
</summary>
559574
Baseline:{" "}
560575
{prettyPrintFloat((props.baseline as number) / props.factor)}
576+
{short_units}
561577
</details>
562578
</Show>
563579
</>
@@ -573,6 +589,7 @@ const ValueCell = (props: {
573589
value={props.value}
574590
baseline={props.baseline}
575591
factor={props.factor}
592+
units={props.units}
576593
/>
577594
}
578595
>
@@ -581,6 +598,7 @@ const ValueCell = (props: {
581598
value={props.value}
582599
baseline={props.baseline}
583600
factor={props.factor}
601+
units={props.units}
584602
/>
585603
</b>
586604
</Show>
@@ -592,6 +610,7 @@ const LowerLimitCell = (props: {
592610
value: number;
593611
lowerLimit: null | undefined | number;
594612
factor: number;
613+
units: string;
595614
bold: boolean;
596615
}) => {
597616
if (
@@ -612,6 +631,7 @@ const LowerLimitCell = (props: {
612631
limit={props.lowerLimit}
613632
percent={percent}
614633
factor={props.factor}
634+
units={props.units}
615635
bold={props.bold}
616636
/>
617637
);
@@ -621,6 +641,7 @@ const UpperLimitCell = (props: {
621641
value: number;
622642
upperLimit: null | undefined | number;
623643
factor: number;
644+
units: string;
624645
bold: boolean;
625646
}) => {
626647
if (
@@ -641,6 +662,7 @@ const UpperLimitCell = (props: {
641662
limit={props.upperLimit}
642663
percent={percent}
643664
factor={props.factor}
665+
units={props.units}
644666
bold={props.bold}
645667
/>
646668
);
@@ -650,18 +672,24 @@ const LimitCell = (props: {
650672
limit: number;
651673
percent: number;
652674
factor: number;
675+
units: string;
653676
bold: boolean;
654677
}) => {
655678
const LimitCellInner = (props: {
656679
limit: number;
657680
percent: number;
658681
factor: number;
659-
}) => (
660-
<>
661-
{prettyPrintFloat(props.limit / props.factor)}
662-
<br />({prettyPrintFloat(props.percent)}%)
663-
</>
664-
);
682+
units: string;
683+
}) => {
684+
const short_units = shortUnits(props.units);
685+
return (
686+
<>
687+
{prettyPrintFloat(props.limit / props.factor)}
688+
{short_units}
689+
<br />({prettyPrintFloat(props.percent)}%)
690+
</>
691+
);
692+
};
665693

666694
return (
667695
<td>
@@ -672,6 +700,7 @@ const LimitCell = (props: {
672700
limit={props.limit}
673701
percent={props.percent}
674702
factor={props.factor}
703+
units={props.units}
675704
/>
676705
}
677706
>
@@ -680,13 +709,19 @@ const LimitCell = (props: {
680709
limit={props.limit}
681710
percent={props.percent}
682711
factor={props.factor}
712+
units={props.units}
683713
/>
684714
</b>
685715
</Show>
686716
</td>
687717
);
688718
};
689719

720+
const shortUnits = (units: string): string => {
721+
const match = units.match(/\(([^)]+)\)/)?.[1];
722+
return match ? ` ${match}` : "";
723+
};
724+
690725
// 30 days
691726
const DEFAULT_ALERT_HISTORY = 30 * 24 * 60 * 60 * 1000;
692727

0 commit comments

Comments
 (0)