[Spice-devel] [PATCH spice-common] quic: Remove some too strict asserts in hot paths

[Victor Toso]

Hi,

On Wed, May 30, 2018 at 11:03:02AM -0400, Frediano Ziglio wrote:
> ping

Just by the thought that I don't remember any assert report on
quick (ever) plus the encoding/decoding time gain,

Acked-by: Victor Toso <victortoso at redhat.com>

I reviewed this before but forgot to reply. I spent most time
wondering how to remove some checks but I'm fine with
spice_extra_checks.

Cheers,
        toso

> > Some assert in the code are doing some paranoid test and in code
> > paths quite hot.
> > The encoding time is reduced by 30-50% while the decoding time
> > is reduced by a 20-30%.
> > 
> > Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
> > ---
> >  common/quic.c             | 20 ++++++++++++++------
> >  common/quic_family_tmpl.c |  4 +++-
> >  2 files changed, 17 insertions(+), 7 deletions(-)
> > 
> > diff --git a/common/quic.c b/common/quic.c
> > index 12e9b0b..c9c9df4 100644
> > --- a/common/quic.c
> > +++ b/common/quic.c
> > @@ -409,8 +409,10 @@ static inline void encode(Encoder *encoder, unsigned int
> > word, unsigned int len)
> >  {
> >      int delta;
> >
> > -    spice_assert(len > 0 && len < 32);
> > -    spice_assert(!(word & ~bppmask[len]));
> > +    if (spice_extra_checks) {
> > +        spice_assert(len > 0 && len < 32);
> > +        spice_assert(!(word & ~bppmask[len]));
> > +    }
> >      if ((delta = ((int)encoder->io_available_bits - len)) >= 0) {
> >          encoder->io_available_bits = delta;
> >          encoder->io_word |= word << encoder->io_available_bits;
> > @@ -422,8 +424,10 @@ static inline void encode(Encoder *encoder, unsigned int
> > word, unsigned int len)
> >      encoder->io_available_bits = 32 - delta;
> >      encoder->io_word = word << encoder->io_available_bits;
> >  
> > -    spice_assert(encoder->io_available_bits < 32);
> > -    spice_assert((encoder->io_word & bppmask[encoder->io_available_bits]) ==
> > 0);
> > +    if (spice_extra_checks) {
> > +        spice_assert(encoder->io_available_bits < 32);
> > +        spice_assert((encoder->io_word &
> > bppmask[encoder->io_available_bits]) == 0);
> > +    }
> >  }
> >  
> >  static inline void encode_32(Encoder *encoder, unsigned int word)
> > @@ -456,7 +460,9 @@ static inline void read_io_word(Encoder *encoder)
> >          __read_io_word_ptr(encoder); //disable inline optimizations
> >          return;
> >      }
> > -    spice_assert(encoder->io_now < encoder->io_end);
> > +    if (spice_extra_checks) {
> > +        spice_assert(encoder->io_now < encoder->io_end);
> > +    }
> >      encoder->io_next_word = GUINT32_FROM_LE(*(encoder->io_now++));
> >  }
> >  
> > @@ -464,7 +470,9 @@ static inline void decode_eatbits(Encoder *encoder, int
> > len)
> >  {
> >      int delta;
> >  
> > -    spice_assert(len > 0 && len < 32);
> > +    if (spice_extra_checks) {
> > +        spice_assert(len > 0 && len < 32);
> > +    }
> >      encoder->io_word <<= len;
> >  
> >      if ((delta = ((int)encoder->io_available_bits - len)) >= 0) {
> > diff --git a/common/quic_family_tmpl.c b/common/quic_family_tmpl.c
> > index 9450f44..cda15bd 100644
> > --- a/common/quic_family_tmpl.c
> > +++ b/common/quic_family_tmpl.c
> > @@ -105,7 +105,9 @@ static void FNAME(update_model)(CommonState *state,
> > s_bucket * const bucket,
> >  
> >  static s_bucket *FNAME(find_bucket)(Channel *channel, const unsigned int
> >  val)
> >  {
> > -    spice_assert(val < (0x1U << BPC));
> > +    if (spice_extra_checks) {
> > +        spice_assert(val < (0x1U << BPC));
> > +    }
> >  
> >      return channel->_buckets_ptrs[val];
> >  }
> _______________________________________________
> Spice-devel mailing list
> Spice-devel at lists.freedesktop.org
> https://lists.freedesktop.org/mailman/listinfo/spice-devel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.freedesktop.org/archives/spice-devel/attachments/20180531/f951a6cf/attachment-0001.sig>