Bug 1916673 - Buffered API for memory/timing distributions

[badboy]

This is the implementation for what's being designed in bug-1915388

• final ack in the design bug
• docs will be done separately (bug-1917809)
• experimental marker

[badboy]

Usage of it WIP: https://phabricator.services.mozilla.com/D221017

[mxinden]

Thank you for the work here.

If I understand this pull request correctly, it only adds a buffered API to MemoryDistribution and TimingDistribution.

In https://phabricator.services.mozilla.com/D220062 I want to add 4 new metrics:

1. http3_udp_datagram_segment_size_sent
2. http3_udp_datagram_segment_size_received
3. http3_udp_datagram_size_received
4. http3_udp_datagram_num_segments_received

(1), (2) and (3) I can model with a MemoryDistribution. (4) on the other hand counts the number of segments received, where values range between 1 and 128.

Here is metrics definition draft from the above Phabricator patch.

  http3_udp_datagram_num_segments_received:
    type: custom_distribution
    unit: integer
    range_min: 0
    # Maximum number of UDP segments per datagram.
    #
    # See e.g. Linux https://github.com/torvalds/linux/blob/20371ba120635d9ab7fc7670497105af8f33eb08/include/linux/udp.h#L111.
    range_max: 128
    bucket_count: 100
    histogram_type: exponential
    description: >
      HTTP3 UDP number of segments per datagram received.
    bugs:
      - https://bugzilla.mozilla.org/show_bug.cgi?id=1906853
    data_reviews:
      - https://bugzilla.mozilla.org/show_bug.cgi?id=1906853
    data_sensitivity:
      - technical
    notification_emails:
      - necko@mozilla.com
      - minden@mozilla.com
    expires: never

I assume this is best modeled by the CustomDistribution. Would a buffered API for CustomDistribution be included in this patch?

(Note, in case you want to ship a buffered API for one distribution only for now, I assume I could as well model (1), (2) and (3) with a CustomDistribution instead of a MemoryDistribution.)

[chutten]

As much as I'd like this to be a r+with_followups, I think we need to decide about the FFI and Kotlin interfaces for certain before we ship this, as subsequent changes will be breaking.

[chutten]

Is there a way to prevent it from being used after commit()? I think the Java-like pattern is tothrow new IllegalStateException("Use after commit");

[badboy]

Hmmm.
Then we need to track that state.

I'm considering to remove the Kotlin part. The first user is Rust only.

[chutten]

Might want protected fun finalize() { /* TODO: bug 1691073 */ report_some_error(); ffi_abandon_buffer(this); or whatever to help report misuse and prevent memory leaks.

[chutten]

Should we clear the buffer to stop folks accidentally committing the same buffer twice? Or is that prevented somewhere/somehow else? For non-FFI we rely on the "called at most once" behaviour of drop, but I don't see us accounting for the lack of that guarantee (or testing that case) here.

[badboy]

Ok, I split Kotlin out so we can tackle that separately.

[chutten]

r+wc