Received-SPF: pass (google.com: domain of linux-kernel+bounces-204423-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Thu, 6 Jun 2024 11:11:38 -0300
From: Arnaldo Carvalho de Melo <acme@kernel.org>
To: Ravi Bangoria <ravi.bangoria@amd.com>
Cc: Ian Rogers <irogers@google.com>, Ingo Molnar <mingo@redhat.com>,
	Namhyung Kim <namhyung@kernel.org>,
	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: <ZmHDmrLCoYPUCv3n@x1>
References: <20240516222159.3710131-1-irogers@google.com>
 <86136e0d-45e9-4bf4-826b-359e4993488f@amd.com>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <86136e0d-45e9-4bf4-826b-359e4993488f@amd.com>

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.

- Arnaldo