Received-SPF: pass (google.com: domain of linux-kernel+bounces-206595-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Date: Fri, 7 Jun 2024 11:18:57 -0700
From: Namhyung Kim <namhyung@kernel.org>
To: Arnaldo Carvalho de Melo <acme@kernel.org>
Cc: Ravi Bangoria <ravi.bangoria@amd.com>, Ian Rogers <irogers@google.com>,
	Ingo Molnar <mingo@redhat.com>,
	Peter Zijlstra <peterz@infradead.org>,
	Mark Rutland <mark.rutland@arm.com>,
	Alexander Shishkin <alexander.shishkin@linux.intel.com>,
	Jiri Olsa <jolsa@kernel.org>,
	Adrian Hunter <adrian.hunter@intel.com>,
	Kan Liang <kan.liang@linux.intel.com>,
	Changbin Du <changbin.du@huawei.com>,
	John Fastabend <john.fastabend@gmail.com>,
	Andrii Nakryiko <andrii@kernel.org>,
	linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org,
	Stephane Eranian <eranian@google.com>
Subject: Re: [PATCH v1] perf top: Make zeroing histogram on refresh the
 default
Message-ID: <ZmNPETT8oxWSpg72@google.com>
References: <20240516222159.3710131-1-irogers@google.com>
 <86136e0d-45e9-4bf4-826b-359e4993488f@amd.com>
 <ZmHDmrLCoYPUCv3n@x1>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
In-Reply-To: <ZmHDmrLCoYPUCv3n@x1>

On Thu, Jun 06, 2024 at 11:11:38AM -0300, Arnaldo Carvalho de Melo wrote:
> On Thu, Jun 06, 2024 at 10:15:00AM +0530, Ravi Bangoria wrote:
> > On 5/17/2024 3:51 AM, Ian Rogers wrote:
> > > Instead of decaying histograms over time change it so that they are
> > > zero-ed on each perf top refresh. Previously the option '-z', or
> > > pressing 'z' in tui mode, would enable this behavior. Decaying samples
> > > is non-intuitive as it isn't how "top" works. Make zeroing on refresh
> > > the default and rename the command line options from 'z' to 'Z' and
> > > 'zero' to 'decay'.
> 
> > I've also felt `perf top` decay as non-intuitive. Esp. when system becomes
> > idle after some heavy workload, even decayed samples are far more compared
> > to samples from currently running processes and thus `perf top` keeps
> > showing already finished processes at the top, which is kind of confusing.
> > fwiw:
>  
> > Acked-by: Ravi Bangoria <ravi.bangoria@amd.com>
> 
> Thanks for voicing your opinion, that is really helpful.
> 
> Changing tool behaviour can have unintended consequences even when done
> with the best intentions and analysis, that is why I'm wary of doing it.
> 
> The --children case generated complaints when we made it the default, so
> we ended up with a ~/.perfconfig option to disable it:
> 
> root@number:~# perf config top.children=false
> root@number:~# perf top -g
> 
> Or enable explicitely:
> 
> root@number:~# perf config top.children=true
> root@number:~# perf top -g
> 
> Same thing with the build id cache, where one can disable it using 'perf
> config', etc.
> 
> So I'd do this initially with a 'perf config top.refresh=zero' instead
> of changing something so few people complained as not being intuitive
> after all those years of having that default.

This kind of thing happens periodically, I guess we can add some message
to help users to set the default config when it's not set.  In TUI, it
can even set the config directly according to the users response.

Thanks,
Namhyung