Re: [PATCH V2] trace-cmd: add option to group like comms for profile

[Steven Rostedt <rostedt@goodmis.org>]

On Thu, 21 May 2015 13:30:08 -0400
Josef Bacik <jbacik@fb.com> wrote:


> V1->V2:
> -renamed the option to --by-comm, added it to trace-cmd report --profile as well
> -fixed up the string hash

Or break it ;-)

> -changed it to merge all events after the fact so it's less error prone

> diff --git a/trace-hash-local.h b/trace-hash-local.h
> index 997b11c..eaeeaaf 100644
> --- a/trace-hash-local.h
> +++ b/trace-hash-local.h
> @@ -48,4 +48,13 @@ static inline unsigned int trace_hash(int val)
>  	return hash;
>  }
>  
> +static inline unsigned int trace_hash_str(char *str)
> +{
> +	int val = 0;
> +	int i;
> +
> +	for (i = 0; str[i]; i++)
> +		val += ((int)str[i]) << (i & 0xffffff);
> +	return trace_hash(val);
> +}
>

I need to clean out my medicine cabinet and remove all the expired
meds. Because I was definitely taking something nasty when I
recommended (i & 0xffffff)!

When i is greater than 32 (which is less than 0xffffff) it will
overflow the addition. What I wanted was that we don't shift more than
24 bits. Where 2 ** 24 - 1 == 0xffffff.

That should be:

	val += ((int)str[i]) << (i % 25);

my bad :-/


To avoid the slow '%', I'll just use '& 0xf', as shifting it 16 times is
enough for this algorithm.

No need to send a new patch, I'll fix it, as it was my brain
fart. I'll review the rest of this patch too and apply it if nothing
sticks out.

Thanks!

-- Steve