From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id F13D8C761A6 for ; Thu, 30 Mar 2023 11:24:27 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231423AbjC3LY1 (ORCPT ); Thu, 30 Mar 2023 07:24:27 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:36416 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S231570AbjC3LY0 (ORCPT ); Thu, 30 Mar 2023 07:24:26 -0400 Received: from dfw.source.kernel.org (dfw.source.kernel.org [139.178.84.217]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 543BB9ED4; Thu, 30 Mar 2023 04:24:17 -0700 (PDT) Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dfw.source.kernel.org (Postfix) with ESMTPS id A7D6C61FF9; Thu, 30 Mar 2023 11:24:16 +0000 (UTC) Received: by smtp.kernel.org (Postfix) with ESMTPSA id E8BABC4339B; Thu, 30 Mar 2023 11:24:15 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1680175456; bh=xFvfkDhlD42bbhG0/jmteNSsxN7TAyt6/qTM1+Zrp4Q=; h=Date:From:To:Cc:Subject:References:In-Reply-To:From; b=AhDEVRDKsT0plQhsp+zjgwto8GumsWvEMq4PGtlbWZaLJRcR+QMecqf7zrZk1cBer ArUUBYmcABR6qGwsvOmSh95C/sASEPyqvbQliNh30X1pXcaHWLQp3pwxgMbmMiclu4 dLBVKFikm6lTMrJZ4tn4byNbaYMszWu2QJDqnKRviROJg7r6ZNmslle4HMgaNQjPKn D6n7yh9NCwZD4XR2hKt9qvKGVDYKOL559qYQTM5Ceyh2+2Tp99TnTIFl5H2CNdY4Gj tOfWSSA4GmtCF95FoUxQaCWW7s7LAa0afiC1a0zXKK5YVsCPx4oBsUeywZkR/tNPXx uWRZLDCCOLX9w== Received: by quaco.ghostprotocols.net (Postfix, from userid 1000) id BBEE74052D; Thu, 30 Mar 2023 08:24:13 -0300 (-03) Date: Thu, 30 Mar 2023 08:24:13 -0300 From: Arnaldo Carvalho de Melo To: Namhyung Kim Cc: Ian Rogers , Peter Zijlstra , Ingo Molnar , Mark Rutland , Alexander Shishkin , Jiri Olsa , Adrian Hunter , Nathan Chancellor , Nick Desaulniers , Tom Rix , James Clark , Andi Kleen , Kan Liang , German Gomez , Sandipan Das , Andres Freund , linux-perf-users@vger.kernel.org, linux-kernel@vger.kernel.org, llvm@lists.linux.dev Subject: Re: [PATCH v1 1/6] perf annotate: Delete session for debug builds Message-ID: References: <20230328235543.1082207-1-irogers@google.com> <20230328235543.1082207-2-irogers@google.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Url: http://acmel.wordpress.com Precedence: bulk List-ID: X-Mailing-List: linux-perf-users@vger.kernel.org Em Wed, Mar 29, 2023 at 05:13:17PM -0700, Namhyung Kim escreveu: > On Wed, Mar 29, 2023 at 6:18 AM Arnaldo Carvalho de Melo > wrote: > > > > Em Wed, Mar 29, 2023 at 10:09:48AM -0300, Arnaldo Carvalho de Melo escreveu: > > > Em Tue, Mar 28, 2023 at 04:55:38PM -0700, Ian Rogers escreveu: > > > > Use the debug build indicator as the guide to free the session. This > > > > implements a behavior described in a comment, which is consequentially > > > > removed. > > > > > > > > Signed-off-by: Ian Rogers > > > > --- > > > > tools/perf/builtin-annotate.c | 16 ++++++---------- > > > > 1 file changed, 6 insertions(+), 10 deletions(-) > > > > > > > > diff --git a/tools/perf/builtin-annotate.c b/tools/perf/builtin-annotate.c > > > > index 4750fac7bf93..98d1b6379230 100644 > > > > --- a/tools/perf/builtin-annotate.c > > > > +++ b/tools/perf/builtin-annotate.c > > > > @@ -692,16 +692,12 @@ int cmd_annotate(int argc, const char **argv) > > > > > > > > out_delete: > > > > /* > > > > - * Speed up the exit process, for large files this can > > > > - * take quite a while. > > > > - * > > > > - * XXX Enable this when using valgrind or if we ever > > > > - * librarize this command. > > > > - * > > > > - * Also experiment with obstacks to see how much speed > > > > - * up we'll get here. > > > > - * > > > > - * perf_session__delete(session); > > > > + * Speed up the exit process by only deleting for debug builds. For > > > > + * large files this can save time. > > > > */ > > > > +#ifndef NDEBUG > > > > + perf_session__delete(annotate.session); > > > > +#endif > > > > > > So now, but default, we will call this, as we don't have this defined > > > only if DEBUG=1 is set, right? > > > > > > ⬢[acme@toolbox perf-tools-next]$ find tools/perf/ -type f | xargs grep NDEBUG > > > tools/perf/util/mutex.c:#ifndef NDEBUG > > > ⬢[acme@toolbox perf-tools-next]$ > > > > We can discuss this later, applied the series with just that zfree() > > change to annotation_options__exit(). > > I don't think it's just an issue in the perf annotate. Maybe we can > do the same for perf report and so on. Yes, I thought at some point of setting some flag, perf_exiting, and then we would stop releasing memory, zfree comes to mind, but then we would still be traversing data structures, taking locks, etc. And we can't just exit() as we may need to flush caches, etc. IIRC this specific case appeared in profiles, so was commented out. > Anyway we could define NDEBUG=1 for release builds from now on. Yes, we should do it. - Arnaldo