linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Namhyung Kim <namhyung@kernel.org>
To: Arnaldo Carvalho de Melo <acme@redhat.com>
Cc: "Wangnan (F)" <wangnan0@huawei.com>,
	Arnaldo Carvalho de Melo <acme@kernel.org>,
	Ingo Molnar <mingo@kernel.org>,
	linux-kernel@vger.kernel.org,
	Adrian Hunter <adrian.hunter@intel.com>,
	Borislav Petkov <bp@suse.de>,
	Chandler Carruth <chandlerc@gmail.com>,
	David Ahern <dsahern@gmail.com>,
	Frederic Weisbecker <fweisbec@gmail.com>,
	Jiri Olsa <jolsa@redhat.com>,
	Stephane Eranian <eranian@google.com>,
	pi3orama <pi3orama@163.com>
Subject: Re: [PATCH 13/16] perf callchain: Switch default to 'graph,0.5,caller'
Date: Wed, 21 Oct 2015 10:44:32 +0900	[thread overview]
Message-ID: <20151021014432.GB628@sejong> (raw)
In-Reply-To: <20151020133816.GD4400@redhat.com>

On Tue, Oct 20, 2015 at 11:38:16AM -0200, Arnaldo Carvalho de Melo wrote:
> Em Tue, Oct 20, 2015 at 09:23:12PM +0800, Wangnan (F) escreveu:
> > On 2015/10/6 5:03, Arnaldo Carvalho de Melo wrote:
> > >From: Arnaldo Carvalho de Melo <acme@redhat.com>
> > >Which is the most common default found in other similar tools.
>  
> > Could you please show me some example about "other similar tools"?
> > For me, in most of the case I prefer callee order because most of my
> > task is to explain the reason why some code get executed too much times
> > than expected.
>  
> > Also, I think changing default settings should be careful.
>  
> > This is my story: after switching to new version of perf, in a period of
> > time there are plenty of perf users in my company be confused by the
> > first column of 'perf report' because the sum of the percentage listed
> > there is much higher than 100%. They find me because they think this is
> > a bug in perf which breaks their routinely profiling work.  The
> > "problem" is caused by the adding of "--children". New perf makes
> > '--children' as the default behavior at the first time it support that
> > option, but the old perf shows things similar to '--no-children'.
> > However, it is hard to explain the principle of call stack accumulation
> > and why we need '--children' to those perf users (they learned perf's
> > command line from others, and don't have enought to read perf
> > documentations or even help output. Althought the title of the first
> > column is changed to 'Children', I don't think they can understand the
> > meaning of it. I think some of them didn't even notice there's an
> > addition column in their output. They just confused and angry). Also,
> > and as you can expect, this change breaks some scripts. In those days I
> > have to make our IM tool response the information of "--no-children"
> > automatically.
> > 
> > This patch changes the default output again. Similar thing will happen
> > another time. I think this time I can make some preparation, for example,
> > prepare new script to restore old behavior?
> 
> I was bitten by the --children thing and took some time to get used to
> it, so I can relate to that...

