From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S965115AbbD1Hj0 (ORCPT ); Tue, 28 Apr 2015 03:39:26 -0400 Received: from LGEMRELSE6Q.lge.com ([156.147.1.121]:50976 "EHLO lgemrelse6q.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932255AbbD1HjZ (ORCPT ); Tue, 28 Apr 2015 03:39:25 -0400 X-Original-SENDERIP: 10.177.220.203 X-Original-MAILFROM: namhyung@kernel.org Date: Tue, 28 Apr 2015 16:30:29 +0900 From: Namhyung Kim To: Arnaldo Carvalho de Melo Cc: Ingo Molnar , Peter Zijlstra , Jiri Olsa , LKML , David Ahern Subject: Re: [PATCH 10/10] perf tools: Move TUI-specific fields out of map_symbol Message-ID: <20150428073029.GA10833@sejong> References: <1429687101-4360-1-git-send-email-namhyung@kernel.org> <1429687101-4360-11-git-send-email-namhyung@kernel.org> <20150427172036.GE16849@kernel.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20150427172036.GE16849@kernel.org> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Hi Arnaldo, On Mon, Apr 27, 2015 at 02:20:36PM -0300, Arnaldo Carvalho de Melo wrote: > Em Wed, Apr 22, 2015 at 04:18:21PM +0900, Namhyung Kim escreveu: > > > @@ -139,24 +139,19 @@ static char tree__folded_sign(bool unfolded) > > return unfolded ? '-' : '+'; > > } > > > > -static char map_symbol__folded(const struct map_symbol *ms) > > -{ > > - return ms->has_children ? tree__folded_sign(ms->unfolded) : ' '; > > -} > > - > > So what is wrong with renaming the above function to > hist_entry__folded() instead of open coding it multiple times? Applied > all the other patches, thanks, Please see below.. The struct map_symbol__folded() was called from hist_entry__folded() and callchain_list__folded(). I just removed map_symbol__folded() since it does not contain the info anymore and open coded it just for the two callers. The real callers of these functions are not touched. > > > static char hist_entry__folded(const struct hist_entry *he) > > { > > - return map_symbol__folded(&he->ms); > > + return he->has_children ? tree__folded_sign(he->unfolded) : ' '; > > } > > > > static char callchain_list__folded(const struct callchain_list *cl) > > { > > - return map_symbol__folded(&cl->ms); > > + return cl->has_children ? tree__folded_sign(cl->unfolded) : ' '; > > } Here. Thanks, Namhyung > > > > -static void map_symbol__set_folding(struct map_symbol *ms, bool unfold) > > +static void callchain_list__set_folding(struct callchain_list *cl, bool unfold) > > { > > - ms->unfolded = unfold ? ms->has_children : false; > > + cl->unfolded = unfold ? cl->has_children : false; > > } > > > > static int callchain_node__count_rows_rb_tree(struct callchain_node *node) > > @@ -192,7 +187,7 @@ static int callchain_node__count_rows(struct callchain_node *node) > > > > list_for_each_entry(chain, &node->val, list) { > > ++n; > > - unfolded = chain->ms.unfolded; > > + unfolded = chain->unfolded; > > } > > > > if (unfolded) > > @@ -214,15 +209,15 @@ static int callchain__count_rows(struct rb_root *chain) > > return n; > > } > > > > -static bool map_symbol__toggle_fold(struct map_symbol *ms) > > +static bool hist_entry__toggle_fold(struct hist_entry *he) > > { > > - if (!ms) > > + if (!he) > > return false; > > > > - if (!ms->has_children) > > + if (!he->has_children) > > return false; > > > > - ms->unfolded = !ms->unfolded; > > + he->unfolded = !he->unfolded; > > return true; > > } > > > > @@ -238,10 +233,10 @@ static void callchain_node__init_have_children_rb_tree(struct callchain_node *no > > list_for_each_entry(chain, &child->val, list) { > > if (first) { > > first = false; > > - chain->ms.has_children = chain->list.next != &child->val || > > + chain->has_children = chain->list.next != &child->val || > > !RB_EMPTY_ROOT(&child->rb_root); > > } else > > - chain->ms.has_children = chain->list.next == &child->val && > > + chain->has_children = chain->list.next == &child->val && > > !RB_EMPTY_ROOT(&child->rb_root); > > } > > > > @@ -255,11 +250,11 @@ static void callchain_node__init_have_children(struct callchain_node *node, > > struct callchain_list *chain; > > > > chain = list_entry(node->val.next, struct callchain_list, list); > > - chain->ms.has_children = has_sibling; > > + chain->has_children = has_sibling; > > > > if (!list_empty(&node->val)) { > > chain = list_entry(node->val.prev, struct callchain_list, list); > > - chain->ms.has_children = !RB_EMPTY_ROOT(&node->rb_root); > > + chain->has_children = !RB_EMPTY_ROOT(&node->rb_root); > > } > > > > callchain_node__init_have_children_rb_tree(node); > > @@ -279,7 +274,7 @@ static void callchain__init_have_children(struct rb_root *root) > > static void hist_entry__init_have_children(struct hist_entry *he) > > { > > if (!he->init_have_children) { > > - he->ms.has_children = !RB_EMPTY_ROOT(&he->sorted_chain); > > + he->has_children = !RB_EMPTY_ROOT(&he->sorted_chain); > > callchain__init_have_children(&he->sorted_chain); > > he->init_have_children = true; > > } > > @@ -287,14 +282,14 @@ static void hist_entry__init_have_children(struct hist_entry *he) > > > > static bool hist_browser__toggle_fold(struct hist_browser *browser) > > { > > - if (map_symbol__toggle_fold(browser->selection)) { > > + if (hist_entry__toggle_fold(browser->he_selection)) { > > struct hist_entry *he = browser->he_selection; > > > > hist_entry__init_have_children(he); > > browser->b.nr_entries -= he->nr_rows; > > browser->nr_callchain_rows -= he->nr_rows; > > > > - if (he->ms.unfolded) > > + if (he->unfolded) > > he->nr_rows = callchain__count_rows(&he->sorted_chain); > > else > > he->nr_rows = 0; > > @@ -321,8 +316,8 @@ static int callchain_node__set_folding_rb_tree(struct callchain_node *node, bool > > > > list_for_each_entry(chain, &child->val, list) { > > ++n; > > - map_symbol__set_folding(&chain->ms, unfold); > > - has_children = chain->ms.has_children; > > + callchain_list__set_folding(chain, unfold); > > + has_children = chain->has_children; > > } > > > > if (has_children) > > @@ -340,8 +335,8 @@ static int callchain_node__set_folding(struct callchain_node *node, bool unfold) > > > > list_for_each_entry(chain, &node->val, list) { > > ++n; > > - map_symbol__set_folding(&chain->ms, unfold); > > - has_children = chain->ms.has_children; > > + callchain_list__set_folding(chain, unfold); > > + has_children = chain->has_children; > > } > > > > if (has_children) > > @@ -366,9 +361,9 @@ static int callchain__set_folding(struct rb_root *chain, bool unfold) > > static void hist_entry__set_folding(struct hist_entry *he, bool unfold) > > { > > hist_entry__init_have_children(he); > > - map_symbol__set_folding(&he->ms, unfold); > > + hist_entry__set_folding(he, unfold); > > > > - if (he->ms.has_children) { > > + if (he->has_children) { > > int n = callchain__set_folding(&he->sorted_chain, unfold); > > he->nr_rows = unfold ? n : 0; > > } else > > @@ -1019,7 +1014,7 @@ do_offset: > > if (offset > 0) { > > do { > > h = rb_entry(nd, struct hist_entry, rb_node); > > - if (h->ms.unfolded) { > > + if (h->unfolded) { > > u16 remaining = h->nr_rows - h->row_offset; > > if (offset > remaining) { > > offset -= remaining; > > @@ -1040,7 +1035,7 @@ do_offset: > > } else if (offset < 0) { > > while (1) { > > h = rb_entry(nd, struct hist_entry, rb_node); > > - if (h->ms.unfolded) { > > + if (h->unfolded) { > > if (first) { > > if (-offset > h->row_offset) { > > offset += h->row_offset; > > @@ -1077,7 +1072,7 @@ do_offset: > > * row_offset at its last entry. > > */ > > h = rb_entry(nd, struct hist_entry, rb_node); > > - if (h->ms.unfolded) > > + if (h->unfolded) > > h->row_offset = h->nr_rows; > > break; > > } > > diff --git a/tools/perf/util/callchain.h b/tools/perf/util/callchain.h > > index 6033a0a212ca..679c2c6d8ade 100644 > > --- a/tools/perf/util/callchain.h > > +++ b/tools/perf/util/callchain.h > > @@ -72,6 +72,10 @@ extern struct callchain_param callchain_param; > > struct callchain_list { > > u64 ip; > > struct map_symbol ms; > > + struct /* for TUI */ { > > + bool unfolded; > > + bool has_children; > > + }; > > char *srcline; > > struct list_head list; > > }; > > diff --git a/tools/perf/util/hist.c b/tools/perf/util/hist.c > > index cc22b9158b93..338770679863 100644 > > --- a/tools/perf/util/hist.c > > +++ b/tools/perf/util/hist.c > > @@ -1163,7 +1163,7 @@ static void hists__remove_entry_filter(struct hists *hists, struct hist_entry *h > > return; > > > > /* force fold unfiltered entry for simplicity */ > > - h->ms.unfolded = false; > > + h->unfolded = false; > > h->row_offset = 0; > > h->nr_rows = 0; > > > > diff --git a/tools/perf/util/sort.h b/tools/perf/util/sort.h > > index 4d923e6e0069..e97cd476d336 100644 > > --- a/tools/perf/util/sort.h > > +++ b/tools/perf/util/sort.h > > @@ -109,6 +109,8 @@ struct hist_entry { > > u16 row_offset; > > u16 nr_rows; > > bool init_have_children; > > + bool unfolded; > > + bool has_children; > > }; > > }; > > char *srcline; > > diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h > > index 09561500164a..8483a864a915 100644 > > --- a/tools/perf/util/symbol.h > > +++ b/tools/perf/util/symbol.h > > @@ -158,8 +158,6 @@ struct ref_reloc_sym { > > struct map_symbol { > > struct map *map; > > struct symbol *sym; > > - bool unfolded; > > - bool has_children; > > }; > > > > struct addr_map_symbol { > > -- > > 2.3.5 > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/