commit 65358985333570a909cfaaeadfc96d98c6f3a532
parent fd03e3542b783b553ab1246d24e937a02da2f296
Author: Jan-Erik Rediger <jrediger@mozilla.com>
Date: Fri, 24 Oct 2025 12:39:29 +0000
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
Diffstat:
5 files changed, 63 insertions(+), 28 deletions(-)
diff --git a/toolkit/components/glean/api/src/ffi/macros.rs b/toolkit/components/glean/api/src/ffi/macros.rs
@@ -199,6 +199,12 @@ macro_rules! test_get_errors {
glean::ErrorType::InvalidOverflow,
];
let mut error_str = None;
+ // Only `events` use this trait right now
+ // Ignoring the unused import simplifies the macro right now.
+ // This might change in the future.
+ #[allow(unused_imports)]
+ use crate::private::TestGetNumErrors;
+
for &error_type in error_types.iter() {
let num_errors = $metric.test_get_num_recorded_errors(error_type);
if num_errors > 0 {
diff --git a/toolkit/components/glean/api/src/private/event.rs b/toolkit/components/glean/api/src/private/event.rs
@@ -165,6 +165,18 @@ impl<K: 'static + ExtraKeys + Send + Sync + Clone> EventMetric<K> {
}
}
+impl<K> crate::private::TestGetNumErrors for EventMetric<K> {
+ fn test_get_num_recorded_errors(&self, error_type: glean::ErrorType) -> i32 {
+ match self {
+ EventMetric::Parent { inner, .. } => inner.test_get_num_recorded_errors(error_type),
+ EventMetric::Child(c) => panic!(
+ "Cannot get the number of recorded errors for {:?} in non-main process!",
+ c.id
+ ),
+ }
+ }
+}
+
#[inherent]
impl<K: 'static + ExtraKeys + Send + Sync + Clone> Event for EventMetric<K> {
type Extra = K;
@@ -199,16 +211,6 @@ impl<K: 'static + ExtraKeys + Send + Sync + Clone> Event for EventMetric<K> {
}
}
}
-
- pub fn test_get_num_recorded_errors(&self, error: glean::ErrorType) -> i32 {
- match self {
- EventMetric::Parent { inner, .. } => inner.test_get_num_recorded_errors(error),
- EventMetric::Child(meta) => panic!(
- "Cannot get the number of recorded errors for {:?} in non-main process!",
- meta.id
- ),
- }
- }
}
#[inherent]
diff --git a/toolkit/components/glean/api/src/private/mod.rs b/toolkit/components/glean/api/src/private/mod.rs
@@ -677,6 +677,21 @@ fn truncate_string_for_marker_to_length(mut input: String, byte_length: usize) -
input
}
+pub trait TestGetNumErrors {
+ /// **Exported for test purposes.**
+ ///
+ /// Gets the number of recorded errors for the given metric and error type.
+ ///
+ /// # Arguments
+ ///
+ /// * `error` - The type of error
+ ///
+ /// # Returns
+ ///
+ /// The number of errors reported.
+ fn test_get_num_recorded_errors(&self, error_type: glean::ErrorType) -> i32;
+}
+
#[cfg(test)]
mod truncation_tests {
use crate::private::truncate_string_for_marker;
diff --git a/toolkit/components/glean/build_scripts/glean_parser_ext/templates/rust.jinja2 b/toolkit/components/glean/build_scripts/glean_parser_ext/templates/rust.jinja2
@@ -432,12 +432,14 @@ pub(crate) mod __glean_metric_maps {
///
/// Panics if no event by the given metric ID could be found.
pub(crate) fn event_test_get_value_wrapper(metric_id: u32, ping_name: Option<String>) -> Option<Vec<RecordedEvent>> {
- match metric_id {
+ let event = match metric_id {
{% for metric_id, event in events_by_id.items() %}
- {{metric_id}} => super::{{event}}.test_get_value(ping_name),
+ {{metric_id}} => &*super::{{event}} as &dyn ::glean::TestGetValue<Output=Vec<RecordedEvent>>,
{% endfor %}
_ => panic!("No event for metric id {}", metric_id),
- }
+ };
+
+ return event.test_get_value(ping_name);
}
/// Check the provided event for errors.
@@ -458,11 +460,15 @@ pub(crate) mod __glean_metric_maps {
#[allow(unused_variables)]
pub(crate) fn event_test_get_error(metric_id: u32) -> Option<String> {
#[cfg(feature = "with_gecko")]
- match metric_id {
+ {
+ let event = match metric_id {
{% for metric_id, event in events_by_id.items() %}
- {{metric_id}} => test_get_errors!(super::{{event}}),
+ {{metric_id}} => &*super::{{event}} as &dyn crate::private::TestGetNumErrors,
{% endfor %}
- _ => panic!("No event for metric id {}", metric_id),
+ _ => panic!("No event for metric id {}", metric_id),
+ };
+
+ return test_get_errors!(event);
}
#[cfg(not(feature = "with_gecko"))]
diff --git a/toolkit/components/glean/tests/pytest/metrics_test_output b/toolkit/components/glean/tests/pytest/metrics_test_output
@@ -1863,13 +1863,15 @@ pub(crate) mod __glean_metric_maps {
///
/// Panics if no event by the given metric ID could be found.
pub(crate) fn event_test_get_value_wrapper(metric_id: u32, ping_name: Option<String>) -> Option<Vec<RecordedEvent>> {
- match metric_id {
- 21 => super::test_nested::event_metric.test_get_value(ping_name),
- 22 => super::test_nested::event_metric_with_extra.test_get_value(ping_name),
- 49 => super::test2_nested::event_metric.test_get_value(ping_name),
- 50 => super::test2_nested::event_metric_with_extra.test_get_value(ping_name),
+ let event = match metric_id {
+ 21 => &*super::test_nested::event_metric as &dyn ::glean::TestGetValue<Output=Vec<RecordedEvent>>,
+ 22 => &*super::test_nested::event_metric_with_extra as &dyn ::glean::TestGetValue<Output=Vec<RecordedEvent>>,
+ 49 => &*super::test2_nested::event_metric as &dyn ::glean::TestGetValue<Output=Vec<RecordedEvent>>,
+ 50 => &*super::test2_nested::event_metric_with_extra as &dyn ::glean::TestGetValue<Output=Vec<RecordedEvent>>,
_ => panic!("No event for metric id {}", metric_id),
- }
+ };
+
+ return event.test_get_value(ping_name);
}
/// Check the provided event for errors.
@@ -1890,12 +1892,16 @@ pub(crate) mod __glean_metric_maps {
#[allow(unused_variables)]
pub(crate) fn event_test_get_error(metric_id: u32) -> Option<String> {
#[cfg(feature = "with_gecko")]
- match metric_id {
- 21 => test_get_errors!(super::test_nested::event_metric),
- 22 => test_get_errors!(super::test_nested::event_metric_with_extra),
- 49 => test_get_errors!(super::test2_nested::event_metric),
- 50 => test_get_errors!(super::test2_nested::event_metric_with_extra),
- _ => panic!("No event for metric id {}", metric_id),
+ {
+ let event = match metric_id {
+ 21 => &*super::test_nested::event_metric as &dyn crate::private::TestGetNumErrors,
+ 22 => &*super::test_nested::event_metric_with_extra as &dyn crate::private::TestGetNumErrors,
+ 49 => &*super::test2_nested::event_metric as &dyn crate::private::TestGetNumErrors,
+ 50 => &*super::test2_nested::event_metric_with_extra as &dyn crate::private::TestGetNumErrors,
+ _ => panic!("No event for metric id {}", metric_id),
+ };
+
+ return test_get_errors!(event);
}
#[cfg(not(feature = "with_gecko"))]