I feel sorry about that.  I did worry about the existing users when
making the --children default and actually I didn't agree with making
it default at first. :-(


> 
> I think we should revert this change in callchain default, enough
> complaints...  Ingo, since you suggested that change, what are your
> thoughts?
> 
> Changing defaults is hard, there is also the horizontal scrolling that
> made we repurpose the right and left arrows, sigh, that one will cause
> some confusion as well...

Yeah, it worries me too.  That's why I used '<' and '>' key for
scrolling in my patch.  Maybe it's worth adding those keys again,
reverting arrows key actions, and show some message that it'll be
changed later?


> 
> It seems we'll need way more preparation for such changes, more
> infrastructure to ease the transition, questioning if the user wants
> that, etc, growing pains :-\

Yes, it reminds me of changing default push behavior in git 2.0.  We
need to provide info and wait for enough time before changing some
behavior IMHO.

Thanks,
Namhyung

  reply	other threads:[~2015-10-21  1:44 UTC|newest]

Thread overview: 55+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-10-05 21:03 [GIT PULL 00/16] perf/core improvements and fixes Arnaldo Carvalho de Melo
2015-10-05 21:03 ` [PATCH 01/16] tools lib api fs: No need to use PATH_MAX + 1 Arnaldo Carvalho de Melo
2015-10-05 21:03 ` [PATCH 02/16] perf evlist: Display DATA_SRC sample type bit Arnaldo Carvalho de Melo
2015-10-05 21:03 ` [PATCH 03/16] perf annotate: Fix sizeof_sym_hist overflow issue Arnaldo Carvalho de Melo
2015-10-05 21:03 ` [PATCH 04/16] perf tools: Export perf_event_attr__set_max_precise_ip() Arnaldo Carvalho de Melo
2015-10-05 21:03 ` [PATCH 05/16] perf tools: Introduce 'P' modifier to request max precision Arnaldo Carvalho de Melo
2015-10-05 21:03 ` [PATCH 06/16] perf tests: Add parsing test for 'P' modifier Arnaldo Carvalho de Melo
2015-10-05 21:03 ` [PATCH 07/16] perf tools: Add support for sorting on the iaddr Arnaldo Carvalho de Melo
2015-10-05 21:03 ` [PATCH 08/16] perf tools: Setup proper width for symbol_iaddr field Arnaldo Carvalho de Melo
2015-10-05 21:03 ` [PATCH 09/16] perf tools: Handle -h and -v options Arnaldo Carvalho de Melo
2015-10-05 21:03 ` [PATCH 10/16] perf tests: Add arch tests Arnaldo Carvalho de Melo
2015-10-05 21:03 ` [PATCH 11/16] perf tests: Move x86 tests into arch directory Arnaldo Carvalho de Melo
2015-10-05 21:03 ` [PATCH 12/16] perf tests: Add Intel CQM test Arnaldo Carvalho de Melo
2015-10-05 21:03 ` [PATCH 13/16] perf callchain: Switch default to 'graph,0.5,caller' Arnaldo Carvalho de Melo
2015-10-09 20:34   ` Brendan Gregg
2015-10-09 21:56     ` Arnaldo Carvalho de Melo
2015-10-09 22:10       ` Brendan Gregg
2015-10-09 22:25         ` Arnaldo Carvalho de Melo
2015-10-20  0:16           ` Brendan Gregg
2015-10-20 12:00             ` Arnaldo Carvalho de Melo
2015-10-20 12:19               ` Frederic Weisbecker
2015-10-20 13:06                 ` Arnaldo Carvalho de Melo
2015-10-20 17:21                   ` Frederic Weisbecker
2015-10-20 18:44                     ` Arnaldo Carvalho de Melo
2015-10-21  1:21                       ` Namhyung Kim
2015-10-21 13:24                         ` Arnaldo Carvalho de Melo
2015-10-21  8:09                     ` Namhyung Kim
2015-10-21 11:57                       ` Wangnan (F)
2015-10-21 16:35                       ` Frederic Weisbecker
     [not found]                   ` <CAAwGriEtYeBytGt9x24=uUqSEy5oJ2HigfA2KXnKyrAioKrtNg@mail.gmail.com>
2015-10-21 16:27                     ` Frederic Weisbecker
2015-10-21 18:28                     ` Brendan Gregg
2015-10-21 19:23                       ` Arnaldo Carvalho de Melo
2015-10-22  0:44                         ` Brendan Gregg
2015-10-21  8:06               ` Ingo Molnar
2015-10-21 13:21                 ` Arnaldo Carvalho de Melo
2015-10-21 19:18                 ` Brendan Gregg
2015-10-10  7:09         ` Ingo Molnar
2015-10-10  7:34           ` Brendan Gregg
2015-10-10  9:07             ` Ingo Molnar
2015-10-12 15:27   ` Frederic Weisbecker
2015-10-13  4:26   ` Namhyung Kim
2015-10-19 23:50     ` Brendan Gregg
2015-10-21  7:29       ` Namhyung Kim
2015-10-20 13:23   ` Wangnan (F)
2015-10-20 13:38     ` Arnaldo Carvalho de Melo
2015-10-21  1:44       ` Namhyung Kim [this message]
2015-10-21  8:48       ` Ingo Molnar
2015-10-21 13:43         ` Arnaldo Carvalho de Melo
2015-10-21 13:46           ` Arnaldo Carvalho de Melo
2015-10-22  8:46           ` Ingo Molnar
2015-10-22 12:36             ` Namhyung Kim
2015-10-05 21:03 ` [PATCH 14/16] perf ui browser: Optional horizontal scrolling key binding Arnaldo Carvalho de Melo
2015-10-05 21:03 ` [PATCH 15/16] perf hists browser: Implement horizontal scrolling Arnaldo Carvalho de Melo
2015-10-05 21:03 ` [PATCH 16/16] perf tools: Fail properly in case pattern matching fails to find tracepoint Arnaldo Carvalho de Melo
2015-10-06  7:09 ` [GIT PULL 00/16] perf/core improvements and fixes Ingo Molnar

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20151021014432.GB628@sejong \
    --to=namhyung@kernel.org \
    --cc=acme@kernel.org \
    --cc=acme@redhat.com \
    --cc=adrian.hunter@intel.com \
    --cc=bp@suse.de \
    --cc=chandlerc@gmail.com \
    --cc=dsahern@gmail.com \
    --cc=eranian@google.com \
    --cc=fweisbec@gmail.com \
    --cc=jolsa@redhat.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=mingo@kernel.org \
    --cc=pi3orama@163.com \
    --cc=wangnan0@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).