Add microbenchmarks for table expiry & eviction - #1042
Conversation
This PR adds the above benchmarks to `cargo ubench` for the above functions at different levels of table occupancy to get a bead on what the actual costs involved are. For expiry/cleanup tests, we set the maximum table size and then mark some P∊[0,1] flows as having timestamps such that the flow should be removed entirely. For eviction benchmarks, we ensure that we have at least one full LFT in addition to a full UFT before processing a packet on a new 5-tuple.
| type Output = Self; | ||
|
|
||
| fn sub(self, rhs: Duration) -> Self::Output { | ||
| cfg_if! { |
There was a problem hiding this comment.
In the no-std case we can have subtraction that results in a negative value. The std case will panic with overflow if rhs is larger than self.inner. Should both cases be a saturating sub?
There was a problem hiding this comment.
We only need this impl for setting up test cases, so I've taken out the no_std half and gated it accordingly. I think saturating there is a good idea for now.
I think for hrtime_t in the kernel context becoming negative is fine: that just means we have a timestamp from before the CPU's TSC was last reset. I've opened #1052 as a related issue, I'd evidently missed the signedness of the type. Fixing that would probably be the best way to unify the behaviour here I think.
|
|
||
| let first_ts = *FIRST_TS.get_or_init(Instant::now); | ||
| let first_ts = *FIRST_TS.get_or_init(|| | ||
| Instant::now() - Duration::from_mins(5) |
There was a problem hiding this comment.
I'm not following what's happening here. We define an instant that is 5 minutes ago and then the moment called now is the time since five minutes ago and whatever time elapsed between the recording of first_ts and return value construction? I think some comments are needed here.
There was a problem hiding this comment.
I think this one was explained in the review for when we were adding in atomic timestamps on table entries but we never transcribed it to the code, sorry.
We have this slightly awkward construction because there's no API on std::time::Instant to convert it into any kind of raw numeric type, whereas gethrtime gives us that quite explicitly. So to enable atomic timestamps for std we need to set some timestamp as our zero value to get a meaningful u64 (or i64). The problem here being that if we want to set flow entries to some time before we run cargo test/cargo ubench, then we do hit a panicking subtract. It might be possible to come up with something using SystemTime, but this is all a work around for the test harness. I've tried to clarify what's going on here better.
2 participants
This PR adds benchmarks to
cargo ubenchfor the above functionsat different levels of table occupancy to get a bead on what
the actual costs involved are.
For expiry/cleanup tests, we set the maximum table size and then mark
some P∊[0,1] flows as having timestamps such that the flow should be
removed entirely. For eviction benchmarks, we ensure that we have at
least one full LFT in addition to a full UFT before processing a packet
on a new 5-tuple.