From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 6533 invoked by alias); 13 Nov 2013 03:02:19 -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 6518 invoked by uid 89); 13 Nov 2013 03:02:18 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=1.4 required=5.0 tests=AWL,BAYES_50,FREEMAIL_ENVFROM_END_DIGIT,FREEMAIL_FROM,RDNS_NONE,SPF_PASS autolearn=no version=3.3.2 X-HELO: mail-pb0-f42.google.com Received: from Unknown (HELO mail-pb0-f42.google.com) (209.85.160.42) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES128-SHA encrypted) ESMTPS; Wed, 13 Nov 2013 03:02:16 +0000 Received: by mail-pb0-f42.google.com with SMTP id uo5so3671828pbc.1 for ; Tue, 12 Nov 2013 19:02:08 -0800 (PST) X-Received: by 10.66.171.13 with SMTP id aq13mr39757407pac.30.1384311728782; Tue, 12 Nov 2013 19:02:08 -0800 (PST) Received: from seba.sebabeach.org.gmail.com (173-13-178-50-sfba.hfc.comcastbusiness.net. [173.13.178.50]) by mx.google.com with ESMTPSA id ed3sm40824072pbc.6.2013.11.12.19.02.07 for (version=TLSv1.2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Tue, 12 Nov 2013 19:02:07 -0800 (PST) From: Doug Evans To: Pedro Alves , gdb-patches@sourceware.org Subject: [PATCH] Don't evaluate condition for non-matching thread References: <5281F6DB.2010603@redhat.com> Date: Wed, 13 Nov 2013 03:03:00 -0000 In-Reply-To: <5281F6DB.2010603@redhat.com> (Pedro Alves's message of "Tue, 12 Nov 2013 09:37:31 +0000") Message-ID: User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2013-11/txt/msg00335.txt.bz2 Pedro Alves writes: > On 11/12/2013 04:05 AM, Doug Evans wrote: > >> This patch simplifies bpstat_check_breakpoint_conditions a bit. >> >> Regression tested on amd64-linux. >> Ok to check in? > > Looks good to me. Thanks. Next patch. Is there a reason the current code is the way it is? Evaluating breakpoint conditions is expensive, and to do so for a thread-specific breakpoint on a non-matching thread is an awful waste. I read the thread-specific breakpoints section of the docs and didn't find anything. Regression tested on amd64-linux, but no claim is made that I'm sure there isn't some gotcha for why things are the way they are (including "Oops, but it's in the wild now and reason X means we can't change it." :-)). Thank goodness the ignore_count check is done after the thread-id check so there's no change w.r.t. ignore counts. [I realize conditions can have side-effects, to, e.g., collect data, but the user has made the breakpoint thread-specific already.] If you think this warrants a NEWS entry I can certainly add one. Ok to check in? 2013-11-12 Doug Evans * breakpoint.c (bpstat_check_breakpoint_conditions): For thread specific breakpoints, don't evaluate breakpoint condition if different thread. diff --git a/gdb/breakpoint.c b/gdb/breakpoint.c index 36252ee..ebb8336 100644 --- a/gdb/breakpoint.c +++ b/gdb/breakpoint.c @@ -5132,6 +5132,14 @@ bpstat_check_breakpoint_conditions (bpstat bs, ptid_t ptid) return; } + /* If this is a thread-specific breakpoint, don't waste cpu evaluating the + condition if this isn't the specified thread. */ + if (b->thread != -1 && b->thread != thread_id) + { + bs->stop = 0; + return; + } + /* Evaluate Python breakpoints that have a "stop" method implemented. */ if (b->py_bp_object) bs->stop = gdbpy_should_stop (b->py_bp_object); @@ -5215,10 +5223,6 @@ bpstat_check_breakpoint_conditions (bpstat bs, ptid_t ptid) { bs->stop = 0; } - else if (b->thread != -1 && b->thread != thread_id) - { - bs->stop = 0; - } else if (b->ignore_count > 0) { b->ignore_count--;