lkml.org 
[lkml]   [2012]   [Mar]   [5]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    Date
    From
    Subject[RFC] Sort inclusive

    Some of our users have been asking for the ability to sort samples by
    inclusive time (time spent in the function + all of the callees in the
    callgraph).

    While something like this is possible via tools such as:

    perf script | gprof2dot.py -f perf

    a simpler, text only tool seems desirable.

    The attached patch seems to work ok on some test programs in
    order=caller mode. The idea is simple:

    For each input callchain such as:

    main -> a -> b -> c

    We create histogram entries for:

    main -> a -> b -> c (original callchain)
    a -> b -> c (inclusive callchain)
    b -> c (inclusive callchain)
    c (inclusive callchain)

    When computing hists->stats.total_period, the patch ignores the inclusive
    callchains and considers only the original callchain.

    However, this doesn't work very well for order=callee, mainly because
    the patch doesn't compute total_period correctly. The number of samples
    look ok (and should be the same as order=caller) for each symbol.

    If the input stream had:

    c <- b <- a <- main

    everything would've worked just fine. But when I step through the code,
    I see:

    some <- kernel <- funcs <- c <- b <- a <- main (original)
    kernel <- funcs <- c <- b <- a <- main (inclusive)
    funcs <- c <- b <- a <- main (inclusive)
    c <- b <- a <- main (inclusive <- but should be the original callchain)
    b <- a <- main (inclusive)
    a <- main (inclusive)
    main (inclusive)

    I tried a couple of hacks (eg: comparing event->ip.ip to al->addr) to
    tell the original callchain from inclusive ones, so the inclusive chains
    can be used for histogram display/sorting, but ignored for other
    purposes.

    I hope others find --sort inclusive functionality useful. Any comments
    on how to solve the order=callee case would be great. I suspect this has
    something to do with histogram output collapsing/resorting.

    -Arun

    commit 29e659f0ca2041f2f1681a0072739165220d7c64
    Author: Arun Sharma <asharma@fb.com>
    Date: Wed Feb 29 21:40:47 2012 +0000

    perf: Add a new sort order: SORT_INCLUSIVE

    Each entry that used to get added once to the histogram, now is added
    chain->nr times, each time with one less entry in the
    callchain.

    This will result in a non-leaf function that appears in a lot of
    samples to get a histogram entry with lots of hits.

    The user can then drill down into the callchains of functions that
    have high inclusive times.

    Signed-off-by: Arun Sharma <asharma@fb.com>

    diff --git a/builtin-report.c b/builtin-report.c
    index 25d34d4..4bcd169 100644
    --- a/builtin-report.c
    +++ b/builtin-report.c
    @@ -60,7 +60,7 @@ static int perf_evsel__add_hist_entry(struct perf_evsel *evsel,
    {
    struct symbol *parent = NULL;
    int err = 0;
    - struct hist_entry *he;
    + struct hist_entry *he = NULL;

    if ((sort__has_parent || symbol_conf.use_callchain) && sample->callchain) {
    err = machine__resolve_callchain(machine, evsel, al->thread,
    @@ -69,16 +69,47 @@ static int perf_evsel__add_hist_entry(struct perf_evsel *evsel,
    return err;
    }

    - he = __hists__add_entry(&evsel->hists, al, parent, sample->period);
    - if (he == NULL)
    - return -ENOMEM;
    + if ((sort__first_dimension == SORT_INCLUSIVE) && (symbol_conf.use_callchain)) {
    + struct callchain_cursor *cursor = &evsel->hists.callchain_cursor;
    + struct callchain_cursor iter = *cursor;
    + struct callchain_cursor new_cursor = *cursor;
    + u64 i;
    +
    + iter.pos = 0;
    + iter.curr = iter.first;
    + for (i = 0; i < cursor->nr; i++) {
    + struct addr_location al_child = *al;
    +
    + err = callchain_get(&iter, &al_child);
    + if (err)
    + return err;
    + he = __hists__add_entry(&evsel->hists, &al_child, parent, sample->period);
    + if (he == NULL)
    + return -ENOMEM;
    +
    + new_cursor.first = iter.curr;
    + new_cursor.nr = cursor->nr - i;
    + if (i != 0)
    + he->inclusive = 1;
    + err = callchain_append(he->callchain,
    + &new_cursor,
    + sample->period);
    + if (err)
    + return err;
    + callchain_cursor_advance(&iter);
    + }
    + } else {
    + he = __hists__add_entry(&evsel->hists, al, parent, sample->period);
    + if (he == NULL)
    + return -ENOMEM;

    - if (symbol_conf.use_callchain) {
    - err = callchain_append(he->callchain,
    - &evsel->hists.callchain_cursor,
    - sample->period);
    - if (err)
    - return err;
    + if (symbol_conf.use_callchain) {
    + err = callchain_append(he->callchain,
    + &evsel->hists.callchain_cursor,
    + sample->period);
    + if (err)
    + return err;
    + }
    }
    /*
    * Only in the newt browser we are doing integrated annotation,
    @@ -595,6 +626,7 @@ int cmd_report(int argc, const char **argv, const char *prefix __used)
    sort_entry__setup_elide(&sort_dso, symbol_conf.dso_list, "dso", stdout);
    sort_entry__setup_elide(&sort_comm, symbol_conf.comm_list, "comm", stdout);
    sort_entry__setup_elide(&sort_sym, symbol_conf.sym_list, "symbol", stdout);
    + sort_entry__setup_elide(&sort_sym, symbol_conf.sym_list, "inclusive", stdout);

    return __cmd_report(&report);
    }
    diff --git a/util/callchain.c b/util/callchain.c
    index 9f7106a..aa4acde 100644
    --- a/util/callchain.c
    +++ b/util/callchain.c
    @@ -459,3 +459,17 @@ int callchain_cursor_append(struct callchain_cursor *cursor,

    return 0;
    }
    +
    +int callchain_get(struct callchain_cursor *cursor,
    + struct addr_location *al)
    +{
    + struct callchain_cursor_node *node = cursor->curr;
    +
    + if (node == NULL) return -1;
    +
    + al->map = node->map;
    + al->sym = node->sym;
    + al->addr = node->ip;
    +
    + return 0;
    +}
    diff --git a/util/callchain.h b/util/callchain.h
    index 7f9c0f1..dcff6ec 100644
    --- a/util/callchain.h
    +++ b/util/callchain.h
    @@ -103,9 +103,13 @@ int callchain_merge(struct callchain_cursor *cursor,

    struct ip_callchain;
    union perf_event;
    +struct addr_location;

    bool ip_callchain__valid(struct ip_callchain *chain,
    const union perf_event *event);
    +
    +int callchain_get(struct callchain_cursor *cursor, struct addr_location *al);
    +
    /*
    * Initialize a cursor before adding entries inside, but keep
    * the previously allocated entries as a cache.
    diff --git a/util/hist.c b/util/hist.c
    index 6f505d1..0eebe46 100644
    --- a/util/hist.c
    +++ b/util/hist.c
    @@ -174,6 +174,7 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template)
    he->ms.map->referenced = true;
    if (symbol_conf.use_callchain)
    callchain_init(he->callchain);
    + he->inclusive = false;
    }

    return he;
    @@ -181,7 +182,7 @@ static struct hist_entry *hist_entry__new(struct hist_entry *template)

    static void hists__inc_nr_entries(struct hists *hists, struct hist_entry *h)
    {
    - if (!h->filtered) {
    + if (!h->filtered && !h->inclusive) {
    hists__calc_col_len(hists, h);
    ++hists->nr_entries;
    hists->stats.total_period += h->period;
    diff --git a/util/sort.c b/util/sort.c
    index 16da30d..1440ad4 100644
    --- a/util/sort.c
    +++ b/util/sort.c
    @@ -259,6 +259,7 @@ static struct sort_dimension sort_dimensions[] = {
    { .name = "symbol", .entry = &sort_sym, },
    { .name = "parent", .entry = &sort_parent, },
    { .name = "cpu", .entry = &sort_cpu, },
    + { .name = "inclusive", .entry = &sort_sym, },
    };

    int sort_dimension__add(const char *tok)
    @@ -298,6 +299,8 @@ int sort_dimension__add(const char *tok)
    sort__first_dimension = SORT_DSO;
    else if (!strcmp(sd->name, "symbol"))
    sort__first_dimension = SORT_SYM;
    + else if (!strcmp(sd->name, "inclusive"))
    + sort__first_dimension = SORT_INCLUSIVE;
    else if (!strcmp(sd->name, "parent"))
    sort__first_dimension = SORT_PARENT;
    else if (!strcmp(sd->name, "cpu"))
    diff --git a/util/sort.h b/util/sort.h
    index 3f67ae3..2d35c11 100644
    --- a/util/sort.h
    +++ b/util/sort.h
    @@ -65,6 +65,7 @@ struct hist_entry {
    bool init_have_children;
    char level;
    bool used;
    + bool inclusive;
    u8 filtered;
    struct symbol *parent;
    union {
    @@ -82,6 +83,7 @@ enum sort_type {
    SORT_SYM,
    SORT_PARENT,
    SORT_CPU,
    + SORT_INCLUSIVE,
    };

    /*

    \
     
     \ /
      Last update: 2012-03-05 23:15    [W:0.031 / U:29.424 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site