lkml.org 
[lkml]   [2015]   [Apr]   [22]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[PATCH 08/10] perf hists browser: Split popup menu actions - part 2
    Date
    Currently perf_evsel__hists_browse() function spins on a huge loop and
    handles many key actions. Since it's hard to read and modify, let's
    split it out into small helper functions.

    The add_XXX_opt() functions are to register popup menu item on the
    selected entry. When it adds an item, it also saves related data into
    struct popup_action and returns 1 so that it can increase the number of
    items (nr_options).

    With this change, we can simplify the code just to call selected
    callback function without considering various conditions. A callback
    function named do_XXX is called with saved data when the item is
    selected by user.

    No functional change intended.

    Signed-off-by: Namhyung Kim <namhyung@kernel.org>
    ---
    tools/perf/ui/browsers/hists.c | 354 +++++++++++++++++++++++++----------------
    1 file changed, 214 insertions(+), 140 deletions(-)

    diff --git a/tools/perf/ui/browsers/hists.c b/tools/perf/ui/browsers/hists.c
    index 7d88a1cdf04b..9bd7b38de64c 100644
    --- a/tools/perf/ui/browsers/hists.c
    +++ b/tools/perf/ui/browsers/hists.c
    @@ -1402,8 +1402,16 @@ close_file_and_continue:
    return ret;
    }

    +struct popup_action {
    + struct thread *thread;
    + struct dso *dso;
    + struct map_symbol ms;
    +
    + int (*fn)(struct hist_browser *browser, struct popup_action *act);
    +};
    +
    static int
    -do_annotate(struct hist_browser *browser, struct map_symbol *ms)
    +do_annotate(struct hist_browser *browser, struct popup_action *act)
    {
    struct perf_evsel *evsel;
    struct annotation *notes;
    @@ -1413,12 +1421,12 @@ do_annotate(struct hist_browser *browser, struct map_symbol *ms)
    if (!objdump_path && perf_session_env__lookup_objdump(browser->env))
    return 0;

    - notes = symbol__annotation(ms->sym);
    + notes = symbol__annotation(act->ms.sym);
    if (!notes->src)
    return 0;

    evsel = hists_to_evsel(browser->hists);
    - err = map_symbol__tui_annotate(ms, evsel, browser->hbt);
    + err = map_symbol__tui_annotate(&act->ms, evsel, browser->hbt);
    he = hist_browser__selected_entry(browser);
    /*
    * offer option to annotate the other branch source or target
    @@ -1434,8 +1442,27 @@ do_annotate(struct hist_browser *browser, struct map_symbol *ms)
    }

    static int
    -do_zoom_thread(struct hist_browser *browser, struct thread *thread)
    +add_annotate_opt(struct hist_browser *browser __maybe_unused,
    + struct popup_action *act, char **optstr,
    + struct map *map, struct symbol *sym)
    {
    + if (sym == NULL || map->dso->annotate_warned)
    + return 0;
    +
    + if (asprintf(optstr, "Annotate %s", sym->name) < 0)
    + return 0;
    +
    + act->ms.map = map;
    + act->ms.sym = sym;
    + act->fn = do_annotate;
    + return 1;
    +}
    +
    +static int
    +do_zoom_thread(struct hist_browser *browser, struct popup_action *act)
    +{
    + struct thread *thread = act->thread;
    +
    if (browser->hists->thread_filter) {
    pstack__remove(browser->pstack, &browser->hists->thread_filter);
    perf_hpp__set_elide(HISTC_THREAD, false);
    @@ -1456,8 +1483,28 @@ do_zoom_thread(struct hist_browser *browser, struct thread *thread)
    }

    static int
    -do_zoom_dso(struct hist_browser *browser, struct dso *dso)
    +add_thread_opt(struct hist_browser *browser, struct popup_action *act,
    + char **optstr, struct thread *thread)
    +{
    + if (thread == NULL)
    + return 0;
    +
    + if (asprintf(optstr, "Zoom %s %s(%d) thread",
    + browser->hists->thread_filter ? "out of" : "into",
    + thread->comm_set ? thread__comm_str(thread) : "",
    + thread->tid) < 0)
    + return 0;
    +
    + act->thread = thread;
    + act->fn = do_zoom_thread;
    + return 1;
    +}
    +
    +static int
    +do_zoom_dso(struct hist_browser *browser, struct popup_action *act)
    {
    + struct dso *dso = act->dso;
    +
    if (browser->hists->dso_filter) {
    pstack__remove(browser->pstack, &browser->hists->dso_filter);
    perf_hpp__set_elide(HISTC_DSO, false);
    @@ -1479,25 +1526,58 @@ do_zoom_dso(struct hist_browser *browser, struct dso *dso)
    }

    static int
    -do_browse_map(struct hist_browser *browser __maybe_unused, struct map *map)
    +add_dso_opt(struct hist_browser *browser, struct popup_action *act,
    + char **optstr, struct dso *dso)
    {
    - map__browse(map);
    + if (dso == NULL)
    + return 0;
    +
    + if (asprintf(optstr, "Zoom %s %s DSO",
    + browser->hists->dso_filter ? "out of" : "into",
    + dso->kernel ? "the Kernel" : dso->short_name) < 0)
    + return 0;
    +
    + act->dso = dso;
    + act->fn = do_zoom_dso;
    + return 1;
    +}
    +
    +static int
    +do_browse_map(struct hist_browser *browser __maybe_unused,
    + struct popup_action *act)
    +{
    + map__browse(act->ms.map);
    return 0;
    }

    static int
    +add_map_opt(struct hist_browser *browser __maybe_unused,
    + struct popup_action *act, char **optstr, struct map *map)
    +{
    + if (map == NULL)
    + return 0;
    +
    + if (asprintf(optstr, "Browse map details") < 0)
    + return 0;
    +
    + act->ms.map = map;
    + act->fn = do_browse_map;
    + return 1;
    +}
    +
    +static int
    do_run_script(struct hist_browser *browser __maybe_unused,
    - struct thread *thread, struct symbol *sym)
    + struct popup_action *act)
    {
    char script_opt[64];
    memset(script_opt, 0, sizeof(script_opt));

    - if (thread) {
    + if (act->thread) {
    scnprintf(script_opt, sizeof(script_opt), " -c %s ",
    - thread__comm_str(thread));
    - } else if (sym) {
    + thread__comm_str(act->thread));
    + } else if (act->ms.sym) {
    scnprintf(script_opt, sizeof(script_opt), " -S %s ",
    - sym->name);
    + act->ms.sym->name);
    }

    script_browse(script_opt);
    @@ -1505,17 +1585,74 @@ do_run_script(struct hist_browser *browser __maybe_unused,
    }

    static int
    -do_switch_data(struct hist_browser *browser __maybe_unused, int key)
    +add_script_opt(struct hist_browser *browser __maybe_unused,
    + struct popup_action *act, char **optstr,
    + struct thread *thread, struct symbol *sym)
    +{
    + if (thread) {
    + if (asprintf(optstr, "Run scripts for samples of thread [%s]",
    + thread__comm_str(thread)) < 0)
    + return 0;
    + } else if (sym) {
    + if (asprintf(optstr, "Run scripts for samples of symbol [%s]",
    + sym->name) < 0)
    + return 0;
    + } else {
    + if (asprintf(optstr, "Run scripts for all samples") < 0)
    + return 0;
    + }
    +
    + act->thread = thread;
    + act->ms.sym = sym;
    + act->fn = do_run_script;
    + return 1;
    +}
    +
    +static int
    +do_switch_data(struct hist_browser *browser __maybe_unused,
    + struct popup_action *act __maybe_unused)
    {
    if (switch_data_file()) {
    ui__warning("Won't switch the data files due to\n"
    "no valid data file get selected!\n");
    - return key;
    + return 0;
    }

    return K_SWITCH_INPUT_DATA;
    }

    +static int
    +add_switch_opt(struct hist_browser *browser,
    + struct popup_action *act, char **optstr)
    +{
    + if (!is_report_browser(browser->hbt))
    + return 0;
    +
    + if (asprintf(optstr, "Switch to another data file in PWD") < 0)
    + return 0;
    +
    + act->fn = do_switch_data;
    + return 1;
    +}
    +
    +static int
    +do_exit_browser(struct hist_browser *browser __maybe_unused,
    + struct popup_action *act __maybe_unused)
    +{
    + return 0;
    +}
    +
    +static int
    +add_exit_opt(struct hist_browser *browser __maybe_unused,
    + struct popup_action *act, char **optstr)
    +{
    + if (asprintf(optstr, "Exit") < 0)
    + return 0;
    +
    + act->fn = do_exit_browser;
    + return 1;
    +}
    +
    static void hist_browser__update_nr_entries(struct hist_browser *hb)
    {
    u64 nr_entries = 0;
    @@ -1546,6 +1683,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
    struct branch_info *bi;
    #define MAX_OPTIONS 16
    char *options[MAX_OPTIONS];
    + struct popup_action actions[MAX_OPTIONS];
    int nr_options = 0;
    int key = -1;
    char buf[64];
    @@ -1600,6 +1738,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
    ui_helpline__push(helpline);

    memset(options, 0, sizeof(options));
    + memset(actions, 0, sizeof(actions));

    perf_hpp__for_each_format(fmt)
    perf_hpp__reset_width(fmt, hists);
    @@ -1610,12 +1749,7 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
    while (1) {
    struct thread *thread = NULL;
    struct dso *dso = NULL;
    - struct map_symbol ms;
    - int choice = 0,
    - annotate = -2, zoom_dso = -2, zoom_thread = -2,
    - annotate_f = -2, annotate_t = -2, browse_map = -2;
    - int scripts_comm = -2, scripts_symbol = -2,
    - scripts_all = -2, switch_data = -2;
    + int choice = 0;

    nr_options = 0;

    @@ -1648,22 +1782,23 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
    browser->selection->map->dso->annotate_warned)
    continue;

    - ms.map = browser->selection->map;
    - ms.sym = browser->selection->sym;
    -
    - do_annotate(browser, &ms);
    + actions->ms.map = browser->selection->map;
    + actions->ms.sym = browser->selection->sym;
    + do_annotate(browser, actions);
    continue;
    case 'P':
    hist_browser__dump(browser);
    continue;
    case 'd':
    - do_zoom_dso(browser, dso);
    + actions->dso = dso;
    + do_zoom_dso(browser, actions);
    continue;
    case 'V':
    browser->show_dso = !browser->show_dso;
    continue;
    case 't':
    - do_zoom_thread(browser, thread);
    + actions->thread = thread;
    + do_zoom_thread(browser, actions);
    continue;
    case '/':
    if (ui_browser__input_window("Symbol to show",
    @@ -1676,12 +1811,15 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
    }
    continue;
    case 'r':
    - if (is_report_browser(hbt))
    - do_run_script(browser, NULL, NULL);
    + if (is_report_browser(hbt)) {
    + actions->thread = NULL;
    + actions->ms.sym = NULL;
    + do_run_script(browser, actions);
    + }
    continue;
    case 's':
    if (is_report_browser(hbt)) {
    - key = do_switch_data(browser, key);
    + key = do_switch_data(browser, actions);
    if (key == K_SWITCH_INPUT_DATA)
    goto out_free_stack;
    }
    @@ -1762,129 +1900,65 @@ static int perf_evsel__hists_browse(struct perf_evsel *evsel, int nr_events,
    if (bi == NULL)
    goto skip_annotation;

    - if (bi->from.sym != NULL &&
    - !bi->from.map->dso->annotate_warned &&
    - asprintf(&options[nr_options], "Annotate %s", bi->from.sym->name) > 0) {
    - annotate_f = nr_options++;
    - }
    -
    - if (bi->to.sym != NULL &&
    - !bi->to.map->dso->annotate_warned &&
    - (bi->to.sym != bi->from.sym ||
    - bi->to.map->dso != bi->from.map->dso) &&
    - asprintf(&options[nr_options], "Annotate %s", bi->to.sym->name) > 0) {
    - annotate_t = nr_options++;
    - }
    + nr_options += add_annotate_opt(browser,
    + &actions[nr_options],
    + &options[nr_options],
    + bi->from.map,
    + bi->from.sym);
    + if (bi->to.sym != bi->from.sym)
    + nr_options += add_annotate_opt(browser,
    + &actions[nr_options],
    + &options[nr_options],
    + bi->to.map,
    + bi->to.sym);
    } else {
    - if (browser->selection->sym != NULL &&
    - !browser->selection->map->dso->annotate_warned) {
    - struct annotation *notes;
    -
    - notes = symbol__annotation(browser->selection->sym);
    -
    - if (notes->src &&
    - asprintf(&options[nr_options], "Annotate %s",
    - browser->selection->sym->name) > 0) {
    - annotate = nr_options++;
    - }
    - }
    + nr_options += add_annotate_opt(browser,
    + &actions[nr_options],
    + &options[nr_options],
    + browser->selection->map,
    + browser->selection->sym);
    }
    skip_annotation:
    - if (thread != NULL &&
    - asprintf(&options[nr_options], "Zoom %s %s(%d) thread",
    - (browser->hists->thread_filter ? "out of" : "into"),
    - (thread->comm_set ? thread__comm_str(thread) : ""),
    - thread->tid) > 0)
    - zoom_thread = nr_options++;
    -
    - if (dso != NULL &&
    - asprintf(&options[nr_options], "Zoom %s %s DSO",
    - (browser->hists->dso_filter ? "out of" : "into"),
    - (dso->kernel ? "the Kernel" : dso->short_name)) > 0)
    - zoom_dso = nr_options++;
    -
    - if (browser->selection != NULL &&
    - browser->selection->map != NULL &&
    - asprintf(&options[nr_options], "Browse map details") > 0)
    - browse_map = nr_options++;
    + nr_options += add_thread_opt(browser, &actions[nr_options],
    + &options[nr_options], thread);
    + nr_options += add_dso_opt(browser, &actions[nr_options],
    + &options[nr_options], dso);
    + nr_options += add_map_opt(browser, &actions[nr_options],
    + &options[nr_options],
    + browser->selection->map);

    /* perf script support */
    if (browser->he_selection) {
    - struct symbol *sym;
    -
    - if (asprintf(&options[nr_options], "Run scripts for samples of thread [%s]",
    - thread__comm_str(browser->he_selection->thread)) > 0)
    - scripts_comm = nr_options++;
    -
    - sym = browser->he_selection->ms.sym;
    - if (sym && sym->namelen &&
    - asprintf(&options[nr_options], "Run scripts for samples of symbol [%s]",
    - sym->name) > 0)
    - scripts_symbol = nr_options++;
    + nr_options += add_script_opt(browser,
    + &actions[nr_options],
    + &options[nr_options],
    + thread, NULL);
    + nr_options += add_script_opt(browser,
    + &actions[nr_options],
    + &options[nr_options],
    + NULL, browser->selection->sym);
    }
    -
    - if (asprintf(&options[nr_options], "Run scripts for all samples") > 0)
    - scripts_all = nr_options++;
    -
    - if (is_report_browser(hbt) && asprintf(&options[nr_options],
    - "Switch to another data file in PWD") > 0)
    - switch_data = nr_options++;
    + nr_options += add_script_opt(browser, &actions[nr_options],
    + &options[nr_options], NULL, NULL);
    + nr_options += add_switch_opt(browser, &actions[nr_options],
    + &options[nr_options]);
    add_exit_option:
    - if (asprintf(&options[nr_options], "Exit") > 0)
    - nr_options++;
    -retry_popup_menu:
    - choice = ui__popup_menu(nr_options, options);
    + nr_options += add_exit_opt(browser, &actions[nr_options],
    + &options[nr_options]);

    - if (choice == nr_options - 1)
    - break;
    -
    - if (choice == -1) {
    - free_popup_options(options, nr_options - 1);
    - continue;
    - }
    -
    - if (choice == annotate || choice == annotate_t || choice == annotate_f) {
    - struct hist_entry *he;
    + do {
    + struct popup_action *act;

    - he = hist_browser__selected_entry(browser);
    - if (he == NULL)
    - continue;
    + choice = ui__popup_menu(nr_options, options);
    + if (choice == -1 || choice >= nr_options)
    + break;

    - if (choice == annotate_f) {
    - ms.map = he->branch_info->from.map;
    - ms.sym = he->branch_info->from.sym;
    - } else if (choice == annotate_t) {
    - ms.map = he->branch_info->to.map;
    - ms.sym = he->branch_info->to.sym;
    - } else {
    - ms = *browser->selection;
    - }
    + act = &actions[choice];
    + key = act->fn(browser, act);
    + } while (key == 1);

    - if (do_annotate(browser, &ms) == 1)
    - goto retry_popup_menu;
    - } else if (choice == browse_map) {
    - do_browse_map(browser, browser->selection->map);
    - } else if (choice == zoom_dso) {
    - do_zoom_dso(browser, dso);
    - } else if (choice == zoom_thread) {
    - do_zoom_thread(browser, thread);
    - }
    - /* perf scripts support */
    - else if (choice == scripts_all || choice == scripts_comm ||
    - choice == scripts_symbol) {
    - if (choice == scripts_comm)
    - do_run_script(browser, browser->he_selection->thread, NULL);
    - if (choice == scripts_symbol)
    - do_run_script(browser, NULL, browser->he_selection->ms.sym);
    - if (choice == scripts_all)
    - do_run_script(browser, NULL, NULL);
    - }
    - /* Switch to another data file */
    - else if (choice == switch_data) {
    - key = do_switch_data(browser, key);
    - if (key == K_SWITCH_INPUT_DATA)
    - break;
    - }
    + if (key == K_SWITCH_INPUT_DATA)
    + break;
    }
    out_free_stack:
    pstack__delete(browser->pstack);
    --
    2.3.5


    \
     
     \ /
      Last update: 2015-04-22 09:41    [W:4.153 / U:0.024 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site