[Spice-devel] [PATCH spice-server 1/2] utils: Get monotonic time in a coherent way

[Frediano Ziglio]

> 
> On Tue, Oct 09, 2018 at 01:14:10PM +0100, Frediano Ziglio wrote:
> > Use a single function to get monotonic time.
> > As lot of code needs nanosecond precision, spice_get_monotonic_time_ns
> > is used.
> > 
> 
> "lots of code needs nanosecond precision"? I looked at a few files using
> spice_get_monotonic_time_ns, and nanosecond precision did not seem
> useful in any of those (the code was adding a timeout in second to it, or
> using g_usleep, or converting it to some other unit straight away). So I
> would not add this misleading comment to the log ;) Maybe this should
> be adding a spice_get_monotonic_get_us helper so that it's clear we
> don't need that much precision?
> 
> Christophe
> 

There are 32 calls and only 4 seems to require less precision than
that. Maybe usec are enough but as at the end we need to call
clock_gettime and use 64 bit integers anyway so using nanoseconds
by default is not more costly. I don't know if switching to
microseconds could cause some wrong rounding, surely not
switching won't have this issue.

You just removed the XX_ms one, now we want the XX_us? At this
point let's have all XX_ns, XX_us, XX_ms and XX_s.

How should I change the comment?

Frediano

> 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  server/main-channel-client.c | 4 ++--
> >  server/reds.c                | 2 +-
> >  server/utils.h               | 3 ++-
> >  3 files changed, 5 insertions(+), 4 deletions(-)
> > 
> > diff --git a/server/main-channel-client.c b/server/main-channel-client.c
> > index b1e5a3e8..54be9934 100644
> > --- a/server/main-channel-client.c
> > +++ b/server/main-channel-client.c
> > @@ -503,7 +503,7 @@ void main_channel_client_handle_pong(MainChannelClient
> > *mcc, SpiceMsgPing *ping,
> >      uint64_t roundtrip;
> >      RedChannelClient* rcc = RED_CHANNEL_CLIENT(mcc);
> >  
> > -    roundtrip = g_get_monotonic_time() - ping->timestamp;
> > +    roundtrip = spice_get_monotonic_time_ns() / NSEC_PER_MICROSEC -
> > ping->timestamp;
> >  
> >      if (ping->id != mcc->priv->net_test_id) {
> >          /*
> > @@ -749,7 +749,7 @@ static void main_channel_marshall_ping(RedChannelClient
> > *rcc,
> >  
> >      red_channel_client_init_send_data(rcc, SPICE_MSG_PING);
> >      ping.id = main_channel_client_next_ping_id(mcc);
> > -    ping.timestamp = g_get_monotonic_time();
> > +    ping.timestamp = spice_get_monotonic_time_ns() / NSEC_PER_MICROSEC;
> >      spice_marshall_msg_ping(m, &ping);
> >  
> >      while (size_left > 0) {
> > diff --git a/server/reds.c b/server/reds.c
> > index bdd6a45c..bff5b68e 100644
> > --- a/server/reds.c
> > +++ b/server/reds.c
> > @@ -3004,7 +3004,7 @@ static void migrate_timeout(void *opaque)
> >  
> >  uint32_t reds_get_mm_time(void)
> >  {
> > -    return g_get_monotonic_time() / 1000;
> > +    return spice_get_monotonic_time_ns() / NSEC_PER_MILLISEC;
> >  }
> >  
> >  void reds_enable_mm_time(RedsState *reds)
> > diff --git a/server/utils.h b/server/utils.h
> > index 07878539..a2747cf7 100644
> > --- a/server/utils.h
> > +++ b/server/utils.h
> > @@ -54,8 +54,9 @@ typedef int64_t red_time_t;
> >  
> >  #define NSEC_PER_SEC      1000000000LL
> >  #define NSEC_PER_MILLISEC 1000000LL
> > +#define NSEC_PER_MICROSEC 1000
> >  
> > -/* FIXME: consider g_get_monotonic_time (), but in microseconds */
> > +/* g_get_monotonic_time() as not enough precision */
> >  static inline red_time_t spice_get_monotonic_time_ns(void)
> >  {
> >      struct timespec time;