Received-SPF: pass (google.com: domain of linux-kernel+bounces-205275-linux.lists.archive=gmail.com@vger.kernel.org designates 2604:1380:45e3:2400::1 as permitted sender) client-ip=2604:1380:45e3:2400::1;
Message-ID: <35b43fd3-54ed-49ba-b8c5-6b7d03c9bb8e@amd.com>
Date: Fri, 7 Jun 2024 09:26:14 +0530
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH v1] perf top: Make zeroing histogram on refresh the
 default
To: Arnaldo Carvalho de Melo <acme@kernel.org>
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>,
 ravi.bangoria@amd.com
References: <20240516222159.3710131-1-irogers@google.com>
 <86136e0d-45e9-4bf4-826b-359e4993488f@amd.com> <ZmHDmrLCoYPUCv3n@x1>
Content-Language: en-US
From: Ravi Bangoria <ravi.bangoria@amd.com>
In-Reply-To: <ZmHDmrLCoYPUCv3n@x1>
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 7bit
Precedence: bulk
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0:
	=?utf-8?B?dk5kRkJwbmU3aWh6cFFYZjA1SHJwZUJpclFnYkZKcHpSNHNhd3ZrOE4yUzkz?=
 =?utf-8?B?eURFbE9qcHQ2dDIrd1ZmRVVOZ2ZLejhmbnc1cEF5bjJYcW0wQjBCZGMvcmJm?=
 =?utf-8?B?MjNRcXJWSEhTMFd5WUxCNFBDSnVlUXd4cW56SkF0dU1XZVltVmNlS2xEUFJz?=
 =?utf-8?B?Y25iekJ4UkFsRHNrK2NMWXhZdjRJVG50YUlTUVpzZjBYOXM1ZmZXeEJsS3Iw?=
 =?utf-8?B?b281VENXWVN4UWlsRUpWRGFOZHhBcnFEM0tZM3l4ZHNPaGVBRzVoYVQ1YkY3?=
 =?utf-8?B?T1hWbjFzMXBTYmpmSzdiWmZ6bGhaTHBkbFFBdFVYMUt5V2tYZFhLSHBONHpR?=
 =?utf-8?B?OFJaaDFvcE5GOC92bGlrQWp0ZWxkUklFeVZkME5mNFhvVFRZOGwxTUpxejJS?=
 =?utf-8?B?bXBMd1BkSUcvMEJJYyt4dnN4NWRIeWtwNDBJUHVCdytUM3lKSksxVkxBTmVL?=
 =?utf-8?B?MnZrSGZRWWJyT0RWbGF2MFV6Umh2dk5uMWlKMzkrSGQrNTlPMU9DczRJck96?=
 =?utf-8?B?WERkeVpQQ1JpTVJrOVcwWk9EeUFVZ0JyQmpvVnJYNldxcWJQQ0o4M2twNU5T?=
 =?utf-8?B?SEpzd0w2UzFBbW1tVFV2d2Q4aUYrVHNVQUhIMXVPaG9yLzYreVRxSVg2dWIx?=
 =?utf-8?B?NCtCRkUvc3hTVFlNalpETkt0cGZhRGdQSllCaGhMNEgxRVI0bnZyVDFyWll4?=
 =?utf-8?B?bVZZVThIYWs3Ri9ad1ltMTRud3FlSlhJS1c3YUNZbUhsay9MdVNVVEh1TlM5?=
 =?utf-8?B?alBLOTJkTFFuZDlCM1dpczJqMUZDeElDOWpDVmZuZENVYUUrZXlGMmxEenZn?=
 =?utf-8?B?YzViaVFBdFRyWDR1OVhVK3hiay9URUhnK1Q4Q1VhaFBDd3pjTHJlYUV1NWRW?=
 =?utf-8?B?VnBlcVNuVUJjZm50QXR2NmJzWlp4a1Y4ZU9xbUxHZ1lEcE5UTVV5cG5hRC82?=
 =?utf-8?B?WktkS1BiSE5vQWZ4dWQyS2xRWVhBNkE2U3p4UzQwT2pFb0dGbXQ2RVhXUVZF?=
 =?utf-8?B?NzdwRC9Jc2pvR2dEUmxhZGQ2T3k1ZDlLSHlxNVB2bkdGOFBOVk9oZWQ4RGFC?=
 =?utf-8?B?eXB3bEkzVkdsVm5HNFloUjBvL3BzM1d6c1VCZUVaR3FtOTlIOUYwSFBwbk10?=
 =?utf-8?B?RWpYck5hRjJVSjltUFE3Yk82RFpDYzhxcTJhSUIrVFZQejR4c1cvNWdGWHRI?=
 =?utf-8?B?azkvS0FBOHVMdnN1bFREVG4rcC91ZTNQdlZDUFJxd1d0YmRPaFVPajlOSzRs?=
 =?utf-8?B?TnBxalFlbFJyM1FQSitOV1NlQkhsK3lQMngvdVJiem1lNFhyaGVFUERtc0E0?=
 =?utf-8?B?SEMrTzNSQXZQYS9nTVJKcmJyaStOdlJYWDNGaTJ0U3lvQ010SzV4cENoL3RH?=
 =?utf-8?B?d2dMTFg0N01zRFBvakxzSXlJd0hFb3VXcWQyZzZ5OXQ4djllZGhEY2F2Rk1G?=
 =?utf-8?B?ZlgyQ1BoQVVRUjQ3aTYxU2tvNm5wMjFQRXJSTEFJZU5SYjMyeThlWDlVbUUx?=
 =?utf-8?B?cWdpZnZhRFZielZITFN4c3RXdkl1Y2QwUnVGYmNCVFRMbFFPVGxoQ0VxRFF5?=
 =?utf-8?B?MHVQZnNKTGZZbm9EeDVONUhsdzR4TWRMYUpNTVZpSmFlMzRxQTh6TW1rTEVM?=
 =?utf-8?B?eUwwUzMxRVF3YVRUOWF5OGZrZmFOd2pBa05ZMzRGYkJuMWtVYkowUThWcXlE?=
 =?utf-8?B?bTRoWGRSek1TOFdvRlNQejdraW9Hd01ZSjhjRG1jT3VoWjdhRDJRcjYrWjBQ?=
 =?utf-8?B?dHQvMGt6YWVkMWJ5cjNTSEtjTkNHVkVMeUtyNmxnOWhyNExEZTVRUGp3L1dP?=
 =?utf-8?B?ZE1nRURhK0ZpMWk2bys1RDRXQ2lJQnYvTmNoZ0V4ZEJ6SFBMOWhKdGlRUjNh?=
 =?utf-8?B?Vk5wL1B1SndFeXRyazlJd05LblR0Qjg0UU8vWHFKY0xBODM4T2t2Zk5aNzB3?=
 =?utf-8?B?aGtRa1piNGs5anlnbE9UMEFFaHpNUTFNak5JNk8xRG5QMDc5VTZiUnhjcUwy?=
 =?utf-8?B?MlFZZ0xmQXN6cTFyZmRpT2NHd25PbTl4bUZaYjRZc2t0aU1nQ1N4cFVmdkty?=
 =?utf-8?B?SmVla3gxdVhiV0ZRRWREUDVDUWZ3akpDNXdMRzBKelgva2k3L0E4c0MySlBM?=
 =?utf-8?Q?V6x8y75JNHYpaYGR36RKvI+lA?=
X-OriginatorOrg: amd.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 925089f4-d905-490a-b0d8-08dc86a5cd76
X-MS-Exchange-CrossTenant-AuthSource: PH7PR12MB6588.namprd12.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 07 Jun 2024 03:56:25.7821
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 3dd8961f-e488-4e60-8e11-a82d994e183d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: IzX4d0PUibaajeJ/yofHqZcmcJGRxzlOLYD2YykcIJ19mCQ1kUvJk3DcqKREO/LFwpj0PTdXOD108x9RbtwykA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BL1PR12MB5945

On 6/6/2024 7:41 PM, 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.

Makes sense. Thanks for the clarification.

Ravi