Re: [patch] Latency Tracer, voluntary-preempt-2.6.8-rc4-O6

[Ingo Molnar <mingo@elte.hu>]

* Paulo Marques <pmarques@grupopie.com> wrote:

> >yeah. Maybe someone will find the time to improve the algorithm. But
> >it's not a highprio thing.
> 
> Well, I found some time and decided to give it a go :)

great, your patch is looking really good!

> The original algorithm took, on average, 1340us per lookup on my P4
> 2.8GHz. The compile settings for the test are not the same on the
> kernel, so this can be only compared against other results from the
> same setup.

ouch. I consider fixing this a quality of implementation issue.

> With the attached patch it takes 14us per lookup. This is almost a
> 100x improvement.

wow! I have tried your patch and /proc/latency_trace now produces
instantaneous output.

> There are still a few issues with this approach. The biggest issue is
> that this is clearly a speed/space trade-off, and maybe we don't want
> to waste the space on a code path that is not supposed to be "hot". If
> this is the case, I can make a smaller patch, that fixes just the name
> "decompression" strcpy's.

your patch doesnt add all that much of code. It adds 288 bytes to .text
and 64 bytes to .data. A typical .config generates 180K of compressed
kallsyms data (with !KALLSYMS_ALL), so your patch increases the kallsyms
overhead by a mere 0.2%. So it's really not an issue - especially since
kallsyms can be disabled in .config. 

i've put your patch into the -O8 patch:

 http://redhat.com/~mingo/voluntary-preempt/voluntary-preempt-2.6.8-rc4-O8

i'd strongly recommend merging this patch upstream as well.

> Just one side note: gcc gives a warning about 2 variables that might
> be used before initialization. I know they are not, and it didn't seem
> a good idea to put extra code just to shut up gcc. What is the
> standard way to convince gcc that those vars are ok?

the standard way is to add the extra initializers. The gcc folks feel
that those rare cases where gcc gets it wrong justify the benefit of
catching lots of real bugs. I've added the extra initialization to -O8.

	Ingo