Skip to content

Commit 6535898

Browse files
committed
Bug 1932617 - Move test_get_num_recorded_errors to a trait and use trait objects r=TravisLong
This reduces the size of the generated function (with huge match statements) significantly. On a local (optimized) Linux build `event_test_get_error` went from 260573 byte to 46245. `event_test_get_value_wrapper` from 49506 to 45559. This is based on a change in the Glean SDK. Differential Revision: https://phabricator.services.mozilla.com/D266375
1 parent fd03e35 commit 6535898

File tree

5 files changed

+63
-28
lines changed

5 files changed

+63
-28
lines changed

toolkit/components/glean/api/src/ffi/macros.rs

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -199,6 +199,12 @@ macro_rules! test_get_errors {
199199
glean::ErrorType::InvalidOverflow,
200200
];
201201
let mut error_str = None;
202+
// Only `events` use this trait right now
203+
// Ignoring the unused import simplifies the macro right now.
204+
// This might change in the future.
205+
#[allow(unused_imports)]
206+
use crate::private::TestGetNumErrors;
207+
202208
for &error_type in error_types.iter() {
203209
let num_errors = $metric.test_get_num_recorded_errors(error_type);
204210
if num_errors > 0 {

toolkit/components/glean/api/src/private/event.rs

Lines changed: 12 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -165,6 +165,18 @@ impl<K: 'static + ExtraKeys + Send + Sync + Clone> EventMetric<K> {
165165
}
166166
}
167167

168+
impl<K> crate::private::TestGetNumErrors for EventMetric<K> {
169+
fn test_get_num_recorded_errors(&self, error_type: glean::ErrorType) -> i32 {
170+
match self {
171+
EventMetric::Parent { inner, .. } => inner.test_get_num_recorded_errors(error_type),
172+
EventMetric::Child(c) => panic!(
173+
"Cannot get the number of recorded errors for {:?} in non-main process!",
174+
c.id
175+
),
176+
}
177+
}
178+
}
179+
168180
#[inherent]
169181
impl<K: 'static + ExtraKeys + Send + Sync + Clone> Event for EventMetric<K> {
170182
type Extra = K;
@@ -199,16 +211,6 @@ impl<K: 'static + ExtraKeys + Send + Sync + Clone> Event for EventMetric<K> {
199211
}
200212
}
201213
}
202-
203-
pub fn test_get_num_recorded_errors(&self, error: glean::ErrorType) -> i32 {
204-
match self {
205-
EventMetric::Parent { inner, .. } => inner.test_get_num_recorded_errors(error),
206-
EventMetric::Child(meta) => panic!(
207-
"Cannot get the number of recorded errors for {:?} in non-main process!",
208-
meta.id
209-
),
210-
}
211-
}
212214
}
213215

214216
#[inherent]

toolkit/components/glean/api/src/private/mod.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -677,6 +677,21 @@ fn truncate_string_for_marker_to_length(mut input: String, byte_length: usize) -
677677
input
678678
}
679679

