Re: [devel] [SCM] packages/rpm: heads/maint

Re: [devel] [SCM] packages/rpm: heads/maint

[Kirill A. Shutemov]

On Fri, Dec 10, 2010 at 01:49:13AM +0300, Alexey M. Tourbin wrote:
> Update of /people/at/packages/rpm.git
> 
> Changes statistics since `4.0.4-alt100.6-3-gc6b45a0' follows:
>  lib/set.c |   65 ++++++++++++++++++++++++++++++++++++------------------------
>  1 files changed, 39 insertions(+), 26 deletions(-)
> 
> Changelog since `4.0.4-alt100.6-3-gc6b45a0' follows:
> commit 6c22ffb66945498de40c7ebb9b48e426fd32321d
> Author: Alexey Tourbin <at@altlinux.ru>
> Date:   Fri Dec 10 02:29:52 2010 +0300
> 
>     set.c: major decode_base62 improvement
>     
>     Kirill's changes still leave some room for optimization.
>     I'm not yet sure if I should revert his changes and apply more
>     aggressive optimizations techniques.
>     
>     Update: callgrind shows major improvement, but the improvement
>     is hardly noticeable when user time gets measured.
> 
> Full diff since `4.0.4-alt100.6-3-gc6b45a0' follows:
> diff --git a/lib/set.c b/lib/set.c
> index 1375a29..b81487f 100644
> --- a/lib/set.c
> +++ b/lib/set.c
> @@ -194,31 +194,39 @@ const int char_to_num[] = {
>      -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1, -1,
>  };
>  
> -static inline
> -void putbits(unsigned long *bitmap, int *offset, unsigned long c, int nbits)
> -{
> -    int quot, rem;
> -
> -    assert(!(c & ~MASK(nbits)));
> -
> -    quot = *offset / BITS_PER_LONG;
> -    rem = *offset % BITS_PER_LONG;
> -
> -    bitmap[quot] |= c << rem;
> -    c >>= BITS_PER_LONG - rem;
> -
> -    if (nbits + rem > (int) BITS_PER_LONG)
> -	bitmap[quot + 1] = c;
> -
> -    *offset += nbits;
> -}
> -
>  /* Main base62 decoding routine: unpack base62 string into bitmap. */
>  static
>  int decode_base62(unsigned long *bitmap, const char *base62)
>  {
> -    int offset = 0;
>      int c;
> +    unsigned long *bitmap_start = bitmap;
> +    unsigned long reg = 0;
> +    int regfill = 0;
> +
> +    inline
> +    void put_bits(int bitc, unsigned long num)
> +    {
> +	reg |= (num << regfill);
> +	regfill += bitc;
> +	if (regfill < sizeof(reg) * 8)
> +	    return;
> +	*bitmap++ = reg;
> +	regfill -= sizeof(reg) * 8;
> +	switch (regfill) {
> +	case 0:
> +	    reg = 0;
> +	    break;
> +	case 2:
> +	    reg = (num >> (bitc - 2));
> +	    break;
> +	case 4:
> +	    reg = (num >> (bitc - 4));
> +	    break;
> +	default:
> +	    assert(0);
> +	    break;
> +	}
> +    }

Please, do not use nested functions. I'm going to try to push set-versions
to rpm.org. C99 compatibility is important.

-- 
 Kirill A. Shutemov

Re: [devel] [SCM] packages/rpm: heads/maint

[Michael Shigorin]

On Fri, Dec 10, 2010 at 10:54:41AM +0200, Kirill A. Shutemov wrote:
> Please, do not use nested functions. I'm going to try to push
> set-versions to rpm.org. C99 compatibility is important.

Насколько понимаю, по крайней мере не стоит менять используемый
C89 без более веских на то причин, чем "мне так больше нравится".
Особенно если у глубокого форка есть надежда на хотя бы частичный
мерж.

-- 
 ---- WBR, Michael Shigorin <mike@altlinux.ru>
  ------ Linux.Kiev http://www.linux.kiev.ua/