1699505 - TimingDistributionMetricType.measure can end without calling `stop` and `cancel` if non-local returns used

Status: RESOLVED FIXED

(Data Platform and Tools :: Glean: SDK, defect, P1)

Description

[Michael Comella (:mcomella) [NI reported issues only: ex-Mozilla]]

In the following code, stopAndAccumulate and cancel will not be called:

fun onCreateView() = PerfStartup.homeActivityOnCreateView.measure {
    val layout = LayoutInflater.inflate(...)
    return layout
}

The problem is that measure is inlined. When inlined, the code looks like the following (I believe, I'm not knowledgeable about inline but you can decompile the code for a more accurate look):

fun onCreateView() {
    val timerId = start()

    val returnValue = try {
        val layout = LayoutInflater.inflate(...)
        return layout
    } catch (e: Exception) {
        cancel(timerId)
        throw e
    }

    // Note: we returned earlier so code below here is never called

    stopAndAccumulate(timerId)
    return returnValue
}

This is problematic: users will probably not expect an early return (especially if it looks like the final return) to cause stop to not be called.

I think there are two solutions:

1. remove inline: this prevents confusing function call semantics
2. add crossinline to the lambda: AIUI, this only allows returns to the lambda rather than the global function but I'm not sure

inline is a performance optimization that (according to IDE warnings) is only non-negligible when code is executed in a loop (it's removing maybe two functional calls and an allocation). The trade-off is that the binary increases in size for each call used.

For our specific use case, I prefer 1): I don't think measure will frequently be called in a tight loop (we rarely write such loops in FE dev and it's unclear we'd ever want to measure them individually) and we have the option of calling start/stop directly if we identify the additional function calls/allocation as a performance bottleneck. Without measure being called from a loop, the positive/negative impact of both of these is pretty vague until the function is called enough – then the binary size increase will be noticeable. So, to me, it seems like we're more likely to hit the negative consequences of inlineing this than gain the benefit.

fwiw, I skimmed this article to help me understand the semantics of inline functions: https://blog.mindorks.com/understanding-inline-noinline-and-crossinline-in-kotlin

Comment 1

[GitHub Bugzilla PR Linker]

Attachment: Link to GitHub pull-request: https://github.com/mozilla/glean/pull/1560

Comment 2

[Alessio Placitelli [:Dexter]]

(In reply to Michael Comella (:mcomella) [needinfo or I won't see it] from comment #0)

In the following code, stopAndAccumulate and cancel will not be called:

fun onCreateView() = PerfStartup.homeActivityOnCreateView.measure {
    val layout = LayoutInflater.inflate(...)
    return layout
}

Woha Michael, you find quite a good pearl here! Great catch and thanks for the detailed bug report!

Comment 3

[Jan-Erik Rediger [:janerik] (Away 2026-04-29 - 2026-05-01]

badboy merged PR #1560: "Bug 1699505 - Kotlin: Don't inline the closure when measuring" in ec0072b.