refactor(client): encapsulate SignedDuration and enhance its API
Hide internal fields to improve type safety and provide helper methods for unit conversions and negative value checks. Fix several feature-gated test compilation issues.
This commit is contained in:
1 parent
bec56d4114
commit
2d6581f31f
12 files changed
+107
-77
No files matched your search
@@ -774,7 +774,7 @@ fn duration_us(duration: Duration) -> u128 {
|
||||
}
|
||||
|
||||
fn signed_duration_us(duration: SignedDuration) -> i128 {
|
||||
duration.ns / 1_000
|
||||
duration.as_micros()
|
||||
}
|
||||
|
||||
fn format_optional_duration(duration: Option<Duration>) -> String {
|
||||
@@ -788,7 +788,7 @@ fn format_duration(duration: Duration) -> String {
|
||||
}
|
||||
|
||||
fn format_signed_duration(duration: SignedDuration) -> String {
|
||||
format_signed_ns(duration.ns as f64)
|
||||
format_signed_ns(duration.as_nanos() as f64)
|
||||
}
|
||||
|
||||
fn format_signed_ns(ns: f64) -> String {
|
||||
@@ -869,8 +869,8 @@ mod tests {
|
||||
received_at: test_timestamp(Duration::from_secs(1) + Duration::from_micros(1500)),
|
||||
rtt: RttSample {
|
||||
raw: Duration::from_micros(1500),
|
||||
adjusted: Some(SignedDuration { ns: 1_200_000 }),
|
||||
effective: SignedDuration { ns: 1_200_000 },
|
||||
adjusted: Some(SignedDuration::from_nanos(1_200_000)),
|
||||
effective: SignedDuration::from_nanos(1_200_000),
|
||||
},
|
||||
server_timing: Some(ServerTiming {
|
||||
receive_wall_ns: Some(1_000),
|
||||
@@ -894,6 +894,7 @@ mod tests {
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(feature = "stats")]
|
||||
fn negative_adjusted_reply_event() -> ClientEvent {
|
||||
ClientEvent::EchoReply {
|
||||
seq: 8,
|
||||
@@ -902,8 +903,8 @@ mod tests {
|
||||
received_at: test_timestamp(Duration::from_secs(1) + Duration::from_micros(500)),
|
||||
rtt: RttSample {
|
||||
raw: Duration::from_micros(500),
|
||||
adjusted: Some(SignedDuration { ns: -1_500_000 }),
|
||||
effective: SignedDuration { ns: -1_500_000 },
|
||||
adjusted: Some(SignedDuration::from_nanos(-1_500_000)),
|
||||
effective: SignedDuration::from_nanos(-1_500_000),
|
||||
},
|
||||
server_timing: Some(ServerTiming {
|
||||
receive_wall_ns: Some(1_000),
|
||||
|
||||
@@ -15,7 +15,7 @@ use irtt_cli::{
|
||||
};
|
||||
use irtt_client::{Client, ClientEvent, OpenOutcome, RecvBudget};
|
||||
|
||||
#[cfg(test)]
|
||||
#[cfg(all(test, feature = "stats"))]
|
||||
use irtt_client::ClientTimestamp;
|
||||
|
||||
#[cfg(feature = "stats")]
|
||||
@@ -247,9 +247,14 @@ impl<W: Write> StreamOutput<'_, W> {
|
||||
|
||||
let line =
|
||||
if self.mode == OutputMode::Human && !matches!(event, ClientEvent::EchoSent { .. }) {
|
||||
#[cfg(feature = "stats")]
|
||||
let human_stats = stats_update.into();
|
||||
#[cfg(not(feature = "stats"))]
|
||||
let human_stats = stats_update;
|
||||
|
||||
Some(format_human_event_with_options(
|
||||
event,
|
||||
Some(stats_update.into()),
|
||||
Some(human_stats),
|
||||
self.human_options,
|
||||
))
|
||||
} else {
|
||||
@@ -426,9 +431,7 @@ mod tests {
|
||||
rtt: RttSample {
|
||||
raw: Duration::from_micros(rtt_us),
|
||||
adjusted: None,
|
||||
effective: SignedDuration {
|
||||
ns: i128::from(rtt_us) * 1_000,
|
||||
},
|
||||
effective: SignedDuration::from_nanos(i128::from(rtt_us) * 1_000),
|
||||
},
|
||||
server_timing: None,
|
||||
one_way: None,
|
||||
@@ -466,7 +469,7 @@ mod tests {
|
||||
rtt: RttSample {
|
||||
raw: Duration::from_micros(1200),
|
||||
adjusted: None,
|
||||
effective: SignedDuration { ns: 1_200_000 },
|
||||
effective: SignedDuration::from_nanos(1_200_000),
|
||||
},
|
||||
server_timing: None,
|
||||
one_way: None,
|
||||
@@ -579,7 +582,7 @@ mod tests {
|
||||
rtt: RttSample {
|
||||
raw: Duration::from_micros(1),
|
||||
adjusted: None,
|
||||
effective: SignedDuration { ns: 1_000 },
|
||||
effective: SignedDuration::from_nanos(1_000),
|
||||
},
|
||||
server_timing: None,
|
||||
one_way: None,
|
||||
|
||||
@@ -206,8 +206,8 @@ mod tests {
|
||||
received_at,
|
||||
rtt: RttSample {
|
||||
raw: Duration::from_micros(1500),
|
||||
adjusted: Some(SignedDuration { ns: 1_200_000 }),
|
||||
effective: SignedDuration { ns: 1_200_000 },
|
||||
adjusted: Some(SignedDuration::from_nanos(1_200_000)),
|
||||
effective: SignedDuration::from_nanos(1_200_000),
|
||||
},
|
||||
server_timing: Some(ServerTiming {
|
||||
receive_wall_ns: None,
|
||||
@@ -252,8 +252,8 @@ mod tests {
|
||||
received_at,
|
||||
rtt: RttSample {
|
||||
raw: Duration::from_micros(1500),
|
||||
adjusted: Some(SignedDuration { ns: 1_200_000 }),
|
||||
effective: SignedDuration { ns: 1_200_000 },
|
||||
adjusted: Some(SignedDuration::from_nanos(1_200_000)),
|
||||
effective: SignedDuration::from_nanos(1_200_000),
|
||||
},
|
||||
server_timing: None,
|
||||
one_way: None,
|
||||
|
||||
@@ -727,9 +727,8 @@ fn compute_rtt(
|
||||
|
||||
let server_processing = compute_server_processing(ts);
|
||||
|
||||
let adjusted = server_processing.map(|sp| SignedDuration {
|
||||
ns: duration_ns_i128(raw) - duration_ns_i128(sp),
|
||||
});
|
||||
let adjusted = server_processing
|
||||
.map(|sp| SignedDuration::from_nanos(duration_ns_i128(raw) - duration_ns_i128(sp)));
|
||||
let effective = adjusted.unwrap_or_else(|| SignedDuration::from_duration(raw));
|
||||
|
||||
RttSample {
|
||||
|
||||
@@ -1,3 +1,4 @@
|
||||
#[cfg(all(target_os = "linux", feature = "ancillary"))]
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -18,8 +18,8 @@ fn server_processing_greater_than_raw_does_not_underflow() {
|
||||
..Default::default()
|
||||
},
|
||||
);
|
||||
assert_eq!(rtt.adjusted, Some(SignedDuration { ns: -999_999_999 }));
|
||||
assert_eq!(rtt.effective, SignedDuration { ns: -999_999_999 });
|
||||
assert_eq!(rtt.adjusted, Some(SignedDuration::from_nanos(-999_999_999)));
|
||||
assert_eq!(rtt.effective, SignedDuration::from_nanos(-999_999_999));
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -117,17 +117,37 @@ pub struct RttSample {
|
||||
pub effective: SignedDuration,
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq)]
|
||||
#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
|
||||
pub struct SignedDuration {
|
||||
pub ns: i128,
|
||||
ns: i128,
|
||||
}
|
||||
|
||||
impl SignedDuration {
|
||||
pub const fn from_nanos(ns: i128) -> Self {
|
||||
Self { ns }
|
||||
}
|
||||
|
||||
pub fn from_duration(duration: Duration) -> Self {
|
||||
Self {
|
||||
ns: i128::try_from(duration.as_nanos()).unwrap_or(i128::MAX),
|
||||
}
|
||||
}
|
||||
|
||||
pub const fn as_nanos(self) -> i128 {
|
||||
self.ns
|
||||
}
|
||||
|
||||
pub const fn as_micros(self) -> i128 {
|
||||
self.ns / 1_000
|
||||
}
|
||||
|
||||
pub const fn as_millis(self) -> i128 {
|
||||
self.ns / 1_000_000
|
||||
}
|
||||
|
||||
pub const fn is_negative(self) -> bool {
|
||||
self.ns < 0
|
||||
}
|
||||
}
|
||||
|
||||
impl From<Duration> for SignedDuration {
|
||||
@@ -170,18 +190,3 @@ pub struct PacketMeta {
|
||||
pub ecn: Option<u8>,
|
||||
pub kernel_rx_timestamp: Option<std::time::SystemTime>,
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
mod tests {
|
||||
use super::PacketMeta;
|
||||
|
||||
#[test]
|
||||
fn packet_meta_default_is_unavailable() {
|
||||
let meta = PacketMeta::default();
|
||||
|
||||
assert_eq!(meta.traffic_class, None);
|
||||
assert_eq!(meta.dscp, None);
|
||||
assert_eq!(meta.ecn, None);
|
||||
assert_eq!(meta.kernel_rx_timestamp, None);
|
||||
}
|
||||
}
|
||||
@@ -352,7 +352,7 @@ fn assert_processing_subtracted(reply: ReplyView<'_>) {
|
||||
assert_eq!(processing, Duration::from_nanos(1));
|
||||
let adjusted = reply.rtt.adjusted.unwrap();
|
||||
assert_eq!(reply.rtt.effective, adjusted);
|
||||
assert!(adjusted.ns < SignedDuration::from_duration(reply.rtt.raw).ns);
|
||||
assert!(adjusted < SignedDuration::from_duration(reply.rtt.raw));
|
||||
}
|
||||
|
||||
fn assert_one_way_presence(
|
||||
|
||||
@@ -203,7 +203,7 @@ fn duration_ns_i128(duration: Duration) -> i128 {
|
||||
}
|
||||
|
||||
fn signed_duration_ns(duration: SignedDuration) -> i128 {
|
||||
duration.ns
|
||||
duration.as_nanos()
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
|
||||
@@ -12,14 +12,21 @@ pub fn ts(ms: u64) -> ClientTimestamp {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn rtt(raw_ms: u64, effective_ms: i128) -> RttSample {
|
||||
let effective = SignedDuration {
|
||||
ns: effective_ms * 1_000_000,
|
||||
};
|
||||
pub fn adjusted_rtt(raw_ms: u64, adjusted_ms: i128) -> RttSample {
|
||||
let adjusted = SignedDuration::from_nanos(adjusted_ms * 1_000_000);
|
||||
RttSample {
|
||||
raw: Duration::from_millis(raw_ms),
|
||||
adjusted: Some(effective),
|
||||
effective,
|
||||
adjusted: Some(adjusted),
|
||||
effective: adjusted,
|
||||
}
|
||||
}
|
||||
|
||||
pub fn unadjusted_rtt(raw_ms: u64) -> RttSample {
|
||||
let raw = Duration::from_millis(raw_ms);
|
||||
RttSample {
|
||||
raw,
|
||||
adjusted: None,
|
||||
effective: SignedDuration::from_duration(raw),
|
||||
}
|
||||
}
|
||||
|
||||
@@ -35,7 +42,15 @@ pub fn sent(seq: u32, sent_at: ClientTimestamp) -> ClientEvent {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn reply(seq: u32, raw_ms: u64, effective_ms: i128) -> ClientEvent {
|
||||
pub fn adjusted_reply(seq: u32, raw_ms: u64, adjusted_ms: i128) -> ClientEvent {
|
||||
reply_with_rtt(seq, raw_ms, adjusted_rtt(raw_ms, adjusted_ms))
|
||||
}
|
||||
|
||||
pub fn unadjusted_reply(seq: u32, raw_ms: u64) -> ClientEvent {
|
||||
reply_with_rtt(seq, raw_ms, unadjusted_rtt(raw_ms))
|
||||
}
|
||||
|
||||
fn reply_with_rtt(seq: u32, raw_ms: u64, rtt: RttSample) -> ClientEvent {
|
||||
let sent_at = ts(seq as u64 * 10);
|
||||
let received_at = ClientTimestamp {
|
||||
mono: sent_at.mono + Duration::from_millis(raw_ms),
|
||||
@@ -46,7 +61,7 @@ pub fn reply(seq: u32, raw_ms: u64, effective_ms: i128) -> ClientEvent {
|
||||
remote: "127.0.0.1:2112".parse().unwrap(),
|
||||
sent_at,
|
||||
received_at,
|
||||
rtt: rtt(raw_ms, effective_ms),
|
||||
rtt,
|
||||
server_timing: Some(ServerTiming {
|
||||
receive_wall_ns: Some(unix_time_ns_after_epoch(sent_at.wall) as i64 + 1_000_000),
|
||||
receive_mono_ns: Some(seq as i64 * 10_000_000 + 1_000_000),
|
||||
@@ -69,7 +84,11 @@ pub fn reply(seq: u32, raw_ms: u64, effective_ms: i128) -> ClientEvent {
|
||||
}
|
||||
}
|
||||
|
||||
pub fn late_reply(seq: u32, raw_ms: u64, effective_ms: i128) -> ClientEvent {
|
||||
pub fn unadjusted_late_reply(seq: u32, raw_ms: u64) -> ClientEvent {
|
||||
late_reply_from(unadjusted_reply(seq, raw_ms))
|
||||
}
|
||||
|
||||
fn late_reply_from(reply: ClientEvent) -> ClientEvent {
|
||||
let ClientEvent::EchoReply {
|
||||
seq,
|
||||
remote,
|
||||
@@ -82,7 +101,7 @@ pub fn late_reply(seq: u32, raw_ms: u64, effective_ms: i128) -> ClientEvent {
|
||||
bytes,
|
||||
packet_meta,
|
||||
..
|
||||
} = reply(seq, raw_ms, effective_ms)
|
||||
} = reply
|
||||
else {
|
||||
unreachable!();
|
||||
};
|
||||
|
||||
@@ -2,19 +2,20 @@ mod common;
|
||||
|
||||
use std::time::{Duration, Instant};
|
||||
|
||||
use common::{late_reply, reply, sent, ts};
|
||||
use common::{adjusted_reply, sent, ts, unadjusted_late_reply, unadjusted_reply};
|
||||
use irtt_client::{ClientEvent, PacketMeta};
|
||||
use irtt_stats::{EventStatsUpdate, IpdvPairUpdate, SampleMode, StatsCollector, StatsConfig};
|
||||
|
||||
#[test]
|
||||
fn running_only_samples_avoid_finite_retention() {
|
||||
let mut collector = StatsCollector::new(StatsConfig::continuous());
|
||||
collector.process(&reply(0, 10, 9));
|
||||
collector.process(&reply(1, 20, 19));
|
||||
collector.process(&unadjusted_reply(0, 10));
|
||||
collector.process(&unadjusted_reply(1, 20));
|
||||
|
||||
let snapshot = collector.snapshot();
|
||||
assert_eq!(snapshot.rtt.primary.median_ns, None);
|
||||
assert_eq!(snapshot.rtt.raw.median_ns, None);
|
||||
assert_eq!(snapshot.rtt.adjusted.count, 0);
|
||||
assert_eq!(snapshot.rtt.adjusted.median_ns, None);
|
||||
assert_eq!(snapshot.ipdv.round_trip.median_ns, None);
|
||||
assert_eq!(snapshot.one_way_delay.send_delay.median_ns, None);
|
||||
@@ -25,7 +26,7 @@ fn running_only_samples_avoid_finite_retention() {
|
||||
fn continuous_mode_tracks_running_samples_without_exact_medians() {
|
||||
let mut collector = StatsCollector::new(StatsConfig::continuous());
|
||||
for seq in 0..4104 {
|
||||
collector.process(&reply(seq, 10, 10));
|
||||
collector.process(&unadjusted_reply(seq, 10));
|
||||
}
|
||||
|
||||
let snapshot = collector.snapshot();
|
||||
@@ -38,14 +39,15 @@ fn continuous_mode_tracks_running_samples_without_exact_medians() {
|
||||
#[test]
|
||||
fn cumulative_rtt_uses_signed_effective_and_tracks_raw() {
|
||||
let mut collector = StatsCollector::new(StatsConfig::finite());
|
||||
collector.process(&reply(0, 1, -2));
|
||||
collector.process(&reply(1, 10, 8));
|
||||
collector.process(&adjusted_reply(0, 1, -2));
|
||||
collector.process(&adjusted_reply(1, 10, 8));
|
||||
|
||||
let snapshot = collector.snapshot();
|
||||
assert_eq!(snapshot.rtt.primary.count, 2);
|
||||
assert_eq!(snapshot.rtt.primary.min_ns, Some(-2_000_000));
|
||||
assert_eq!(snapshot.rtt.primary.median_ns, Some(3_000_000.0));
|
||||
assert_eq!(snapshot.rtt.raw.total_ns, 11_000_000);
|
||||
assert_eq!(snapshot.rtt.adjusted.count, 2);
|
||||
}
|
||||
|
||||
#[test]
|
||||
@@ -53,8 +55,8 @@ fn late_unique_counts_and_duplicates_do_not_update_duplicate_measurements() {
|
||||
let mut collector = StatsCollector::new(StatsConfig::finite());
|
||||
collector.process(&sent(0, ts(0)));
|
||||
collector.process(&sent(1, ts(10)));
|
||||
collector.process(&reply(1, 10, 9));
|
||||
collector.process(&late_reply(0, 20, 19));
|
||||
collector.process(&unadjusted_reply(1, 10));
|
||||
collector.process(&unadjusted_late_reply(0, 20));
|
||||
collector.process(&ClientEvent::DuplicateReply {
|
||||
seq: 0,
|
||||
remote: "127.0.0.1:2112".parse().unwrap(),
|
||||
@@ -84,7 +86,7 @@ fn final_loss_uses_sent_minus_unique_replies_not_echo_loss_events() {
|
||||
sent_at: ts(0),
|
||||
timeout_at: Instant::now(),
|
||||
});
|
||||
collector.process(&late_reply(0, 10, 9));
|
||||
collector.process(&unadjusted_late_reply(0, 10));
|
||||
|
||||
let snapshot = collector.snapshot();
|
||||
assert_eq!(snapshot.events.loss_events, 1);
|
||||
@@ -114,9 +116,9 @@ fn assert_one_ipdv_pair(
|
||||
#[test]
|
||||
fn ipdv_is_sequence_adjacent_and_gap_preserving() {
|
||||
let mut collector = StatsCollector::new(StatsConfig::finite());
|
||||
let first = collector.process(&reply(0, 10, 10));
|
||||
let gap = collector.process(&reply(2, 15, 15));
|
||||
let adjacent = collector.process(&reply(3, 12, 12));
|
||||
let first = collector.process(&unadjusted_reply(0, 10));
|
||||
let gap = collector.process(&unadjusted_reply(2, 15));
|
||||
let adjacent = collector.process(&unadjusted_reply(3, 12));
|
||||
|
||||
let snapshot = collector.snapshot();
|
||||
assert!(first.contributed_sample);
|
||||
@@ -134,8 +136,8 @@ fn ipdv_is_sequence_adjacent_and_gap_preserving() {
|
||||
#[test]
|
||||
fn late_reply_can_complete_ipdv_pair() {
|
||||
let mut collector = StatsCollector::new(StatsConfig::finite());
|
||||
collector.process(&reply(1, 20, 20));
|
||||
let update = collector.process(&late_reply(0, 10, 10));
|
||||
collector.process(&unadjusted_reply(1, 20));
|
||||
let update = collector.process(&unadjusted_late_reply(0, 10));
|
||||
|
||||
let snapshot = collector.snapshot();
|
||||
|
||||
@@ -149,8 +151,8 @@ fn late_reply_can_complete_ipdv_pair() {
|
||||
#[test]
|
||||
fn update_exposes_directional_ipdv_when_available() {
|
||||
let mut collector = StatsCollector::new(StatsConfig::finite());
|
||||
collector.process(&reply(0, 10, 10));
|
||||
let update = collector.process(&reply(1, 13, 13));
|
||||
collector.process(&unadjusted_reply(0, 10));
|
||||
let update = collector.process(&unadjusted_reply(1, 13));
|
||||
|
||||
assert!(update.contributed_sample);
|
||||
|
||||
@@ -163,9 +165,9 @@ fn update_exposes_directional_ipdv_when_available() {
|
||||
fn gap_fill_update_exposes_both_completed_ipdv_pairs() {
|
||||
let mut collector = StatsCollector::new(StatsConfig::finite());
|
||||
|
||||
let first = collector.process(&reply(0, 10, 10));
|
||||
let gap = collector.process(&reply(2, 20, 20));
|
||||
let fill = collector.process(&reply(1, 13, 13));
|
||||
let first = collector.process(&unadjusted_reply(0, 10));
|
||||
let gap = collector.process(&unadjusted_reply(2, 20));
|
||||
let fill = collector.process(&unadjusted_reply(1, 13));
|
||||
|
||||
assert!(first.contributed_sample);
|
||||
assert!(first.ipdv_pairs.is_empty());
|
||||
@@ -192,7 +194,7 @@ fn gap_fill_update_exposes_both_completed_ipdv_pairs() {
|
||||
#[test]
|
||||
fn server_processing_and_one_way_require_available_samples() {
|
||||
let mut collector = StatsCollector::new(StatsConfig::finite());
|
||||
collector.process(&reply(0, 10, 9));
|
||||
collector.process(&unadjusted_reply(0, 10));
|
||||
collector.process(&ClientEvent::LateReply {
|
||||
seq: 9,
|
||||
highest_seen: 10,
|
||||
@@ -220,8 +222,8 @@ fn server_processing_and_one_way_require_available_samples() {
|
||||
#[test]
|
||||
fn older_cumulative_server_receive_count_does_not_regress() {
|
||||
let mut collector = StatsCollector::new(StatsConfig::finite());
|
||||
collector.process(&reply(1, 10, 10));
|
||||
collector.process(&late_reply(0, 10, 10));
|
||||
collector.process(&unadjusted_reply(1, 10));
|
||||
collector.process(&unadjusted_late_reply(0, 10));
|
||||
|
||||
let snapshot = collector.snapshot();
|
||||
assert_eq!(snapshot.packets.server_packets_received, Some(2));
|
||||
@@ -235,8 +237,8 @@ fn rolling_count_eviction_recomputes_from_bounded_events() {
|
||||
rolling_time: None,
|
||||
});
|
||||
collector.process(&sent(0, ts(0)));
|
||||
collector.process(&reply(0, 10, 10));
|
||||
collector.process(&reply(1, 20, 20));
|
||||
collector.process(&unadjusted_reply(0, 10));
|
||||
collector.process(&unadjusted_reply(1, 20));
|
||||
|
||||
let rolling = collector.rolling_count().unwrap();
|
||||
assert_eq!(rolling.packets.packets_sent, 0);
|
||||
@@ -276,7 +278,7 @@ fn directional_loss_uses_server_received_count_when_available() {
|
||||
let mut collector = StatsCollector::new(StatsConfig::finite());
|
||||
collector.process(&sent(0, ts(0)));
|
||||
collector.process(&sent(1, ts(10)));
|
||||
collector.process(&reply(0, 10, 10));
|
||||
collector.process(&unadjusted_reply(0, 10));
|
||||
|
||||
let loss = collector.snapshot().loss;
|
||||
assert_eq!(loss.lost_packets, 1);
|
||||
|
||||
Reference in new issue
Block a user