Re: Adding latency tracking to trace-cmd

[Viktor Rosendahl <viktor.rosendahl@gmail.com>]

On Tue, 2021-04-06 at 16:42 -0400, Steven Rostedt wrote:
 > On Tue, 6 Apr 2021 20:03:46 +0000
 > <Viktor.Rosendahl@bmw.de> wrote:
 >
<clip>
 > > Without the --random option, I would at this point need to start 
another
 > > test
 > > campaign, only then would I start seeing the longer latencies from 
bar().
 > >
 > > On the other hand, if I use the --random option with the 
latency-collector,
 > > then
 > > what will happen is that sometimes the latency-collector will open 
the trace
 > > file immediately and other times it will sleep and wait for up to 
one second
 > > before doing so. If it opens the file immediately, we will get the 
first
 > > latency. If based on that random toin coss function decides to 
sleep, then
 > > we
 > > will get the second.
 > >
 > > If a long test camaping is exectuted, and foobar() is called 
several times
 > > during that campaign, then there is a good probability that we will 
capture
 > > both
 > > the latency from foo() and the one from bar().
 > > Now --random is a bit more complicated because it actually tosses 
the coin
 > > again
 > > if another latency occurs when it is sleeping before opening the 
file. The
 > > probability of that coin toss function are chosen so that if we 
assume that
 > > there is a maximum of N closely occuring latencies, we will get 
each of them
 > > with probability 1/N. If the latency-collector detects a situation 
were it
 > > actually has detected N closely occuring latencies, it will 
automatically
 > > increase N to N + 1 and update the probablities of the coin toss
 > > accordingly.
 > >
 > > So basically the idea is that by acting randomly, we will not 
systematically
 > > lose a particular latency. It will also not make matters worse if 
we never
 > > get
 > > latencies that occur close to each other; the only drawback is that we
 > > sometimes
 > > wait for one second before opening the trace file and that doesn't 
really
 > > matter
 > > if there aren't any closely occuring latencies.
 >
 > Hmm, sounds more like "--various" would be better than "--random". Just
 > because it appears you wont to try different timings. Having a --random
 > option just doesn't sound like it's what you expect it to be.
 >

I guess that you are right that with --random many people may assume the 
sleep time to be random. Perhaps --dizzy-sleep or --arbitrary-sleep 
would be better?

For the latency-collector, I used the name --random because the behavior 
is based on the lrand48_r() call, seeded by /dev/urandom. In my thinking 
it's the sleeping behavior that is random, not the sleep time.

I guess that a mathematical purist may say that the behavior is 
arbitrary rather than random, because if the value N is different from 
two, then there are unequal probabilities between the two choices.

At first when I developed the latency-collector, I thought that using a 
random sleep time would be the right approach but I came to the 
conclusion that it is not a good idea.

If we have the case with a burst of two latencies, where the first one 
is 5 ms, , then if we for example sleep randomly between 0 and 500 ms, 
we will only have a 1% chance to get the first latency. If we use the 
random algorithm from the latency-collector, with N=2, then we have 50% 
chance, which is much better. Even if we would have been paranoid to 
initialize with N=5, in order to prepare for bursts sizes of up to 5, 
then we would still have a 20% chance.

If we have a random sleep time then we would need to make assumptions on 
how long the latencies are and how much time there is between them, and 
there is no way to guess that beforehand.

With the random behavior, we only need to make an assumption about how 
many latencies there are going to be in a burst, which is the value N. 
This is also impossible to know with certainty but we can make some 
educated guess about it. Also, as I already mentioned, if the 
latency-collector ever encounters N latencies in a burst, then it will 
automatically increment N.

 > There's already a "-s" option that takes a sleep interval between 
wakeups,
 > which sounds similar to what you have. Perhaps we can make "-1" a special
 > value to do the "random" wakeup thing. "0" is already special to make it
 > wake up when it detects data in the buffer.
 >

To me "-s -1" feels a bit too cryptic and non-descriptive. Also, I 
wonder how many would read the description of the -s option carefully 
enough to notice it.

On the other hand, trace-cmd record seems to already use most of the 
alphabet as short options.

Do you think it could be acceptable to add a new long option, such as 
--dizzy-sleep?

best regards,

Viktor