Do we need to check shouldRecord twice for every metric operation?
Categories
(Data Platform and Tools :: Glean: SDK, defect, P3)
Tracking
(Not tracked)
People
(Reporter: chutten, Unassigned)
References
Details
(Whiteboard: [telemetry:glean-rs:m8])
To add to a CounterMetricType in Kotlin we end up doing the following
public fun add
private fun shouldRecord
FFI/glean_counter_should_record
pub trait MetricType fn should_record
Dispatch
FFI/glean_counter_add
pub fn CounterMetric.add
pub trait MetricType fn should_record
<and then the addition>
pub trait MetricType fn should_record
is repeated, and we end up crossing the FFI boundary twice to do it. Can we avoid any/all of that?
I naively posited we could get rid of the kotlin-side shouldRecord altogether (it removes some FFI surface as well, which might be desirable), but Alessio pointed out that the Dispatch might be the most important part of the equation: we might want to preserve the behaviour where we don't dispatch if we shouldn't record.
Also, I said "crossing the FFI boundary twice" as though the two crossings were equivalent. In the case of, e.g. glean_string_list_set
we have to convert a List<String>
for FFI. The present arrangement allows us to avoid that conversion in cases when we shouldn't record.
Reporter | ||
Comment 1•6 years ago
|
||
Oops, missed an FFI boundary. kt/shouldRecord
also calls FFI/glean_is_initialized
Reporter | ||
Comment 2•6 years ago
|
||
Looks like bug 1557339 is looking in to whether we need that third one at all, which may change the calculus here.
Updated•6 years ago
|
Updated•6 years ago
|
Comment 3•6 years ago
|
||
We changed how the Kotlin side works since then and only do a disabled
check on the Kotlin side and do should record
checks on the Rust side.
This bug is therefore invalid now.
Description
•