From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 3010 invoked by alias); 28 Apr 2014 22:06:48 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 2993 invoked by uid 89); 28 Apr 2014 22:06:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-3.2 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_LOW,RP_MATCHES_RCVD,SPF_PASS autolearn=ham version=3.3.2 X-HELO: mail-vc0-f173.google.com Received: from mail-vc0-f173.google.com (HELO mail-vc0-f173.google.com) (209.85.220.173) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Mon, 28 Apr 2014 22:06:46 +0000 Received: by mail-vc0-f173.google.com with SMTP id ik5so4607561vcb.4 for ; Mon, 28 Apr 2014 15:06:44 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:mime-version:in-reply-to:references:date :message-id:subject:from:to:cc:content-type; bh=T5a2IaLLeMco5NRiFkiu4Ko8dZzquMpNIDGeeyWeDy8=; b=CVZXur2kc9Dj6mXvYq8TPxWpsffBPVI68KPP4Uk/9mdaNtQYHCm5oudlUo/k9OPVYu Roz03ujL7plcjJOcmyVZ5j9qjGK2ys/6VCGMZTY2Nuby38aV46IRgEot1INEUzuUaa2r CEvVf3Ge78cOCz8ybDG5e2ye1gIyPPALvYMzwvZlFddoaDn6iz+deqKR0kcsMN5mDSNY i0Up2bYrgfR2XIJM6mkWxP2VoCP2sejGZtr41URYxV5pcEO2UmHfdKa3SwaTgo9u2ZFB 0KL1RqVvgoeFwzC3rmRf0D0SRQjSNBuZoqQjTsgynlagGft267gqNXvmldAUWbiPT7As Enhg== X-Gm-Message-State: ALoCoQlpP/dj7N5iqaLkhT52ifVgS96gl7viIGcVm2Zqf8sIoq90Fk3hcUpcCrD9c04cd52w9G10vL5WfwHoWZo0Ngil5Mc21MtPL+Th8DGi4NHgQBPYxopCH8HMDdBP/gcQeHfwxB4QaGgHUEtp8BZgTX69snc/+MMfZXZr6F+slzUldd+ItAr9nymZimnU5SPYsyW8vrSW+SKOgTqaj7xONl+r4b+gvA== MIME-Version: 1.0 X-Received: by 10.52.3.129 with SMTP id c1mr2905577vdc.37.1398722804031; Mon, 28 Apr 2014 15:06:44 -0700 (PDT) Received: by 10.52.13.101 with HTTP; Mon, 28 Apr 2014 15:06:43 -0700 (PDT) In-Reply-To: <1398716623-16991-1-git-send-email-marc.khouzam@ericsson.com> References: <1398716623-16991-1-git-send-email-marc.khouzam@ericsson.com> Date: Mon, 28 Apr 2014 22:06:00 -0000 Message-ID: Subject: Re: [PATCH] PR breakpoints/15697: Remove =breakpoint-modified when hitting dprintf From: Doug Evans To: Marc Khouzam Cc: gdb-patches Content-Type: text/plain; charset=UTF-8 X-IsSubscribed: yes X-SW-Source: 2014-04/txt/msg00602.txt.bz2 On Mon, Apr 28, 2014 at 1:23 PM, Marc Khouzam wrote: > GDB currently sends a =breakpoint-modified for every dprintf hit. > This can cause performance degradation at the frontend. The below > patch prevents GDB from sending this event for dprintf when the > event is triggered by the dprintf being hit. This means the event > is not sent when the hit-count is incremented or the ignore-count > is decremented due to a hit. > > https://sourceware.org/bugzilla/show_bug.cgi?id=15697 > > This was discussed last year: > http://sourceware.org/ml/gdb-patches/2013-03/msg00260.html > > No regressions. > > Ok? > > Thanks > > Marc > > --- > > DPrintfs can be hit very often since they don't interrupt > the execution. Having a =breakpoint-modified event at > every hit can cause performance degradation at the > frontend. The only value in this event is to indicate that > the hit-count has changed. For the hit-count value however, > it is sufficient for a frontend to get the latest value upon > request and not through an asynchronous event. > > We also remove =breakpoint-modified when hitting a dprintf > and decrementing the ignore count. If the ignore count is > set to a high number, the =breakpoint-modified could also > cause performance degradation as it will be sent at every > dprintf hit which decrements the ignore count. > > 2014-04-28 Marc Khouzam > > PR breakpoints/15697 > * breakpoint.c (bpstat_check_breakpoint_conditions): > Don't call observer_notify_breakpoint_modified for dprintf. > * breakpoint.c (bpstat_stop_status): Ditto. Nit. No need to write "* breakpoint.c" twice. > --- > gdb/breakpoint.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c > index f422998..25615eb 100644 > --- a/gdb/breakpoint.c > +++ b/gdb/breakpoint.c > @@ -5378,7 +5378,8 @@ bpstat_check_breakpoint_conditions (bpstat bs, ptid_t ptid) > bs->stop = 0; > /* Increase the hit count even though we don't stop. */ > ++(b->hit_count); > - observer_notify_breakpoint_modified (b); > + if (b->type != bp_dprintf) > + observer_notify_breakpoint_modified (b); > } > } > > @@ -5515,7 +5516,8 @@ bpstat_stop_status (struct address_space *aspace, > if (bs->stop) > { > ++(b->hit_count); > - observer_notify_breakpoint_modified (b); > + if (b->type != bp_dprintf) > + observer_notify_breakpoint_modified (b); > > /* We will stop here. */ > if (b->disposition == disp_disable) > -- > 1.7.9.5 > Hi. I've read the mentioned thread and I agree with your analysis. The patch is ok with me, but give it a few days for others to comment. Adding a comment explaining why the test is present might be useful, but I'm ok with skipping it. E.g., something like /* Don't send notifications for dprintf, there can be a lot and aren't useful to frontends. */