680+
pub trait TestGetNumErrors {
681+
/// **Exported for test purposes.**
682+
///
683+
/// Gets the number of recorded errors for the given metric and error type.
684+
///
685+
/// # Arguments
686+
///
687+
/// * `error` - The type of error
688+
///
689+
/// # Returns
690+
///
691+
/// The number of errors reported.
692+
fn test_get_num_recorded_errors(&self, error_type: glean::ErrorType) -> i32;
693+
}
694+
680695
#[cfg(test)]
681696
mod truncation_tests {
682697
use crate::private::truncate_string_for_marker;

toolkit/components/glean/build_scripts/glean_parser_ext/templates/rust.jinja2

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -432,12 +432,14 @@ pub(crate) mod __glean_metric_maps {
432432
///
433433
/// Panics if no event by the given metric ID could be found.
434434
pub(crate) fn event_test_get_value_wrapper(metric_id: u32, ping_name: Option<String>) -> Option<Vec<RecordedEvent>> {
435-
match metric_id {
435+
let event = match metric_id {
436436
{% for metric_id, event in events_by_id.items() %}
437-
{{metric_id}} => super::{{event}}.test_get_value(ping_name),
437+
{{metric_id}} => &*super::{{event}} as &dyn ::glean::TestGetValue<Output=Vec<RecordedEvent>>,
438438
{% endfor %}
439439
_ => panic!("No event for metric id {}", metric_id),
440-
}
440+
};
441+
442+
return event.test_get_value(ping_name);
441443
}
442444

443445
/// Check the provided event for errors.
@@ -458,11 +460,15 @@ pub(crate) mod __glean_metric_maps {
458460
#[allow(unused_variables)]
459461
pub(crate) fn event_test_get_error(metric_id: u32) -> Option<String> {
460462
#[cfg(feature = "with_gecko")]
461-
match metric_id {
463+
{
464+
let event = match metric_id {
462465
{% for metric_id, event in events_by_id.items() %}
463-
{{metric_id}} => test_get_errors!(super::{{event}}),
466+
{{metric_id}} => &*super::{{event}} as &dyn crate::private::TestGetNumErrors,
464467
{% endfor %}
465-
_ => panic!("No event for metric id {}", metric_id),
468+
_ => panic!("No event for metric id {}", metric_id),
469+
};
470+
471+
return test_get_errors!(event);
466472
}
467473

468474
#[cfg(not(feature = "with_gecko"))]

toolkit/components/glean/tests/pytest/metrics_test_output

Lines changed: 18 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1863,13 +1863,15 @@ pub(crate) mod __glean_metric_maps {
18631863
///
18641864
/// Panics if no event by the given metric ID could be found.
18651865
pub(crate) fn event_test_get_value_wrapper(metric_id: u32, ping_name: Option<String>) -> Option<Vec<RecordedEvent>> {
1866-
match metric_id {
1867-
21 => super::test_nested::event_metric.test_get_value(ping_name),
1868-
22 => super::test_nested::event_metric_with_extra.test_get_value(ping_name),
1869-
49 => super::test2_nested::event_metric.test_get_value(ping_name),
1870-
50 => super::test2_nested::event_metric_with_extra.test_get_value(ping_name),
1866+
let event = match metric_id {
1867+
21 => &*super::test_nested::event_metric as &dyn ::glean::TestGetValue<Output=Vec<RecordedEvent>>,
1868+
22 => &*super::test_nested::event_metric_with_extra as &dyn ::glean::TestGetValue<Output=Vec<RecordedEvent>>,
1869+
49 => &*super::test2_nested::event_metric as &dyn ::glean::TestGetValue<Output=Vec<RecordedEvent>>,
1870+
50 => &*super::test2_nested::event_metric_with_extra as &dyn ::glean::TestGetValue<Output=Vec<RecordedEvent>>,
18711871
_ => panic!("No event for metric id {}", metric_id),
1872-
}
1872+
};
1873+
1874+
return event.test_get_value(ping_name);
18731875
}
18741876

18751877
/// Check the provided event for errors.
@@ -1890,12 +1892,16 @@ pub(crate) mod __glean_metric_maps {
18901892
#[allow(unused_variables)]
18911893
pub(crate) fn event_test_get_error(metric_id: u32) -> Option<String> {
18921894
#[cfg(feature = "with_gecko")]
1893-
match metric_id {
1894-
21 => test_get_errors!(super::test_nested::event_metric),
1895-
22 => test_get_errors!(super::test_nested::event_metric_with_extra),
1896-
49 => test_get_errors!(super::test2_nested::event_metric),
1897-
50 => test_get_errors!(super::test2_nested::event_metric_with_extra),
1898-
_ => panic!("No event for metric id {}", metric_id),
1895+
{
1896+
let event = match metric_id {
1897+
21 => &*super::test_nested::event_metric as &dyn crate::private::TestGetNumErrors,
1898+
22 => &*super::test_nested::event_metric_with_extra as &dyn crate::private::TestGetNumErrors,
1899+
49 => &*super::test2_nested::event_metric as &dyn crate::private::TestGetNumErrors,
1900+
50 => &*super::test2_nested::event_metric_with_extra as &dyn crate::private::TestGetNumErrors,
1901+
_ => panic!("No event for metric id {}", metric_id),
1902+
};
1903+
1904+
return test_get_errors!(event);
18991905
}
19001906

19011907
#[cfg(not(feature = "with_gecko"))]

0 commit comments

Comments
 (0)