lkml.org 
[lkml]   [2009]   [Jun]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH -tip] perf_counter tools: shorten names for events

* Jaswinder Singh Rajput <jaswinder@kernel.org> wrote:

> After :
>
> Performance counter stats for 'ls -lR /usr/include/':
>
> 283542921 dL1-loads (scaled from 23.28%)
> 1848314 dL1-load-misses (scaled from 22.94%)
> 168963 dL1-stores (scaled from 22.94%)
> 739249 dL1-prefetches (scaled from 22.45%)
> 501021 dL1-prefetch-misses (scaled from 22.25%)
> 275037259 iL1-loads (scaled from 23.40%)
> 6030825 iL1-load-misses (scaled from 23.26%)
> 166760 iL1-prefetches (scaled from 24.31%)
> 7224781 LLC-loads (scaled from 24.76%)
> 821097 LLC-load-misses (scaled from 24.07%)
> 7070549 LLC-stores (scaled from 24.45%)
> 251586242 dTLB-loads (scaled from 24.65%)
> 5127780 dTLB-load-misses (scaled from 23.99%)
> 276782014 iTLB-loads (scaled from 23.77%)
> 16787 iTLB-load-misses (scaled from 23.72%)
> 123408502 branches (scaled from 22.88%)
> 5843856 branch-misses (scaled from 22.87%)
>
> 1.417039891 seconds time elapsed.

ok, this output looks pretty good and intuitive to me (please
integrate suggestions from Thomas), but the patch itself needs
another iteration i think:

> static char *hw_cache[][MAX_ALIASES] = {
> - { "L1-data", "l1-d", "l1d" },
> - { "L1-instruction", "l1-i", "l1i" },
> - { "L2", "l2" },
> - { "Data-TLB", "dtlb", "d-tlb" },
> - { "Instruction-TLB", "itlb", "i-tlb" },
> - { "Branch", "bpu" , "btb", "bpc" },
> + { "dL1", "L1-d", "l1d", },
> + { "iL1", "L1-i", "l1i", },
> + { "LLC", "L2", },
> + { "dTLB", "d-tlb", },
> + { "iTLB", "i-tlb", },
> + { "branch", "branches", "bpu", "btb", "bpc", },
> };
>
> static char *hw_cache_op[][MAX_ALIASES] = {
> - { "Load", "read" },
> - { "Store", "write" },
> - { "Prefetch", "speculative-read", "speculative-load" },
> + { "load", "loads", "read", },
> + { "store", "stores", "write", },
> + { "prefetch", "prefetches", "speculative-read", "speculative-load", },
> };
>
> static char *hw_cache_result[][MAX_ALIASES] = {
> - { "Reference", "ops", "access" },
> - { "Miss" },
> + { "refs", "ops", "access", },
> + { "misses", "miss", },
> };
>
> char *event_name(int counter)
> @@ -123,10 +123,25 @@ char *event_name(int counter)
> if (cache_result > PERF_COUNT_HW_CACHE_RESULT_MAX)
> return "unknown-ext-hardware-cache-result";
>
> - sprintf(name, "%s-Cache-%s-%ses",
> - hw_cache[cache_type][0],
> - hw_cache_op[cache_op][0],
> - hw_cache_result[cache_result][0]);
> + /*
> + * special handling for branches
> + * we are only interested in BPU, READ
> + */
> + if (cache_type == PERF_COUNT_HW_CACHE_BPU && cache_op)
> + return "unknown";
> + else if (cache_type == PERF_COUNT_HW_CACHE_BPU) {
> + if (cache_result)
> + sprintf(name, "%s-%s", hw_cache[cache_type][0],
> + hw_cache_result[cache_result][0]);
> + else
> + sprintf(name, "%s", hw_cache[cache_type][1]);
> + } else if (cache_result)
> + sprintf(name, "%s-%s-%s", hw_cache[cache_type][0],
> + hw_cache_op[cache_op][0],
> + hw_cache_result[cache_result][0]);
> + else
> + sprintf(name, "%s-%s", hw_cache[cache_type][0],
> + hw_cache_op[cache_op][1]);
>
> return name;

Firstly, please run your patches through checkpatch - it will report
a real problem in your patch.

Secondly, this special-casing of the BPU isnt very clean in this
form. The BPU isnt 'special' because it deals with instructions -
it's special because it's for all practical purposes read-only.

So we should extend our table with a read-only flag, and the BPU and
the iTLB should be listed as read-only. (iTLB-store-miss is another
thing that makes no sense) For those we should skip the 'store'
bits.

That way the generic code does not have this special-case wart
dependent on PERF_COUNT_HW_CACHE_BPU.

Ingo


\
 
 \ /
  Last update: 2009-06-25 11:37    [W:1.736 / U:0.192 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site